Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 12 additions & 3 deletions src/hotspot/share/gc/g1/g1ConcurrentMark.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -714,6 +714,11 @@ class G1ClearBitMapTask : public WorkerTask {
}

HeapWord* region_clear_limit(G1HeapRegion* r) {
// A garbage collection might have made the region unavailable after a yield during
// clearing. Just return bottom as the limit, causing the clearing for this region to end.
if (G1CollectedHeap::heap()->region_at_or_null(r->hrm_index()) == nullptr) {
return r->bottom();
}
// During a Concurrent Undo Mark cycle, the per region top_at_mark_start and
// live_words data are current wrt to the _mark_bitmap. We use this information
// to only clear ranges of the bitmap that require clearing.
Expand Down Expand Up @@ -743,7 +748,7 @@ class G1ClearBitMapTask : public WorkerTask {
}

HeapWord* cur = r->bottom();
HeapWord* const end = region_clear_limit(r);
HeapWord* end = region_clear_limit(r);

size_t const chunk_size_in_words = G1ClearBitMapTask::chunk_size() / HeapWordSize;

Expand All @@ -761,8 +766,12 @@ class G1ClearBitMapTask : public WorkerTask {
assert(!suspendible() || _cm->is_in_reset_for_next_cycle(), "invariant");

// Abort iteration if necessary.
if (has_aborted()) {
return true;
if (suspendible() && _cm->do_yield_check()) {
if (_cm->has_aborted()) {
return true;
}
// Re-read end. The region might have been uncommitted.
end = region_clear_limit(r);
}
}
assert(cur >= end, "Must have completed iteration over the bitmap for region %u.", r->hrm_index());
Expand Down
6 changes: 3 additions & 3 deletions src/hotspot/share/gc/g1/g1UncommitRegionTask.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2020, 2025, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2020, 2026, 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 @@ -58,9 +58,9 @@ void G1UncommitRegionTask::enqueue() {

G1UncommitRegionTask* uncommit_task = instance();
if (!uncommit_task->is_active()) {
// Change state to active and schedule using UncommitInitialDelayMs.
// Change state to active and schedule.
uncommit_task->set_active(true);
G1CollectedHeap::heap()->service_thread()->schedule_task(uncommit_task, UncommitInitialDelayMs);
G1CollectedHeap::heap()->service_thread()->schedule_task(uncommit_task, G1UncommitInitialDelay);
}
}

Expand Down
4 changes: 1 addition & 3 deletions src/hotspot/share/gc/g1/g1UncommitRegionTask.hpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2020, 2022, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2020, 2026, 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 @@ -34,8 +34,6 @@ class G1UncommitRegionTask : public G1ServiceTask {
// This limit is small enough to ensure that the duration of each invocation
// is short, while still making reasonable progress.
static const uint UncommitSizeLimit = 128 * M;
// Initial delay in milliseconds after GC before the regions are uncommitted.
static const uint UncommitInitialDelayMs = 100;
// The delay between two uncommit task executions.
static const uint UncommitTaskDelayMs = 10;

Expand Down
6 changes: 5 additions & 1 deletion src/hotspot/share/gc/g1/g1_globals.hpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2001, 2025, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2001, 2026, 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 @@ -184,6 +184,10 @@
"shrink attempt.") \
range(0, 100) \
\
develop(uint, G1UncommitInitialDelay, 100, \
"Delay in milliseconds until regions just made eligible for " \
"uncommit are actually uncommitted.") \
\
product(uint, G1CPUUsageDeviationPercent, 25, DIAGNOSTIC, \
"The acceptable deviation (in percent) from the target GC CPU " \
"usage (based on GCTimeRatio). Creates a tolerance range " \
Expand Down
9 changes: 1 addition & 8 deletions src/hotspot/share/runtime/frame.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1237,9 +1237,6 @@ void frame::interpreter_frame_verify_monitor(BasicObjectLock* value) const {

#ifndef PRODUCT

// Returns true iff the address p is readable and *(intptr_t*)p != errvalue
extern "C" bool dbg_is_safe(const void* p, intptr_t errvalue);

class FrameValuesOopClosure: public OopClosure, public DerivedOopClosure {
private:
GrowableArray<oop*>* _oops;
Expand Down Expand Up @@ -1269,17 +1266,13 @@ class FrameValuesOopClosure: public OopClosure, public DerivedOopClosure {
_derived->push(derived_loc);
}

bool is_good(oop* p) {
return *p == nullptr || (dbg_is_safe(*p, -1) && dbg_is_safe((*p)->klass_without_asserts(), -1) && oopDesc::is_oop_or_null(*p));
}
void describe(FrameValues& values, int frame_no) {
for (int i = 0; i < _oops->length(); i++) {
oop* p = _oops->at(i);
values.describe(frame_no, (intptr_t*)p, err_msg("oop%s for #%d", is_good(p) ? "" : " (BAD)", frame_no));
values.describe(frame_no, (intptr_t*)p, err_msg("oop for #%d", frame_no));
}
for (int i = 0; i < _narrow_oops->length(); i++) {
narrowOop* p = _narrow_oops->at(i);
// we can't check for bad compressed oops, as decoding them might crash
values.describe(frame_no, (intptr_t*)p, err_msg("narrow oop for #%d", frame_no));
}
assert(_base->length() == _derived->length(), "should be the same");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -235,22 +235,46 @@ private void processPath(
return;
}
String modName = fullPath.substring(1, modEnd);
String pkgPath = fullPath.substring(modEnd + 1);
String resPath = fullPath.substring(modEnd + 1);
int pathEnd = resPath.lastIndexOf('/');

Node parentNode = getDirectoryNode(modName, modulesRoot);
boolean isPreviewPath = false;
if (pkgPath.startsWith(PREVIEW_PREFIX)) {
if (resPath.startsWith("META-INF/")) {
parentNode = getDirectoryNode("META-INF", parentNode);
if (!resPath.startsWith(PREVIEW_PREFIX)) {
// Non-preview META-INF paths are resources, not package
// hierarchies, so directory and file names may contain dots.
for (int i = "META-INF".length(), j; i != pathEnd; i = j) {
j = resPath.indexOf('/', i + 1);
parentNode = getDirectoryNode(resPath.substring(i + 1, j), parentNode);
}
String resourceName = resPath.substring(pathEnd + 1);
Node resourceNode = parentNode.getChildren(resourceName);
if (resourceNode == null) {
new ResourceNode(resourceName, parentNode);
} else if (!(resourceNode instanceof ResourceNode)) {
throw new InvalidTreeException(resourceNode);
}
return;
}
// For preview paths, process nodes relative to the preview directory.
pkgPath = pkgPath.substring(PREVIEW_PREFIX.length());
Node metaInf = getDirectoryNode("META-INF", parentNode);
parentNode = getDirectoryNode("preview", metaInf);
parentNode = getDirectoryNode("preview", parentNode);
resPath = resPath.substring(PREVIEW_PREFIX.length());
pathEnd -= PREVIEW_PREFIX.length();
isPreviewPath = true;
}

int pathEnd = pkgPath.lastIndexOf('/');
// From invariants tested above, this must now be well-formed.
String fullPkgName = (pathEnd == -1) ? "" : pkgPath.substring(0, pathEnd).replace('/', '.');
String resourceName = pkgPath.substring(pathEnd + 1);
String pkgPath = (pathEnd == -1) ? "" : resPath.substring(0, pathEnd);
if (pkgPath.contains(".")) {
// Non META-INF entries are package paths. Dots in path segment
// names would otherwise be confused with package separators.
System.err.println("Invalid package path, skipping " + pkgPath);
return;
}
String fullPkgName = pkgPath.replace('/', '.');
String resourceName = resPath.substring(pathEnd + 1);
// Intermediate packages are marked "empty" (no resources). This might
// later be merged with a non-empty link for the same package.
ModuleLink emptyLink = ModuleLink.forEmptyPackage(modName, isPreviewPath);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,133 @@
/*
* Copyright (c) 2026, 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
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/

package gc.g1;

/*
* @test TestUncommitDuringConcurrentBitmapClear
* @bug 8385369
* @requires vm.gc.G1
* @requires vm.debug
* @requires vm.flagless
* @requires vm.bits == 64
* @requires os.maxMemory > 8g
* @summary Verify that G1 does not crash while uncommitting a region whose
* bitmap is currently being cleared.
* Options are geared towards uncommitting aggressively. Also use a large
* region size so that corresponding bitmaps get uncommitted always too.
* @library /test/lib
* @build jdk.test.whitebox.WhiteBox
* @run driver jdk.test.lib.helpers.ClassFileInstaller jdk.test.whitebox.WhiteBox
* @run main/othervm
* -Xbootclasspath/a:.
* -XX:+UnlockDiagnosticVMOptions -XX:+WhiteBoxAPI
* -Xmx8g
* -Xms32m
* -XX:G1HeapRegionSize=16m
* -XX:+UseG1GC
* -XX:ConcGCThreads=1
* -XX:GCTimeRatio=1
* -XX:G1CPUUsageShrinkThreshold=1
* -XX:G1ShrinkByPercentOfAvailable=100
* -XX:G1UncommitInitialDelay=0
* -Xlog:gc+marking,gc,gc+ergo+heap=debug
* gc.g1.TestUncommitDuringConcurrentBitmapClear
*/

import java.util.concurrent.FutureTask;
import java.util.concurrent.TimeUnit;

import jdk.test.whitebox.WhiteBox;

/*
* Repeatedly make the concurrent cycle stop after the cleanup pause, issuing
* young GCs during the Concurrent Cleanup for Next Mark phase. Humongous
* regions allocated and dropped before that should get eager-reclaimed and
* their memory uncommitted while bitmap clearing runs.
*/
public class TestUncommitDuringConcurrentBitmapClear {

private static final WhiteBox WB = WhiteBox.getWhiteBox();

private static final int NumObjs = 400; // Number of humongous objects to allocate
// per attempt. Sized to fill a fair amount of
// the available memory.
private static final int LargeObjSize = 9 * 1024 * 1024; // Large enough to be a humongous object.

private static Object[] objects;

private static void test() throws Exception {

// This task drops the humongous objects, making them eligible for
// uncommit, and starts the concurrent bitmap clearing. While it is
// running, the caller triggers GCs that may or may not trigger the issue.
FutureTask<Void> concurrentClearTask = new FutureTask<>(() -> {
objects = null;
WB.concurrentGCRunTo(WB.G1_BEFORE_CLEANUP_COMPLETED);
return null;
});

try {
System.out.println("taking control");
WB.concurrentGCAcquireControl();

// Allocate a new set of humongous objects. Acquire control first to avoid
// unnecessary concurrent cycles due to that allocation. We do not need them.
objects = new Object[NumObjs];
for (int i = 0; i < objects.length; i++) {
objects[i] = new byte[LargeObjSize];
}

WB.concurrentGCRunTo(WB.G1_AFTER_CLEANUP_STARTED);

new Thread(concurrentClearTask).start();

int numYoungGCs = 0;
// Execute at least one young GC, even if the concurrent
// clear bitmap finishes very quickly.
do {
WB.youngGC();
numYoungGCs++;
// Wait a bit. This should give the concurrent clear task a chance
// to finish execution.
Thread.sleep(1);
} while (!concurrentClearTask.isDone() && numYoungGCs < 200);

concurrentClearTask.get(30, TimeUnit.SECONDS); // Propagates exceptions, if any.
} finally {
WB.concurrentGCRunToIdle();

System.out.println("Releasing control");
WB.concurrentGCReleaseControl();
}
}

public static void main(String[] args) throws Exception {
if (!WB.supportsConcurrentGCBreakpoints()) {
throw new RuntimeException("G1 should support GC breakpoints");
}
for (int i = 0; i < 20; i++) {
test();
}
}
}
32 changes: 30 additions & 2 deletions test/jdk/jdk/internal/jimage/ImageReaderTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import tests.JImageGenerator;

import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.nio.file.Path;
import java.util.Arrays;
import java.util.List;
Expand All @@ -58,6 +59,7 @@

/*
* @test
* @bug 8385355
* @summary Tests for ImageReader.
* @modules java.base/jdk.internal.jimage
* jdk.jlink/jdk.tools.jlink.internal
Expand All @@ -72,13 +74,18 @@
/// There is no mutable test instance state to worry about.
@TestInstance(PER_CLASS)
public class ImageReaderTest {
// The '@' prefix marks the entry as a preview entry which will be placed in
// the '/modules/<module>/META-INF/preview/...' namespace.
// The '@' prefix marks the entry as a preview class entry which will be placed in
// the '/modules/<module>/META-INF/preview/...' namespace. The '!' prefix marks
// the entry as a non-class resource path.
private static final Map<String, List<String>> IMAGE_ENTRIES = Map.of(
"modfoo", Arrays.asList(
"com.foo.HasPreviewVersion",
"com.foo.NormalFoo",
"com.foo.bar.NormalBar",
"!META-INF/maven/com.google.code.findbugs/jsr305/pom.properties",
"!META-INF/z",
"!META-INF/collision/child.properties",
"!META-INF/collision",
// Replaces original class in preview mode.
"@com.foo.HasPreviewVersion",
// New class in existing package in preview mode.
Expand Down Expand Up @@ -138,6 +145,23 @@ public void testModuleResources() throws IOException {
}
}

@Test
public void testMetaInfResourcesAreNotPackagePaths() throws IOException {
for (PreviewMode mode : List.of(PreviewMode.ENABLED, PreviewMode.DISABLED)) {
try (ImageReader reader = ImageReader.open(image, mode)) {
assertResource(reader, "modfoo", "META-INF/maven/com.google.code.findbugs/jsr305/pom.properties");
assertResource(reader, "modfoo", "META-INF/z");
assertResource(reader, "modfoo", "META-INF/collision/child.properties");
assertDirContents(reader, "/modules/modfoo/META-INF", "MANIFEST.MF", "collision", "maven", "z");
assertDirContents(reader, "/modules/modfoo/META-INF/collision", "child.properties");
assertDirContents(reader, "/modules/modfoo/META-INF/maven", "com.google.code.findbugs");
assertDirContents(reader, "/modules/modfoo/META-INF/maven/com.google.code.findbugs", "jsr305");
assertDirContents(reader, "/modules/modfoo/META-INF/maven/com.google.code.findbugs/jsr305", "pom.properties");
assertAbsent(reader, "/modules/modfoo/META-INF/maven/com/google/code/findbugs/jsr305/pom.properties");
}
}
}

@ParameterizedTest
@CsvSource(delimiter = ':', value = {
"modfoo:com/foo/HasPreviewVersion.class",
Expand Down Expand Up @@ -416,6 +440,10 @@ public static Path buildJImage(Map<String, List<String>> entries) {
jar.addEntry("module-info.class", InMemoryJavaCompiler.compile("module-info", moduleInfo));

classes.forEach(fqn -> {
if (fqn.startsWith("!")) {
jar.addEntry(fqn.substring(1), "resource".getBytes(StandardCharsets.UTF_8));
return;
}
boolean isPreviewEntry = fqn.startsWith("@");
if (isPreviewEntry) {
fqn = fqn.substring(1);
Expand Down
Loading