RFR: 8139800: Remove OopsInGenClosure
Stefan Karlsson
stefan.karlsson at oracle.com
Tue Aug 25 10:15:31 UTC 2020
Hi all,
Please review this patch to remove OopsInGenClosure.
https://cr.openjdk.java.net/~stefank/8139800/webrev.01/
https://bugs.openjdk.java.net/browse/JDK-8139800
The patch builds upon the cleanup patches that are current out for
review: JDK-8252245, JDK-8252289, JDK-8252294.
- The preparation patches remove most external usages of
OopsInGenClosure. However, there are two usages left:
oop_since_save_marks_iterate and young_process_roots. These usages call
set_generation before applying the closure, and reset_generation()
after. I'll explain below why those calls are not needed anymore:
---
After the recent cleanups, FastScanClosure is the only concrete class of
OopsInGenClosure. FastScanClosure is used to "scan" the young generation
(DefNew). But, it is actually used in two different situations, and
require different barrier code depending on where it is used:
1) Scan the old gen for pointers into the young gen, and apply a store
barrier.
2) Scan non-old gen parts, including oops in CLDs (and associated Metadata)
This can be seen here, where the passed in boolean corresponds to the
FastScanClosure::_gc_barrier variable:
573 FastScanClosure fsc_with_no_gc_barrier(this, false);
574 FastScanClosure fsc_with_gc_barrier(this, true);
This in turn is used here:
77 template <class T> inline void FastScanClosure::do_oop_work(T* p) {
... < Handling the pointer int young gen >
87 if (is_scanning_a_cld()) {
88 do_cld_barrier();
89 } else if (_gc_barrier) {
90 // Now call parent closure
91 do_barrier(p);
92 }
Where line 91 deals with the (1) case above, and 87-88 deals with the
(2) case.
The do_barrier function called in 91, is provided by OopsInGenClosure.
The CLD barrier code is provided by OopsInClassLoaderDataOrGenClosure.
This class inherits from OopsInGenClosure, but doesn't use the
functionality. It inherits from OopsInGenClosure so that they both can
be passed through the same code path.
In the patch common code is extracted into a revised FastScanClosure
super class, that only performs the young gen scanning code, and
delegates the barrier code to the derived classes.
Two derived classes are created:
- DefNewYoungerGenClosure provides the old-to-young gen write barrier
and is used for case (1).
- DefNewScanClosure provides the CLD barrier for case (2).
After the separation, it's apparent that DefNewScanClosure doesn't use
the OopsInGenClosure code, and the only external usages are calls to
set_generation/reset_generation. There are no actual usages of the
values set by those functions. The set_generation/reset_generation calls
that can be removed are here:
SerialHeap::young_process_roots:
...
if (_process_strong_tasks->try_claim_task(GCH_PS_younger_gens)) {
- root_closure->reset_generation();
}
and template <typename OopClosureType>
void DefNewGeneration::oop_since_save_marks_iterate(OopClosureType* cl) {
- cl->set_generation(this);
eden()->oop_since_save_marks_iterate(cl);
to()->oop_since_save_marks_iterate(cl);
from()->oop_since_save_marks_iterate(cl);
- cl->reset_generation();
DefNewYoungerGenClosure does use the variables changed by
set_generation/reset_generation, but the only callers always pass in the
old generation:
void TenuredGeneration::oop_since_save_marks_iterate(OopClosureType*
blk) {
- blk->set_generation(this);
_the_space->oop_since_save_marks_iterate(blk);
- blk->reset_generation();
and:
void SerialHeap::young_process_roots
...
- old_gen_closure->set_generation(_old_gen);
rem_set()->at_younger_refs_iterate();
old_gen()->younger_refs_iterate(old_gen_closure, scope->n_threads());
- old_gen_closure->reset_generation();
So, instead of doing this over and over again, the code is moved to the
constructor. After that, there's no external usages of OopsInGenClosure,
and the class can be removed from the class hierarchy.
---
- I've renamed some of the variables to make it more apparent what
generation the code is dealing with.
- Note: The FastScanClosure and its derived classes should be moved to
gc/serial, but I've left that for a separate RFE.
- Note: GCH_PS_younger_gens should probably be removed, but I've left
that cleanup for a separate RFE.
Testing: Testing on tier1-tier7 on Linux with forced usages of
-XX:+UseSerialGC.
Thanks,
StefanK
More information about the hotspot-gc-dev
mailing list