From bde01f316e2b159e7bc712062cadea4a14a92a08 Mon Sep 17 00:00:00 2001 From: johnc Date: Thu, 3 Jan 2013 16:28:22 -0800 Subject: [PATCH] 8004816: G1: Kitchensink failures after marking stack changes Summary: Reset the marking state, including the mark stack overflow flag, in the event of a marking stack overflow during serial reference processing. Reviewed-by: jmasa --- .../gc_implementation/g1/concurrentMark.cpp | 58 +++++++++---------- .../gc_implementation/g1/concurrentMark.hpp | 15 +++-- .../g1/concurrentMarkThread.cpp | 12 ++-- 3 files changed, 43 insertions(+), 42 deletions(-) diff --git a/src/share/vm/gc_implementation/g1/concurrentMark.cpp b/src/share/vm/gc_implementation/g1/concurrentMark.cpp index e03b2f41c..3199cbf5a 100644 --- a/src/share/vm/gc_implementation/g1/concurrentMark.cpp +++ b/src/share/vm/gc_implementation/g1/concurrentMark.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2001, 2012, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2001, 2013, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -192,6 +192,7 @@ bool CMMarkStack::allocate(size_t capacity) { setEmpty(); _capacity = (jint) capacity; _saved_index = -1; + _should_expand = false; NOT_PRODUCT(_max_depth = 0); return true; } @@ -747,8 +748,8 @@ void ConcurrentMark::reset() { assert(_heap_end != NULL, "heap bounds should look ok"); assert(_heap_start < _heap_end, "heap bounds should look ok"); - // reset all the marking data structures and any necessary flags - clear_marking_state(); + // Reset all the marking data structures and any necessary flags + reset_marking_state(); if (verbose_low()) { gclog_or_tty->print_cr("[global] resetting"); @@ -766,6 +767,23 @@ void ConcurrentMark::reset() { set_concurrent_marking_in_progress(); } + +void ConcurrentMark::reset_marking_state(bool clear_overflow) { + _markStack.set_should_expand(); + _markStack.setEmpty(); // Also clears the _markStack overflow flag + if (clear_overflow) { + clear_has_overflown(); + } else { + assert(has_overflown(), "pre-condition"); + } + _finger = _heap_start; + + for (uint i = 0; i < _max_worker_id; ++i) { + CMTaskQueue* queue = _task_queues->queue(i); + queue->set_empty(); + } +} + void ConcurrentMark::set_phase(uint active_tasks, bool concurrent) { assert(active_tasks <= _max_worker_id, "we should not have more"); @@ -796,7 +814,7 @@ void ConcurrentMark::set_phase(uint active_tasks, bool concurrent) { void ConcurrentMark::set_non_marking_state() { // We set the global marking state to some default values when we're // not doing marking. - clear_marking_state(); + reset_marking_state(); _active_tasks = 0; clear_concurrent_marking_in_progress(); } @@ -963,7 +981,7 @@ void ConcurrentMark::enter_first_sync_barrier(uint worker_id) { // not clear the overflow flag since we rely on it being true when // we exit this method to abort the pause and restart concurent // marking. - clear_marking_state(concurrent() /* clear_overflow */); + reset_marking_state(concurrent() /* clear_overflow */); force_overflow()->update(); if (G1Log::fine()) { @@ -1257,8 +1275,9 @@ void ConcurrentMark::checkpointRootsFinal(bool clear_all_soft_refs) { if (has_overflown()) { // Oops. We overflowed. Restart concurrent marking. _restart_for_overflow = true; - // Clear the flag. We do not need it any more. - clear_has_overflown(); + // Clear the marking state because we will be restarting + // marking due to overflowing the global mark stack. + reset_marking_state(); if (G1TraceMarkStackOverflow) { gclog_or_tty->print_cr("\nRemark led to restart for overflow."); } @@ -1282,6 +1301,8 @@ void ConcurrentMark::checkpointRootsFinal(bool clear_all_soft_refs) { /* option */ VerifyOption_G1UseNextMarking); } assert(!restart_for_overflow(), "sanity"); + // Completely reset the marking state since marking completed + set_non_marking_state(); } // Expand the marking stack, if we have to and if we can. @@ -1289,11 +1310,6 @@ void ConcurrentMark::checkpointRootsFinal(bool clear_all_soft_refs) { _markStack.expand(); } - // Reset the marking state if marking completed - if (!restart_for_overflow()) { - set_non_marking_state(); - } - #if VERIFY_OBJS_PROCESSED _scan_obj_cl.objs_processed = 0; ThreadLocalObjQueue::objs_enqueued = 0; @@ -2963,22 +2979,6 @@ void ConcurrentMark::verify_no_cset_oops(bool verify_stacks, } #endif // PRODUCT -void ConcurrentMark::clear_marking_state(bool clear_overflow) { - _markStack.set_should_expand(); - _markStack.setEmpty(); // Also clears the _markStack overflow flag - if (clear_overflow) { - clear_has_overflown(); - } else { - assert(has_overflown(), "pre-condition"); - } - _finger = _heap_start; - - for (uint i = 0; i < _max_worker_id; ++i) { - CMTaskQueue* queue = _task_queues->queue(i); - queue->set_empty(); - } -} - // Aggregate the counting data that was constructed concurrently // with marking. class AggregateCountDataHRClosure: public HeapRegionClosure { @@ -3185,7 +3185,7 @@ void ConcurrentMark::abort() { // Clear the liveness counting data clear_all_count_data(); // Empty mark stack - clear_marking_state(); + reset_marking_state(); for (uint i = 0; i < _max_worker_id; ++i) { _tasks[i]->clear_region_fields(); } diff --git a/src/share/vm/gc_implementation/g1/concurrentMark.hpp b/src/share/vm/gc_implementation/g1/concurrentMark.hpp index 8089c9829..34c5601e0 100644 --- a/src/share/vm/gc_implementation/g1/concurrentMark.hpp +++ b/src/share/vm/gc_implementation/g1/concurrentMark.hpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2001, 2012, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2001, 2013, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -478,16 +478,19 @@ protected: // It resets the global marking data structures, as well as the // task local ones; should be called during initial mark. void reset(); - // It resets all the marking data structures. - void clear_marking_state(bool clear_overflow = true); - // It should be called to indicate which phase we're in (concurrent - // mark or remark) and how many threads are currently active. - void set_phase(uint active_tasks, bool concurrent); + // Resets all the marking data structures. Called when we have to restart + // marking or when marking completes (via set_non_marking_state below). + void reset_marking_state(bool clear_overflow = true); + // We do this after we're done with marking so that the marking data // structures are initialised to a sensible and predictable state. void set_non_marking_state(); + // It should be called to indicate which phase we're in (concurrent + // mark or remark) and how many threads are currently active. + void set_phase(uint active_tasks, bool concurrent); + // prints all gathered CM-related statistics void print_stats(); diff --git a/src/share/vm/gc_implementation/g1/concurrentMarkThread.cpp b/src/share/vm/gc_implementation/g1/concurrentMarkThread.cpp index 40d2aece1..3247e8769 100644 --- a/src/share/vm/gc_implementation/g1/concurrentMarkThread.cpp +++ b/src/share/vm/gc_implementation/g1/concurrentMarkThread.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2001, 2012, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2001, 2013, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -159,13 +159,11 @@ void ConcurrentMarkThread::run() { VM_CGC_Operation op(&final_cl, verbose_str, true /* needs_pll */); VMThread::execute(&op); } - if (cm()->restart_for_overflow() && - G1TraceMarkStackOverflow) { - gclog_or_tty->print_cr("Restarting conc marking because of MS overflow " - "in remark (restart #%d).", iter); - } - if (cm()->restart_for_overflow()) { + if (G1TraceMarkStackOverflow) { + gclog_or_tty->print_cr("Restarting conc marking because of MS overflow " + "in remark (restart #%d).", iter); + } if (G1Log::fine()) { gclog_or_tty->date_stamp(PrintGCDateStamps); gclog_or_tty->stamp(PrintGCTimeStamps); -- GitLab