8189748: More precise closures for WeakProcessor::weak_oops_do calls
Reviewed-by: pliden, sjohanss
--- a/src/hotspot/share/compiler/oopMap.cpp Mon Oct 23 03:15:19 2017 -0400
+++ b/src/hotspot/share/compiler/oopMap.cpp Mon Oct 23 11:20:53 2017 +0200
@@ -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);
}
--- a/src/hotspot/share/gc/cms/concurrentMarkSweepGeneration.cpp Mon Oct 23 03:15:19 2017 -0400
+++ b/src/hotspot/share/gc/cms/concurrentMarkSweepGeneration.cpp Mon Oct 23 11:20:53 2017 +0200
@@ -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()) {
{
--- a/src/hotspot/share/gc/cms/parNewGeneration.cpp Mon Oct 23 03:15:19 2017 -0400
+++ b/src/hotspot/share/gc/cms/parNewGeneration.cpp Mon Oct 23 11:20:53 2017 +0200
@@ -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.
--- a/src/hotspot/share/gc/g1/g1ConcurrentMark.cpp Mon Oct 23 03:15:19 2017 -0400
+++ b/src/hotspot/share/gc/g1/g1ConcurrentMark.cpp Mon Oct 23 11:20:53 2017 +0200
@@ -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()) {
--- a/src/hotspot/share/gc/g1/g1MarkSweep.cpp Mon Oct 23 03:15:19 2017 -0400
+++ b/src/hotspot/share/gc/g1/g1MarkSweep.cpp Mon Oct 23 11:20:53 2017 +0200
@@ -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());
--- a/src/hotspot/share/gc/parallel/psMarkSweep.cpp Mon Oct 23 03:15:19 2017 -0400
+++ b/src/hotspot/share/gc/parallel/psMarkSweep.cpp Mon Oct 23 11:20:53 2017 +0200
@@ -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);
--- a/src/hotspot/share/gc/parallel/psParallelCompact.cpp Mon Oct 23 03:15:19 2017 -0400
+++ b/src/hotspot/share/gc/parallel/psParallelCompact.cpp Mon Oct 23 11:20:53 2017 +0200
@@ -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);
--- a/src/hotspot/share/gc/parallel/psScavenge.cpp Mon Oct 23 03:15:19 2017 -0400
+++ b/src/hotspot/share/gc/parallel/psScavenge.cpp Mon Oct 23 11:20:53 2017 +0200
@@ -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) {
--- a/src/hotspot/share/gc/serial/defNewGeneration.cpp Mon Oct 23 03:15:19 2017 -0400
+++ b/src/hotspot/share/gc/serial/defNewGeneration.cpp Mon Oct 23 11:20:53 2017 +0200
@@ -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.
--- a/src/hotspot/share/gc/serial/genMarkSweep.cpp Mon Oct 23 03:15:19 2017 -0400
+++ b/src/hotspot/share/gc/serial/genMarkSweep.cpp Mon Oct 23 11:20:53 2017 +0200
@@ -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());
--- a/src/hotspot/share/gc/shared/weakProcessor.cpp Mon Oct 23 03:15:19 2017 -0400
+++ b/src/hotspot/share/gc/shared/weakProcessor.cpp Mon Oct 23 11:20:53 2017 +0200
@@ -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);
}
--- a/src/hotspot/share/gc/shared/weakProcessor.hpp Mon Oct 23 03:15:19 2017 -0400
+++ b/src/hotspot/share/gc/shared/weakProcessor.hpp Mon Oct 23 11:20:53 2017 +0200
@@ -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);
--- a/src/hotspot/share/memory/iterator.cpp Mon Oct 23 03:15:19 2017 -0400
+++ b/src/hotspot/share/memory/iterator.cpp Mon Oct 23 11:20:53 2017 +0200
@@ -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);
}
--- a/src/hotspot/share/memory/iterator.hpp Mon Oct 23 03:15:19 2017 -0400
+++ b/src/hotspot/share/memory/iterator.hpp Mon Oct 23 11:20:53 2017 +0200
@@ -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.