From 1629a9059bd2e0f07559a384be4276c7dc13eff2 Mon Sep 17 00:00:00 2001 From: Thomas Schatzl Date: Wed, 22 Nov 2023 17:17:11 +0000 Subject: [PATCH] 8320331: G1 Full GC Heap verification relies on metadata not reset before verification Reviewed-by: iwalulya, ayang --- src/hotspot/share/gc/g1/g1CollectedHeap.cpp | 2 +- src/hotspot/share/gc/g1/g1FullCollector.cpp | 1 + src/hotspot/share/gc/g1/heapRegion.hpp | 17 ++++++++++++----- src/hotspot/share/gc/g1/heapRegion.inline.hpp | 9 ++++++++- .../runtime/Metaspace/FragmentMetaspace.java | 12 +++++++++++- 5 files changed, 33 insertions(+), 8 deletions(-) diff --git a/src/hotspot/share/gc/g1/g1CollectedHeap.cpp b/src/hotspot/share/gc/g1/g1CollectedHeap.cpp index ba657e650a0..5b0ac46405d 100644 --- a/src/hotspot/share/gc/g1/g1CollectedHeap.cpp +++ b/src/hotspot/share/gc/g1/g1CollectedHeap.cpp @@ -2765,7 +2765,7 @@ bool G1CollectedHeap::check_young_list_empty() { // Remove the given HeapRegion from the appropriate region set. void G1CollectedHeap::prepare_region_for_full_compaction(HeapRegion* hr) { - if (hr->is_humongous()) { + if (hr->is_humongous()) { _humongous_set.remove(hr); } else if (hr->is_old()) { _old_set.remove(hr); diff --git a/src/hotspot/share/gc/g1/g1FullCollector.cpp b/src/hotspot/share/gc/g1/g1FullCollector.cpp index 039f13019ab..62e7cdfabd4 100644 --- a/src/hotspot/share/gc/g1/g1FullCollector.cpp +++ b/src/hotspot/share/gc/g1/g1FullCollector.cpp @@ -171,6 +171,7 @@ public: PrepareRegionsClosure(G1FullCollector* collector) : _collector(collector) { } bool do_heap_region(HeapRegion* hr) { + hr->prepare_for_full_gc(); G1CollectedHeap::heap()->prepare_region_for_full_compaction(hr); _collector->before_marking_update_attribute_table(hr); return false; diff --git a/src/hotspot/share/gc/g1/heapRegion.hpp b/src/hotspot/share/gc/g1/heapRegion.hpp index 9d366e476b5..45d39924aba 100644 --- a/src/hotspot/share/gc/g1/heapRegion.hpp +++ b/src/hotspot/share/gc/g1/heapRegion.hpp @@ -174,6 +174,7 @@ public: void update_bot_for_block(HeapWord* start, HeapWord* end); + void prepare_for_full_gc(); // Update heap region that has been compacted to be consistent after Full GC. void reset_compacted_after_full_gc(HeapWord* new_top); // Update skip-compacting heap region to be consistent after Full GC. @@ -229,11 +230,17 @@ private: HeapWord* volatile _top_at_mark_start; // The area above this limit is fully parsable. This limit - // is equal to bottom except from Remark and until the region has been - // scrubbed concurrently. The scrubbing ensures that all dead objects (with - // possibly unloaded classes) have beenreplaced with filler objects that - // are parsable. Below this limit the marking bitmap must be used to - // determine size and liveness. + // is equal to bottom except + // + // * from Remark and until the region has been scrubbed concurrently. The + // scrubbing ensures that all dead objects (with possibly unloaded classes) + // have been replaced with filler objects that are parsable. + // * after the marking phase in the Full GC pause until the objects have been + // moved. Some (debug) code iterates over the heap after marking but before + // compaction. + // + // Below this limit the marking bitmap must be used to determine size and + // liveness. HeapWord* volatile _parsable_bottom; // Amount of dead data in the region. diff --git a/src/hotspot/share/gc/g1/heapRegion.inline.hpp b/src/hotspot/share/gc/g1/heapRegion.inline.hpp index 625828b632e..e7e10ea5ffe 100644 --- a/src/hotspot/share/gc/g1/heapRegion.inline.hpp +++ b/src/hotspot/share/gc/g1/heapRegion.inline.hpp @@ -167,6 +167,13 @@ inline size_t HeapRegion::block_size(const HeapWord* p, HeapWord* const pb) cons return cast_to_oop(p)->size(); } +inline void HeapRegion::prepare_for_full_gc() { + // After marking and class unloading the heap temporarily contains dead objects + // with unloaded klasses. Moving parsable_bottom makes some (debug) code correctly + // skip dead objects. + _parsable_bottom = top(); +} + inline void HeapRegion::reset_compacted_after_full_gc(HeapWord* new_top) { set_top(new_top); // After a compaction the mark bitmap in a movable region is invalid. @@ -188,7 +195,7 @@ inline void HeapRegion::reset_skip_compacting_after_full_gc() { inline void HeapRegion::reset_after_full_gc_common() { // Everything above bottom() is parsable and live. - _parsable_bottom = bottom(); + reset_parsable_bottom(); // Clear unused heap memory in debug builds. if (ZapUnusedHeapArea) { diff --git a/test/hotspot/jtreg/runtime/Metaspace/FragmentMetaspace.java b/test/hotspot/jtreg/runtime/Metaspace/FragmentMetaspace.java index 61cebb528b4..8dcc83a1122 100644 --- a/test/hotspot/jtreg/runtime/Metaspace/FragmentMetaspace.java +++ b/test/hotspot/jtreg/runtime/Metaspace/FragmentMetaspace.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2013, 2018, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2013, 2023, 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 @@ -29,6 +29,16 @@ * @run main/othervm/timeout=200 -Xmx1g FragmentMetaspace */ +/** + * @test id=8320331 + * @bug 8320331 + * @requires vm.debug + * @library /test/lib + * @modules java.base/jdk.internal.misc + * @modules java.compiler + * @run main/othervm/timeout=200 -XX:+UnlockDiagnosticVMOptions -XX:+VerifyDuringGC -Xmx1g FragmentMetaspace + */ + import java.io.IOException; import jdk.test.lib.classloader.GeneratingCompilingClassLoader;