--- old/src/hotspot/share/gc/g1/g1HeapVerifier.cpp 2020-06-08 15:04:00.479203478 -0700 +++ new/src/hotspot/share/gc/g1/g1HeapVerifier.cpp 2020-06-08 15:04:00.343198359 -0700 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2016, 2019, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2016, 2020, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -326,7 +326,7 @@ if (cl.has_holes()) { log_warning(gc, verify)("All free regions should be at the top end of the heap, but" " we found holes. This is probably caused by (unmovable) humongous" - " allocations, and may lead to fragmentation while" + " allocations or active GCLocker, and may lead to fragmentation while" " writing archive heap memory regions."); } if (cl.has_humongous()) { @@ -334,7 +334,6 @@ " may lead to fragmentation while" " writing archive heap memory regions."); } - assert(!cl.has_unexpected_holes(), "all holes should have been caused by humongous regions"); } class VerifyArchivePointerRegionClosure: public HeapRegionClosure { --- old/src/hotspot/share/gc/shared/collectedHeap.cpp 2020-06-08 15:04:01.011223504 -0700 +++ new/src/hotspot/share/gc/shared/collectedHeap.cpp 2020-06-08 15:04:00.875218385 -0700 @@ -240,6 +240,7 @@ do_full_collection(false); // don't clear all soft refs break; } + case GCCause::_archive_time_gc: case GCCause::_metadata_GC_clear_soft_refs: { HandleMark hm; do_full_collection(true); // do clear all soft refs --- old/src/hotspot/share/memory/metaspaceShared.cpp 2020-06-08 15:04:01.547243681 -0700 +++ new/src/hotspot/share/memory/metaspaceShared.cpp 2020-06-08 15:04:01.411238562 -0700 @@ -36,6 +36,7 @@ #include "classfile/systemDictionary.hpp" #include "classfile/systemDictionaryShared.hpp" #include "code/codeCache.hpp" +#include "gc/shared/gcLocker.hpp" #include "gc/shared/softRefPolicy.hpp" #include "interpreter/bytecodeStream.hpp" #include "interpreter/bytecodes.hpp" @@ -1229,6 +1230,7 @@ void print_heap_region_stats(GrowableArray *heap_mem, const char *name, size_t total_size); void relocate_to_requested_base_address(CHeapBitMap* ptrmap); + void run_gc(); public: @@ -1636,7 +1638,26 @@ } } +void VM_PopulateDumpSharedSpace::run_gc() { + if (HeapShared::is_heap_object_archiving_allowed()) { + // Avoid fragmentation while archiving heap objects. + // We do this inside a safepoint, so that no further allocation can happen after GC + // has finished. + if (GCLocker::is_active()) { + // This should not happen during -Xshare:dump. If you see this, probably the Java core lib + // has been modified such that JNI code is executed in some clean up threads after + // we have finished class loading. + log_warning(cds)("GC locker is held; GC is skipped. This may produce suboptimal archive heap regions."); + } else { + log_info(cds)("Run GC ..."); + Universe::heap()->collect_as_vm_thread(GCCause::_archive_time_gc); + log_info(cds)("Run GC done"); + } + } +} + void VM_PopulateDumpSharedSpace::doit() { + run_gc(); CHeapBitMap ptrmap; MetaspaceShared::initialize_ptr_marker(&ptrmap); @@ -1955,14 +1976,9 @@ link_and_cleanup_shared_classes(CATCH); log_info(cds)("Rewriting and linking classes: done"); - if (HeapShared::is_heap_object_archiving_allowed()) { - // Avoid fragmentation while archiving heap objects. - Universe::heap()->soft_ref_policy()->set_should_clear_all_soft_refs(true); - Universe::heap()->collect(GCCause::_archive_time_gc); - Universe::heap()->soft_ref_policy()->set_should_clear_all_soft_refs(false); - } - VM_PopulateDumpSharedSpace op; + MutexLocker ml(THREAD, HeapShared::is_heap_object_archiving_allowed() ? + Heap_lock : NULL); // needed for collect_as_vm_thread VMThread::execute(&op); } } --- old/test/hotspot/jtreg/runtime/cds/appcds/javaldr/GCDuringDump.java 2020-06-08 15:04:02.099264460 -0700 +++ new/test/hotspot/jtreg/runtime/cds/appcds/javaldr/GCDuringDump.java 2020-06-08 15:04:01.963259341 -0700 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2017, 2019, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2017, 2020, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -41,6 +41,7 @@ }; public static String agentClasses[] = { GCDuringDumpTransformer.class.getName(), + GCDuringDumpTransformer.MyCleaner.class.getName(), }; public static void main(String[] args) throws Throwable { @@ -55,15 +56,18 @@ String gcLog = Boolean.getBoolean("test.cds.verbose.gc") ? "-Xlog:gc*=info,gc+region=trace,gc+alloc+region=debug" : "-showversion"; - for (int i=0; i<2; i++) { + for (int i=0; i<3; i++) { // i = 0 -- run without agent = no extra GCs // i = 1 -- run with agent = cause extra GCs + // i = 2 -- run with agent = cause extra GCs + use java.lang.ref.Cleaner String extraArg = (i == 0) ? "-showversion" : "-javaagent:" + agentJar; String extraOption = (i == 0) ? "-showversion" : "-XX:+AllowArchivingWithJavaAgent"; + String extraOption2 = (i != 2) ? "-showversion" : "-Dtest.with.cleaner=true"; TestCommon.testDump(appJar, TestCommon.list(Hello.class.getName()), - "-XX:+UnlockDiagnosticVMOptions", extraOption, + "-XX:+UnlockDiagnosticVMOptions", extraOption, extraOption2, + "-Xlog:exceptions=trace", extraArg, "-Xmx32m", gcLog); TestCommon.run( --- old/test/hotspot/jtreg/runtime/cds/appcds/javaldr/GCDuringDumpTransformer.java 2020-06-08 15:04:02.619284035 -0700 +++ new/test/hotspot/jtreg/runtime/cds/appcds/javaldr/GCDuringDumpTransformer.java 2020-06-08 15:04:02.483278915 -0700 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2017, 2019, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2017, 2020, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -25,18 +25,40 @@ import java.lang.instrument.ClassFileTransformer; import java.lang.instrument.Instrumentation; import java.lang.instrument.IllegalClassFormatException; +import java.lang.ref.Cleaner; import java.security.ProtectionDomain; public class GCDuringDumpTransformer implements ClassFileTransformer { + static boolean TEST_WITH_CLEANER = Boolean.getBoolean("test.with.cleaner"); + static Cleaner cleaner; + static Thread thread; + static Object garbage; + + static { + if (TEST_WITH_CLEANER) { + cleaner = Cleaner.create(); + garbage = new Object(); + cleaner.register(garbage, new MyCleaner()); + System.out.println("Registered cleaner"); + } + } + public byte[] transform(ClassLoader loader, String name, Class classBeingRedefined, ProtectionDomain pd, byte[] buffer) throws IllegalClassFormatException { - try { - makeGarbage(); - } catch (Throwable t) { - t.printStackTrace(); + if (TEST_WITH_CLEANER) { + if (name.equals("Hello")) { + garbage = null; + System.out.println("Unreferenced GCDuringDumpTransformer.garbage"); + } + } else { try { - Thread.sleep(200); // let GC to have a chance to run - } catch (Throwable t2) {} + makeGarbage(); + } catch (Throwable t) { + t.printStackTrace(); + try { + Thread.sleep(200); // let GC to have a chance to run + } catch (Throwable t2) {} + } } return null; @@ -45,7 +67,7 @@ private static Instrumentation savedInstrumentation; public static void premain(String agentArguments, Instrumentation instrumentation) { - System.out.println("ClassFileTransformer.premain() is called"); + System.out.println("ClassFileTransformer.premain() is called: TEST_WITH_CLEANER = " + TEST_WITH_CLEANER); instrumentation.addTransformer(new GCDuringDumpTransformer(), /*canRetransform=*/true); savedInstrumentation = instrumentation; } @@ -63,4 +85,15 @@ Object[] a = new Object[10000]; } } + + static class MyCleaner implements Runnable { + static int count = 0; + int i = count++; + public void run() { + // Allocate something. This will cause G1 to allocate an EDEN region. + // See JDK-8245925 + Object o = new Object(); + System.out.println("cleaning " + i); + } + } }