RFR: 8048248: G1 Class Unloading after completing a concurrent mark cycle

[Erik Helin]

Hi Stefan and Mikael,

thanks for all your hard work with this patch!

On Thursday 03 July 2014 21:57:29 PM Stefan Karlsson wrote:
> Hi,
> 
> A new patch can be found at:
> http://cr.openjdk.java.net/~stefank/8048248/webrev.02/
> http://cr.openjdk.java.net/~stefank/8048248/webrev.02.delta/

This looks good to me, Reviewed!

Thanks,
Erik

> The new patch:
> 1) Fixes a bug when the user specifies -XX:ParallelGCThreads=0
> 
> 2) Fixes most of Thomas Schatzl's review comments
> 
> 3) Fixes a bug in how G1RemarkGCTraceTime is used, which caused
> incorrect measurements of the phases "System Dictionary Unloading" and
> "Parallel Unloading".
> 
> thanks,
> StefanK
> 
> On 2014-07-03 13:14, Stefan Karlsson wrote:
> > Hi again,
> > 
> > Here's a new patch.
> > 
> > Changes:
> > 1) The first webrev didn't include the change to this file:
> > http://cr.openjdk.java.net/~stefank/8048248/webrev.01/test/testlibrary/whi
> > tebox/sun/hotspot/WhiteBox.java.udiff.html
> > 
> > 
> > 2) Fixes a bug that happens with Class Redefinition.
> > http://cr.openjdk.java.net/~stefank/8048248/webrev.01/src/share/vm/classfi
> > le/metadataOnStackMark.cpp.udiff.html
> > 
> > 
> > We don't want to call
> > CodeCache::alive_nmethods_do(nmethod::mark_on_stack) unnecessarily,
> > since its one of the more expensive operations done during the remark
> > pause. If Class Redefinition isn't used we don't need to mark through
> > the code cache, since no deallocated metadata should have made its way
> > into a nmethod. Unfortunately, this is not true for Class
> > Redefinition. Class Redefinition will create new versions of Methods
> > and ConstantPools and needs to keep the old versions alive until all
> > references to the old version have been cleaned out from the JVM.
> > 
> > The current patch only calls
> > CodeCache::alive_nmethods_do(nmethod::mark_on_stack) if Class
> > Redefintion is used. This has the effect that code using Class
> > Redefinition will have higher remark pauses.
> > 
> > In an earlier version of the G1 class unloading patch I parallelized
> > and combined the nmethod mark_on_stack code with the CodeCache
> > cleaning code, but it was removed in favor of removing the call to
> > CodeCache::alive_nmethods_do. We might want to revive that patch, or
> > optimize this some other way, but I would prefer to not do that in
> > this patch.
> > 
> > thanks,
> > StefanK
> > 
> > On 2014-07-01 15:44, Stefan Karlsson wrote:
> >> Hi all,
> >> 
> >> Please, review this patch to enable unloading of classes and other
> >> metadata after a G1 concurrent cycle.
> >> 
> >> http://cr.openjdk.java.net/~stefank/8048248/webrev.00/
> >> https://bugs.openjdk.java.net/browse/JDK-8048248
> >> 
> >> The patch includes the following changes:
> >> 
> >> 1) Tracing through alive Klasses and CLDs during concurrent mark,
> >> instead of marking all of them during the initial mark pause.
> >> 2) Making HeapRegions walkable in the presence of unparseable objects
> >> due to their classes being unloaded.
> >> 3) The process roots code has been changed to allow G1's combined
> >> initial mark and scavenge.
> >> 4) The CodeBlobClosures have been refactored to distinguish the
> >> marking variant from the oop updating variants.
> >> 5) Calls to the G1 pre-barrier have been added to some places, such
> >> as the StringTable, to guard against object resurrection, similar to
> >> how j.l.ref.Reference#get is treated with a read barrier.
> >> 6) Parallelizing the cleaning of metadata and compiled methods during
> >> the remark pause.
> >> 
> >> A number of patches to prepare for this RFE has already been pushed
> >> to JDK 9:
> >> 
> >> 8047362: Add a version of CompiledIC_at that doesn't create a new
> >> RelocIterator
> >> 8047326: Consolidate all CompiledIC::CompiledIC implementations and
> >> move it to compiledIC.cpp
> >> 8047323: Remove unused _copy_metadata_obj_cl in
> >> G1CopyingKeepAliveClosure
> >> 8047373: Clean the ExceptionCache in one pass
> >> 8046670: Make CMS metadata aware closures applicable for other
> >> collectors
> >> 8035746: Add missing Klass::oop_is_instanceClassLoader() function
> >> 8035648: Don't use Handle in java_lang_String::print
> >> 8035412: Cleanup ClassLoaderData::is_alive
> >> 8035393: Use CLDClosure instead of CLDToOopClosure in
> >> frame::oops_interpreted_do
> >> 8034764: Use process_strong_roots to adjust the StringTable
> >> 8034761: Remove the do_code_roots parameter from process_strong_roots
> >> 8033923: Use BufferingOopClosure for G1 code root scanning
> >> 8033764: Remove the usage of StarTask from BufferingOopClosure
> >> 8012687: Remove unused is_root checks and closures
> >> 8047818: G1 HeapRegions can no longer be ContiguousSpaces
> >> 8048214: Linker error when compiling G1SATBCardTableModRefBS after
> >> include order changes
> >> 8047821: G1 Does not use the save_marks functionality as intended
> >> 8047820: G1 Block offset table does not need to support generic Space
> >> classes
> >> 8047819: G1 HeapRegionDCTOC does not need to inherit
> >> ContiguousSpaceDCTOC
> >> 8038405: Clean up some virtual fucntions in Space class hierarchy
> >> 8038412: Move object_iterate_careful down from Space to
> >> ContigousSpace and CFLSpace
> >> 8038404: Move object_iterate_mem from Space to CMS since it is only
> >> ever used by CMS
> >> 8038399: Remove dead oop_iterate MemRegion variants from SharedHeap,
> >> Generation and Space classe
> >> 8037958: ConcurrentMark::cleanup leaks BitMaps if VerifyDuringGC is
> >> enabled
> >> 8032379: Remove the is_scavenging flag to process_strong_roots
> >> 
> >> Testing:
> >> 
> >> We've been running Kitchensink, gc-test-suite, internal nightly
> >> testing and test lists, and CRM FA benchmarks.
> >> 
> >> thanks,
> >> StefanK & Mikael Gerdin