--- old/src/share/vm/gc_implementation/concurrentMarkSweep/cmsOopClosures.hpp Fri Nov 14 16:43:37 2008 +++ new/src/share/vm/gc_implementation/concurrentMarkSweep/cmsOopClosures.hpp Fri Nov 14 16:43:36 2008 @@ -325,22 +325,28 @@ // For objects in CMS generation, this closure marks // given objects (transitively) as being reachable/live. // This is currently used during the (weak) reference object -// processing phase of the CMS final checkpoint step. +// processing phase of the CMS final checkpoint step, as +// well as during the concurrent precleaning of the discovered +// reference lists. class CMSKeepAliveClosure: public OopClosure { + friend class CMSDrainMarkingStackClosure; // for assertion checking private: CMSCollector* _collector; const MemRegion _span; CMSMarkStack* _mark_stack; CMSBitMap* _bit_map; + bool _concurrent_precleaning; protected: DO_OOP_WORK_DEFN public: CMSKeepAliveClosure(CMSCollector* collector, MemRegion span, - CMSBitMap* bit_map, CMSMarkStack* mark_stack): + CMSBitMap* bit_map, CMSMarkStack* mark_stack, + bool concurrent_precleaning): _collector(collector), _span(span), _bit_map(bit_map), - _mark_stack(mark_stack) { + _mark_stack(mark_stack), + _concurrent_precleaning(concurrent_precleaning) { assert(!_span.is_empty(), "Empty span could spell trouble"); } virtual void do_oop(oop* p); --- old/src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepGeneration.cpp Fri Nov 14 16:43:41 2008 +++ new/src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepGeneration.cpp Fri Nov 14 16:43:40 2008 @@ -538,6 +538,7 @@ _survivor_chunk_capacity(0), // -- ditto -- _survivor_chunk_index(0), // -- ditto -- _ser_pmc_preclean_ovflw(0), + _ser_kac_preclean_ovflw(0), _ser_pmc_remark_ovflw(0), _par_pmc_remark_ovflw(0), _ser_kac_ovflw(0), @@ -4388,10 +4389,10 @@ CMSPrecleanRefsYieldClosure yield_cl(this); assert(rp->span().equals(_span), "Spans should be equal"); CMSKeepAliveClosure keep_alive(this, _span, &_markBitMap, - &_markStack); + &_markStack, true /* preclean */); CMSDrainMarkingStackClosure complete_trace(this, - _span, &_markBitMap, &_markStack, - &keep_alive); + _span, &_markBitMap, &_markStack, + &keep_alive, true /* preclean */); // We don't want this step to interfere with a young // collection because we don't want to take CPU @@ -4852,17 +4853,19 @@ // recurrence of that condition. assert(_markStack.isEmpty(), "No grey objects"); size_t ser_ovflw = _ser_pmc_remark_ovflw + _ser_pmc_preclean_ovflw + - _ser_kac_ovflw; + _ser_kac_ovflw + _ser_kac_preclean_ovflw; if (ser_ovflw > 0) { if (PrintCMSStatistics != 0) { gclog_or_tty->print_cr("Marking stack overflow (benign) " - "(pmc_pc="SIZE_FORMAT", pmc_rm="SIZE_FORMAT", kac="SIZE_FORMAT")", + "(pmc_pc="SIZE_FORMAT", pmc_rm="SIZE_FORMAT", kac="SIZE_FORMAT + ", kac_preclean="SIZE_FORMAT")", _ser_pmc_preclean_ovflw, _ser_pmc_remark_ovflw, - _ser_kac_ovflw); + _ser_kac_ovflw, _ser_kac_preclean_ovflw); } _markStack.expand(); _ser_pmc_remark_ovflw = 0; _ser_pmc_preclean_ovflw = 0; + _ser_kac_preclean_ovflw = 0; _ser_kac_ovflw = 0; } if (_par_pmc_remark_ovflw > 0 || _par_kac_ovflw > 0) { @@ -5693,10 +5696,10 @@ ReferenceProcessor* rp = ref_processor(); assert(rp->span().equals(_span), "Spans should be equal"); CMSKeepAliveClosure cmsKeepAliveClosure(this, _span, &_markBitMap, - &_markStack); + &_markStack, false /* !preclean */); CMSDrainMarkingStackClosure cmsDrainMarkingStackClosure(this, _span, &_markBitMap, &_markStack, - &cmsKeepAliveClosure); + &cmsKeepAliveClosure, false /* !preclean */); { TraceTime t("weak refs processing", PrintGCDetails, false, gclog_or_tty); if (rp->processing_is_mt()) { @@ -8302,8 +8305,29 @@ } ) if (simulate_overflow || !_mark_stack->push(obj)) { - _collector->push_on_overflow_list(obj); - _collector->_ser_kac_ovflw++; + if (_concurrent_precleaning) { + // We dirty the overflown object and let the remark + // phase deal with it. + assert(_collector->overflow_list_is_empty(), "Error"); + // In the case of object arrays, we need to dirty all of + // the cards that the object spans. No locking or atomics + // are needed since no one else can be mutating the mod union + // table. + if (obj->is_objArray()) { + size_t sz = obj->size(); + HeapWord* end_card_addr = + (HeapWord*)round_to((intptr_t)(addr+sz), CardTableModRefBS::card_size); + MemRegion redirty_range = MemRegion(addr, end_card_addr); + assert(!redirty_range.is_empty(), "Arithmetical tautology"); + _collector->_modUnionTable.mark_range(redirty_range); + } else { + _collector->_modUnionTable.mark(addr); + } + _collector->_ser_kac_preclean_ovflw++; + } else { + _collector->push_on_overflow_list(obj); + _collector->_ser_kac_ovflw++; + } } } } @@ -8400,6 +8424,8 @@ void CMSDrainMarkingStackClosure::do_void() { // the max number to take from overflow list at a time const size_t num = _mark_stack->capacity()/4; + assert(!_concurrent_precleaning || _collector->overflow_list_is_empty(), + "Overflow list should be NULL during concurrent phases"); while (!_mark_stack->isEmpty() || // if stack is empty, check the overflow list _collector->take_from_overflow_list(num, _mark_stack)) { --- old/src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepGeneration.hpp Fri Nov 14 16:43:49 2008 +++ new/src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepGeneration.hpp Fri Nov 14 16:43:48 2008 @@ -592,6 +592,7 @@ size_t _ser_pmc_preclean_ovflw; size_t _ser_pmc_remark_ovflw; size_t _par_pmc_remark_ovflw; + size_t _ser_kac_preclean_ovflw; size_t _ser_kac_ovflw; size_t _par_kac_ovflw; NOT_PRODUCT(size_t _num_par_pushes;) @@ -1749,6 +1750,9 @@ // work-routine/closure used to complete transitive // marking of objects as live after a certain point // in which an initial set has been completely accumulated. +// This closure is currently used both during the final +// remark stop-world phase, as well as during the concurrent +// precleaning of the discovered reference lists. class CMSDrainMarkingStackClosure: public VoidClosure { CMSCollector* _collector; MemRegion _span; @@ -1755,15 +1759,21 @@ CMSMarkStack* _mark_stack; CMSBitMap* _bit_map; CMSKeepAliveClosure* _keep_alive; + bool _concurrent_precleaning; public: CMSDrainMarkingStackClosure(CMSCollector* collector, MemRegion span, CMSBitMap* bit_map, CMSMarkStack* mark_stack, - CMSKeepAliveClosure* keep_alive): + CMSKeepAliveClosure* keep_alive, + bool concurrent_precleaning): _collector(collector), _span(span), _bit_map(bit_map), _mark_stack(mark_stack), - _keep_alive(keep_alive) { } + _keep_alive(keep_alive), + _concurrent_precleaning(concurrent_precleaning) { + assert(_concurrent_precleaning == _keep_alive->_concurrent_precleaning, + "Mismatch"); + } void do_void(); }; --- old/src/share/vm/memory/referenceProcessor.cpp Fri Nov 14 16:43:54 2008 +++ new/src/share/vm/memory/referenceProcessor.cpp Fri Nov 14 16:43:53 2008 @@ -47,7 +47,9 @@ } bool empty() const { return head() == ReferenceProcessor::sentinel_ref(); } size_t length() { return _len; } - void set_length(size_t len) { _len = len; } + void set_length(size_t len) { _len = len; } + void inc_length(size_t inc) { _len += inc; assert(_len > 0, "Error"); } + void dec_length(size_t dec) { _len -= dec; } private: // Set value depending on UseCompressedOops. This could be a template class // but then we have to fix all the instantiations and declarations that use this class. @@ -436,13 +438,13 @@ // The "allow_null_referent" argument tells us to allow for the possibility // of a NULL referent in the discovered Reference object. This typically // happens in the case of concurrent collectors that may have done the - // discovery concurrently or interleaved with mutator execution. + // discovery concurrently, or interleaved, with mutator execution. inline void load_ptrs(DEBUG_ONLY(bool allow_null_referent)); // Move to the next discovered reference. inline void next(); - // Remove the current reference from the list and move to the next. + // Remove the current reference from the list inline void remove(); // Make the Reference object active again. @@ -476,7 +478,6 @@ inline size_t removed() const { return _removed; } ) -private: inline void move_to_next(); private: @@ -553,7 +554,7 @@ oopDesc::store_heap_oop((oop*)_prev_next, _next); } NOT_PRODUCT(_removed++); - move_to_next(); + _refs_list.dec_length(1); } inline void DiscoveredListIterator::move_to_next() { @@ -591,12 +592,13 @@ gclog_or_tty->print_cr("Dropping reference (" INTPTR_FORMAT ": %s" ") by policy", iter.obj(), iter.obj()->blueprint()->internal_name()); } + // Remove Reference object from list + iter.remove(); // Make the Reference object active again iter.make_active(); // keep the referent around iter.make_referent_alive(); - // Remove Reference object from list - iter.remove(); + iter.move_to_next(); } else { iter.next(); } @@ -629,12 +631,13 @@ iter.obj(), iter.obj()->blueprint()->internal_name()); } // The referent is reachable after all. + // Remove Reference object from list. + iter.remove(); // Update the referent pointer as necessary: Note that this // should not entail any recursive marking because the // referent must already have been traversed. iter.make_referent_alive(); - // Remove Reference object from list - iter.remove(); + iter.move_to_next(); } else { iter.next(); } @@ -670,6 +673,7 @@ } else { keep_alive->do_oop((oop*)next_addr); } + iter.move_to_next(); } else { iter.next(); } @@ -832,9 +836,9 @@ } java_lang_ref_Reference::set_discovered(move_tail, ref_lists[to_idx].head()); ref_lists[to_idx].set_head(move_head); - ref_lists[to_idx].set_length(ref_lists[to_idx].length() + refs_to_move); + ref_lists[to_idx].inc_length(refs_to_move); ref_lists[from_idx].set_head(new_head); - ref_lists[from_idx].set_length(ref_lists[from_idx].length() - refs_to_move); + ref_lists[from_idx].dec_length(refs_to_move); } else { ++to_idx; } @@ -923,7 +927,6 @@ void ReferenceProcessor::clean_up_discovered_reflist(DiscoveredList& refs_list) { assert(!discovery_is_atomic(), "Else why call this method?"); DiscoveredListIterator iter(refs_list, NULL, NULL); - size_t length = refs_list.length(); while (iter.has_next()) { iter.load_ptrs(DEBUG_ONLY(true /* allow_null_referent */)); oop next = java_lang_ref_Reference::next(iter.obj()); @@ -941,12 +944,11 @@ ) // Remove Reference object from list iter.remove(); - --length; + iter.move_to_next(); } else { iter.next(); } } - refs_list.set_length(length); NOT_PRODUCT( if (PrintGCDetails && TraceReferenceGC) { gclog_or_tty->print( @@ -1024,7 +1026,7 @@ // We have separate lists for enqueueing so no synchronization // is necessary. refs_list.set_head(obj); - refs_list.set_length(refs_list.length() + 1); + refs_list.inc_length(1); if (_discovered_list_needs_barrier) { _bs->write_ref_field((void*)discovered_addr, current_head); guarantee(false, "Needs to be fixed: YSR"); } @@ -1168,7 +1170,7 @@ _bs->write_ref_field((oop*)discovered_addr, current_head); } list->set_head(obj); - list->set_length(list->length() + 1); + list->inc_length(1); } // In the MT discovery case, it is currently possible to see @@ -1209,13 +1211,13 @@ TraceTime tt("Preclean SoftReferences", PrintGCDetails && PrintReferenceGC, false, gclog_or_tty); for (int i = 0; i < _num_q; i++) { + if (yield->should_return()) { + return; + } preclean_discovered_reflist(_discoveredSoftRefs[i], is_alive, keep_alive, complete_gc, yield); } } - if (yield->should_return()) { - return; - } // Weak references { @@ -1222,13 +1224,13 @@ TraceTime tt("Preclean WeakReferences", PrintGCDetails && PrintReferenceGC, false, gclog_or_tty); for (int i = 0; i < _num_q; i++) { + if (yield->should_return()) { + return; + } preclean_discovered_reflist(_discoveredWeakRefs[i], is_alive, keep_alive, complete_gc, yield); } } - if (yield->should_return()) { - return; - } // Final references { @@ -1235,13 +1237,13 @@ TraceTime tt("Preclean FinalReferences", PrintGCDetails && PrintReferenceGC, false, gclog_or_tty); for (int i = 0; i < _num_q; i++) { + if (yield->should_return()) { + return; + } preclean_discovered_reflist(_discoveredFinalRefs[i], is_alive, keep_alive, complete_gc, yield); } } - if (yield->should_return()) { - return; - } // Phantom references { @@ -1248,6 +1250,9 @@ TraceTime tt("Preclean PhantomReferences", PrintGCDetails && PrintReferenceGC, false, gclog_or_tty); for (int i = 0; i < _num_q; i++) { + if (yield->should_return()) { + return; + } preclean_discovered_reflist(_discoveredPhantomRefs[i], is_alive, keep_alive, complete_gc, yield); } @@ -1256,9 +1261,12 @@ // Walk the given discovered ref list, and remove all reference objects // whose referents are still alive, whose referents are NULL or which -// are not active (have a non-NULL next field). NOTE: For this to work -// correctly, refs discovery can not be happening concurrently with this -// step. +// are not active (have a non-NULL next field). NOTE: When we are +// thus precleaning the ref lists (which happens single-threaded today), +// we do not disable refs discovery to honour the correct semantics of +// java.lang.Reference. As a result, we need to be careful below +// that ref removal steps interleave safely with ref discovery steps +// (in this thread). void ReferenceProcessor::preclean_discovered_reflist(DiscoveredList& refs_list, BoolObjectClosure* is_alive, @@ -1266,7 +1274,6 @@ VoidClosure* complete_gc, YieldClosure* yield) { DiscoveredListIterator iter(refs_list, keep_alive, is_alive); - size_t length = refs_list.length(); while (iter.has_next()) { iter.load_ptrs(DEBUG_ONLY(true /* allow_null_referent */)); oop obj = iter.obj(); @@ -1281,7 +1288,6 @@ } // Remove Reference object from list iter.remove(); - --length; // Keep alive its cohort. iter.make_referent_alive(); if (UseCompressedOops) { @@ -1291,12 +1297,11 @@ oop* next_addr = (oop*)java_lang_ref_Reference::next_addr(obj); keep_alive->do_oop(next_addr); } + iter.move_to_next(); } else { iter.next(); } } - refs_list.set_length(length); - // Close the reachable set complete_gc->do_void(); --- old/src/share/vm/runtime/globals.hpp Fri Nov 14 16:43:59 2008 +++ new/src/share/vm/runtime/globals.hpp Fri Nov 14 16:43:58 2008 @@ -1474,7 +1474,7 @@ "CMSPrecleanNumerator:CMSPrecleanDenominator yields convergence" \ " ratio") \ \ - product(bool, CMSPrecleanRefLists1, false, \ + product(bool, CMSPrecleanRefLists1, true, \ "Preclean ref lists during (initial) preclean phase") \ \ product(bool, CMSPrecleanRefLists2, false, \