Skip to content

Commit

Permalink
8320331: G1 Full GC Heap verification relies on metadata not reset be…
Browse files Browse the repository at this point in the history
…fore verification

Reviewed-by: iwalulya, ayang
  • Loading branch information
Thomas Schatzl committed Nov 22, 2023
1 parent 93bdc2a commit 1629a90
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 8 deletions.
2 changes: 1 addition & 1 deletion src/hotspot/share/gc/g1/g1CollectedHeap.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
1 change: 1 addition & 0 deletions src/hotspot/share/gc/g1/g1FullCollector.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,7 @@ class PrepareRegionsClosure : public HeapRegionClosure {
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;
Expand Down
17 changes: 12 additions & 5 deletions src/hotspot/share/gc/g1/heapRegion.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,7 @@ class HeapRegion : public CHeapObj<mtGC> {

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.
Expand Down Expand Up @@ -229,11 +230,17 @@ class HeapRegion : public CHeapObj<mtGC> {
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.
Expand Down
9 changes: 8 additions & 1 deletion src/hotspot/share/gc/g1/heapRegion.inline.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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) {
Expand Down
12 changes: 11 additions & 1 deletion test/hotspot/jtreg/runtime/Metaspace/FragmentMetaspace.java
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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;

Expand Down

3 comments on commit 1629a90

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@GoeLin
Copy link
Member

@GoeLin GoeLin commented on 1629a90 Apr 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/backport jdk21u-dev

@openjdk
Copy link

@openjdk openjdk bot commented on 1629a90 Apr 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@GoeLin the backport was successfully created on the branch backport-GoeLin-1629a905 in my personal fork of openjdk/jdk21u-dev. To create a pull request with this backport targeting openjdk/jdk21u-dev:master, just click the following link:

➡️ Create pull request

The title of the pull request is automatically filled in correctly and below you find a suggestion for the pull request body:

Hi all,

This pull request contains a backport of commit 1629a905 from the openjdk/jdk repository.

The commit being backported was authored by Thomas Schatzl on 22 Nov 2023 and was reviewed by Ivan Walulya and Albert Mingkun Yang.

Thanks!

If you need to update the source branch of the pull then run the following commands in a local clone of your personal fork of openjdk/jdk21u-dev:

$ git fetch https://github.com/openjdk-bots/jdk21u-dev.git backport-GoeLin-1629a905:backport-GoeLin-1629a905
$ git checkout backport-GoeLin-1629a905
# make changes
$ git add paths/to/changed/files
$ git commit --message 'Describe additional changes made'
$ git push https://github.com/openjdk-bots/jdk21u-dev.git backport-GoeLin-1629a905

Please sign in to comment.