# HG changeset patch # User stefank # Date 1508755442 0 # Node ID 7300cb446de8f961c73ede37d21e2916e98a7c4c # Parent 5af0dc07c0e787a1aa9d15e7a643a64ce0ccf0de# Parent b1c020fc35a3b6eab228600cb976812d5f67bace Merge diff -r 5af0dc07c0e7 -r 7300cb446de8 src/hotspot/share/compiler/oopMap.cpp --- a/src/hotspot/share/compiler/oopMap.cpp Mon Oct 23 11:56:30 2017 +0200 +++ b/src/hotspot/share/compiler/oopMap.cpp Mon Oct 23 10:44:02 2017 +0000 @@ -30,6 +30,7 @@ #include "compiler/oopMap.hpp" #include "gc/shared/collectedHeap.hpp" #include "memory/allocation.inline.hpp" +#include "memory/iterator.hpp" #include "memory/resourceArea.hpp" #include "runtime/frame.inline.hpp" #include "runtime/signature.hpp" @@ -263,13 +264,6 @@ return m; } -class DoNothingClosure: public OopClosure { - public: - void do_oop(oop* p) {} - void do_oop(narrowOop* p) {} -}; -static DoNothingClosure do_nothing; - static void add_derived_oop(oop* base, oop* derived) { #if !defined(TIERED) && !defined(INCLUDE_JVMCI) COMPILER1_PRESENT(ShouldNotReachHere();) @@ -310,7 +304,7 @@ void OopMapSet::oops_do(const frame *fr, const RegisterMap* reg_map, OopClosure* f) { // add derived oops to a table - all_do(fr, reg_map, f, add_derived_oop, &do_nothing); + all_do(fr, reg_map, f, add_derived_oop, &do_nothing_cl); } diff -r 5af0dc07c0e7 -r 7300cb446de8 src/hotspot/share/gc/cms/concurrentMarkSweepGeneration.cpp --- a/src/hotspot/share/gc/cms/concurrentMarkSweepGeneration.cpp Mon Oct 23 11:56:30 2017 +0200 +++ b/src/hotspot/share/gc/cms/concurrentMarkSweepGeneration.cpp Mon Oct 23 10:44:02 2017 +0000 @@ -5181,15 +5181,17 @@ rp->setup_policy(false); verify_work_stacks_empty(); - CMSKeepAliveClosure cmsKeepAliveClosure(this, _span, &_markBitMap, - &_markStack, false /* !preclean */); - CMSDrainMarkingStackClosure cmsDrainMarkingStackClosure(this, - _span, &_markBitMap, &_markStack, - &cmsKeepAliveClosure, false /* !preclean */); ReferenceProcessorPhaseTimes pt(_gc_timer_cm, rp->num_q()); { GCTraceTime(Debug, gc, phases) t("Reference Processing", _gc_timer_cm); + // Setup keep_alive and complete closures. + CMSKeepAliveClosure cmsKeepAliveClosure(this, _span, &_markBitMap, + &_markStack, false /* !preclean */); + CMSDrainMarkingStackClosure cmsDrainMarkingStackClosure(this, + _span, &_markBitMap, &_markStack, + &cmsKeepAliveClosure, false /* !preclean */); + ReferenceProcessorStats stats; if (rp->processing_is_mt()) { // Set the degree of MT here. If the discovery is done MT, there @@ -5225,13 +5227,13 @@ pt.print_all_references(); } + // This is the point where the entire marking should have completed. + verify_work_stacks_empty(); + { GCTraceTime(Debug, gc, phases) t("Weak Processing", _gc_timer_cm); - WeakProcessor::weak_oops_do(&_is_alive_closure, &cmsKeepAliveClosure, &cmsDrainMarkingStackClosure); - } - - // This is the point where the entire marking should have completed. - verify_work_stacks_empty(); + WeakProcessor::weak_oops_do(&_is_alive_closure, &do_nothing_cl); + } if (should_unload_classes()) { { diff -r 5af0dc07c0e7 -r 7300cb446de8 src/hotspot/share/gc/cms/parNewGeneration.cpp --- a/src/hotspot/share/gc/cms/parNewGeneration.cpp Mon Oct 23 11:56:30 2017 +0200 +++ b/src/hotspot/share/gc/cms/parNewGeneration.cpp Mon Oct 23 10:44:02 2017 +0000 @@ -1000,7 +1000,13 @@ _gc_tracer.report_tenuring_threshold(tenuring_threshold()); pt.print_all_references(); - WeakProcessor::weak_oops_do(&is_alive, &keep_alive, &evacuate_followers); + assert(gch->no_allocs_since_save_marks(), "evacuation should be done at this point"); + + WeakProcessor::weak_oops_do(&is_alive, &keep_alive); + + // Verify that the usage of keep_alive only forwarded + // the oops and did not find anything new to copy. + assert(gch->no_allocs_since_save_marks(), "unexpectedly copied objects"); if (!promotion_failed()) { // Swap the survivor spaces. diff -r 5af0dc07c0e7 -r 7300cb446de8 src/hotspot/share/gc/g1/g1ConcurrentMark.cpp --- a/src/hotspot/share/gc/g1/g1ConcurrentMark.cpp Mon Oct 23 11:56:30 2017 +0200 +++ b/src/hotspot/share/gc/g1/g1ConcurrentMark.cpp Mon Oct 23 10:44:02 2017 +0000 @@ -1604,23 +1604,6 @@ // Is alive closure. G1CMIsAliveClosure g1_is_alive(g1h); - // Instances of the 'Keep Alive' and 'Complete GC' closures used - // in serial reference processing. Note these closures are also - // used for serially processing (by the the current thread) the - // JNI references during parallel reference processing. - // - // These closures do not need to synchronize with the worker - // threads involved in parallel reference processing as these - // instances are executed serially by the current thread (e.g. - // reference processing is not multi-threaded and is thus - // performed by the current thread instead of a gang worker). - // - // The gang tasks involved in parallel reference processing create - // their own instances of these closures, which do their own - // synchronization among themselves. - G1CMKeepAliveAndDrainClosure g1_keep_alive(this, task(0), true /* is_serial */); - G1CMDrainMarkingStackClosure g1_drain_mark_stack(this, task(0), true /* is_serial */); - // Inner scope to exclude the cleaning of the string and symbol // tables from the displayed time. { @@ -1635,6 +1618,23 @@ rp->setup_policy(clear_all_soft_refs); assert(_global_mark_stack.is_empty(), "mark stack should be empty"); + // Instances of the 'Keep Alive' and 'Complete GC' closures used + // in serial reference processing. Note these closures are also + // used for serially processing (by the the current thread) the + // JNI references during parallel reference processing. + // + // These closures do not need to synchronize with the worker + // threads involved in parallel reference processing as these + // instances are executed serially by the current thread (e.g. + // reference processing is not multi-threaded and is thus + // performed by the current thread instead of a gang worker). + // + // The gang tasks involved in parallel reference processing create + // their own instances of these closures, which do their own + // synchronization among themselves. + G1CMKeepAliveAndDrainClosure g1_keep_alive(this, task(0), true /* is_serial */); + G1CMDrainMarkingStackClosure g1_drain_mark_stack(this, task(0), true /* is_serial */); + // We need at least one active thread. If reference processing // is not multi-threaded we use the current (VMThread) thread, // otherwise we use the work gang from the G1CollectedHeap and @@ -1688,9 +1688,12 @@ assert(!rp->discovery_enabled(), "Post condition"); } + assert(has_overflown() || _global_mark_stack.is_empty(), + "Mark stack should be empty (unless it has overflown)"); + { GCTraceTime(Debug, gc, phases) debug("Weak Processing", _gc_timer_cm); - WeakProcessor::weak_oops_do(&g1_is_alive, &g1_keep_alive, &g1_drain_mark_stack); + WeakProcessor::weak_oops_do(&g1_is_alive, &do_nothing_cl); } if (has_overflown()) { diff -r 5af0dc07c0e7 -r 7300cb446de8 src/hotspot/share/gc/g1/g1MarkSweep.cpp --- a/src/hotspot/share/gc/g1/g1MarkSweep.cpp Mon Oct 23 11:56:30 2017 +0200 +++ b/src/hotspot/share/gc/g1/g1MarkSweep.cpp Mon Oct 23 10:44:02 2017 +0000 @@ -182,16 +182,14 @@ pt.print_all_references(); } + // This is the point where the entire marking should have completed. + assert(GenMarkSweep::_marking_stack.is_empty(), "Marking should have completed"); + { GCTraceTime(Debug, gc, phases) trace("Weak Processing", gc_timer()); - WeakProcessor::weak_oops_do(&GenMarkSweep::is_alive, - &GenMarkSweep::keep_alive, - &GenMarkSweep::follow_stack_closure); + WeakProcessor::weak_oops_do(&GenMarkSweep::is_alive, &do_nothing_cl); } - // This is the point where the entire marking should have completed. - assert(GenMarkSweep::_marking_stack.is_empty(), "Marking should have completed"); - if (ClassUnloading) { GCTraceTime(Debug, gc, phases) trace("Class Unloading", gc_timer()); diff -r 5af0dc07c0e7 -r 7300cb446de8 src/hotspot/share/gc/parallel/psMarkSweep.cpp --- a/src/hotspot/share/gc/parallel/psMarkSweep.cpp Mon Oct 23 11:56:30 2017 +0200 +++ b/src/hotspot/share/gc/parallel/psMarkSweep.cpp Mon Oct 23 10:44:02 2017 +0000 @@ -543,14 +543,14 @@ pt.print_all_references(); } + // This is the point where the entire marking should have completed. + assert(_marking_stack.is_empty(), "Marking should have completed"); + { GCTraceTime(Debug, gc, phases) t("Weak Processing", _gc_timer); - WeakProcessor::weak_oops_do(is_alive_closure(), mark_and_push_closure(), follow_stack_closure()); + WeakProcessor::weak_oops_do(is_alive_closure(), &do_nothing_cl); } - // This is the point where the entire marking should have completed. - assert(_marking_stack.is_empty(), "Marking should have completed"); - { GCTraceTime(Debug, gc, phases) t("Class Unloading", _gc_timer); diff -r 5af0dc07c0e7 -r 7300cb446de8 src/hotspot/share/gc/parallel/psParallelCompact.cpp --- a/src/hotspot/share/gc/parallel/psParallelCompact.cpp Mon Oct 23 11:56:30 2017 +0200 +++ b/src/hotspot/share/gc/parallel/psParallelCompact.cpp Mon Oct 23 10:44:02 2017 +0000 @@ -2119,14 +2119,14 @@ pt.print_all_references(); } + // This is the point where the entire marking should have completed. + assert(cm->marking_stacks_empty(), "Marking should have completed"); + { GCTraceTime(Debug, gc, phases) tm("Weak Processing", &_gc_timer); - WeakProcessor::weak_oops_do(is_alive_closure(), &mark_and_push_closure, &follow_stack_closure); + WeakProcessor::weak_oops_do(is_alive_closure(), &do_nothing_cl); } - // This is the point where the entire marking should have completed. - assert(cm->marking_stacks_empty(), "Marking should have completed"); - { GCTraceTime(Debug, gc, phases) tm_m("Class Unloading", &_gc_timer); diff -r 5af0dc07c0e7 -r 7300cb446de8 src/hotspot/share/gc/parallel/psScavenge.cpp --- a/src/hotspot/share/gc/parallel/psScavenge.cpp Mon Oct 23 11:56:30 2017 +0200 +++ b/src/hotspot/share/gc/parallel/psScavenge.cpp Mon Oct 23 10:44:02 2017 +0000 @@ -407,15 +407,14 @@ scavenge_midpoint.update(); - PSKeepAliveClosure keep_alive(promotion_manager); - PSEvacuateFollowersClosure evac_followers(promotion_manager); - // Process reference objects discovered during scavenge { GCTraceTime(Debug, gc, phases) tm("Reference Processing", &_gc_timer); reference_processor()->setup_policy(false); // not always_clear reference_processor()->set_active_mt_degree(active_workers); + PSKeepAliveClosure keep_alive(promotion_manager); + PSEvacuateFollowersClosure evac_followers(promotion_manager); ReferenceProcessorStats stats; ReferenceProcessorPhaseTimes pt(&_gc_timer, reference_processor()->num_q()); if (reference_processor()->processing_is_mt()) { @@ -442,18 +441,24 @@ pt.print_enqueue_phase(); } + assert(promotion_manager->stacks_empty(),"stacks should be empty at this point"); + + PSScavengeRootsClosure root_closure(promotion_manager); + { GCTraceTime(Debug, gc, phases) tm("Weak Processing", &_gc_timer); - WeakProcessor::weak_oops_do(&_is_alive_closure, &keep_alive, &evac_followers); + WeakProcessor::weak_oops_do(&_is_alive_closure, &root_closure); } { GCTraceTime(Debug, gc, phases) tm("Scrub String Table", &_gc_timer); // Unlink any dead interned Strings and process the remaining live ones. - PSScavengeRootsClosure root_closure(promotion_manager); StringTable::unlink_or_oops_do(&_is_alive_closure, &root_closure); } + // Verify that usage of root_closure didn't copy any objects. + assert(promotion_manager->stacks_empty(),"stacks should be empty at this point"); + // Finally, flush the promotion_manager's labs, and deallocate its stacks. promotion_failure_occurred = PSPromotionManager::post_scavenge(_gc_tracer); if (promotion_failure_occurred) { diff -r 5af0dc07c0e7 -r 7300cb446de8 src/hotspot/share/gc/serial/defNewGeneration.cpp --- a/src/hotspot/share/gc/serial/defNewGeneration.cpp Mon Oct 23 11:56:30 2017 +0200 +++ b/src/hotspot/share/gc/serial/defNewGeneration.cpp Mon Oct 23 10:44:02 2017 +0000 @@ -659,7 +659,12 @@ gc_tracer.report_tenuring_threshold(tenuring_threshold()); pt.print_all_references(); - WeakProcessor::weak_oops_do(&is_alive, &keep_alive, &evacuate_followers); + assert(gch->no_allocs_since_save_marks(), "save marks have not been newly set."); + + WeakProcessor::weak_oops_do(&is_alive, &keep_alive); + + // Verify that the usage of keep_alive didn't copy any objects. + assert(gch->no_allocs_since_save_marks(), "save marks have not been newly set."); if (!_promotion_failed) { // Swap the survivor spaces. diff -r 5af0dc07c0e7 -r 7300cb446de8 src/hotspot/share/gc/serial/genMarkSweep.cpp --- a/src/hotspot/share/gc/serial/genMarkSweep.cpp Mon Oct 23 11:56:30 2017 +0200 +++ b/src/hotspot/share/gc/serial/genMarkSweep.cpp Mon Oct 23 10:44:02 2017 +0000 @@ -218,14 +218,14 @@ gc_tracer()->report_gc_reference_stats(stats); } + // This is the point where the entire marking should have completed. + assert(_marking_stack.is_empty(), "Marking should have completed"); + { GCTraceTime(Debug, gc, phases) tm_m("Weak Processing", gc_timer()); - WeakProcessor::weak_oops_do(&is_alive, &keep_alive, &follow_stack_closure); + WeakProcessor::weak_oops_do(&is_alive, &do_nothing_cl); } - // This is the point where the entire marking should have completed. - assert(_marking_stack.is_empty(), "Marking should have completed"); - { GCTraceTime(Debug, gc, phases) tm_m("Class Unloading", gc_timer()); diff -r 5af0dc07c0e7 -r 7300cb446de8 src/hotspot/share/gc/shared/weakProcessor.cpp --- a/src/hotspot/share/gc/shared/weakProcessor.cpp Mon Oct 23 11:56:30 2017 +0200 +++ b/src/hotspot/share/gc/shared/weakProcessor.cpp Mon Oct 23 10:44:02 2017 +0000 @@ -27,16 +27,12 @@ #include "prims/jvmtiExport.hpp" #include "runtime/jniHandles.hpp" -void WeakProcessor::weak_oops_do(BoolObjectClosure* is_alive, OopClosure* keep_alive, VoidClosure* complete) { +void WeakProcessor::weak_oops_do(BoolObjectClosure* is_alive, OopClosure* keep_alive) { JNIHandles::weak_oops_do(is_alive, keep_alive); JvmtiExport::weak_oops_do(is_alive, keep_alive); - - if (complete != NULL) { - complete->do_void(); - } } void WeakProcessor::oops_do(OopClosure* closure) { AlwaysTrueClosure always_true; - weak_oops_do(&always_true, closure, NULL); + weak_oops_do(&always_true, closure); } diff -r 5af0dc07c0e7 -r 7300cb446de8 src/hotspot/share/gc/shared/weakProcessor.hpp --- a/src/hotspot/share/gc/shared/weakProcessor.hpp Mon Oct 23 11:56:30 2017 +0200 +++ b/src/hotspot/share/gc/shared/weakProcessor.hpp Mon Oct 23 10:44:02 2017 +0000 @@ -37,10 +37,7 @@ // Visit all oop*s and apply the keep_alive closure if the referenced // object is considered alive by the is_alive closure, otherwise do some // container specific cleanup of element holding the oop. - // - // The complete closure is used as a post-processing step, - // called after all container have been processed. - static void weak_oops_do(BoolObjectClosure* is_alive, OopClosure* keep_alive, VoidClosure* complete = NULL); + static void weak_oops_do(BoolObjectClosure* is_alive, OopClosure* keep_alive); // Visit all oop*s and apply the given closure. static void oops_do(OopClosure* closure); diff -r 5af0dc07c0e7 -r 7300cb446de8 src/hotspot/share/memory/iterator.cpp --- a/src/hotspot/share/memory/iterator.cpp Mon Oct 23 11:56:30 2017 +0200 +++ b/src/hotspot/share/memory/iterator.cpp Mon Oct 23 10:44:02 2017 +0000 @@ -29,6 +29,8 @@ #include "utilities/debug.hpp" #include "utilities/globalDefinitions.hpp" +DoNothingClosure do_nothing_cl; + void CLDToOopClosure::do_cld(ClassLoaderData* cld) { cld->oops_do(_oop_closure, _must_claim_cld); } diff -r 5af0dc07c0e7 -r 7300cb446de8 src/hotspot/share/memory/iterator.hpp --- a/src/hotspot/share/memory/iterator.hpp Mon Oct 23 11:56:30 2017 +0200 +++ b/src/hotspot/share/memory/iterator.hpp Mon Oct 23 10:44:02 2017 +0000 @@ -48,6 +48,13 @@ virtual void do_oop(narrowOop* o) = 0; }; +class DoNothingClosure : public OopClosure { + public: + virtual void do_oop(oop* p) {} + virtual void do_oop(narrowOop* p) {} +}; +extern DoNothingClosure do_nothing_cl; + // ExtendedOopClosure adds extra code to be run during oop iterations. // This is needed by the GC and is extracted to a separate type to not // pollute the OopClosure interface.