From 97cdfb92f8ac195b9c8a872a34ffa8b19fd74860 Mon Sep 17 00:00:00 2001 From: Tobias Hartmann Date: Mon, 29 Jun 2020 08:18:23 +0200 Subject: [PATCH 01/13] 8247832: [Graal] Many Javafuzzer tests failures with Graal, due to unexpected results, after last update JDK-8243380 Cherry-picking GR-24281 Reviewed-by: roland, kvn --- .../core/test/LateMembarInsertionTest.java | 46 -------------- ...tileReadEliminateWrongMemoryStateTest.java | 61 +++++++++++++++++++ .../nodes/memory/VolatileReadNode.java | 17 +----- .../DefaultJavaLoweringProvider.java | 4 +- 4 files changed, 66 insertions(+), 62 deletions(-) create mode 100644 src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.core.test/src/org/graalvm/compiler/core/test/VolatileReadEliminateWrongMemoryStateTest.java diff --git a/src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.core.test/src/org/graalvm/compiler/core/test/LateMembarInsertionTest.java b/src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.core.test/src/org/graalvm/compiler/core/test/LateMembarInsertionTest.java index 591de0a36cf..c1d361b2395 100644 --- a/src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.core.test/src/org/graalvm/compiler/core/test/LateMembarInsertionTest.java +++ b/src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.core.test/src/org/graalvm/compiler/core/test/LateMembarInsertionTest.java @@ -195,45 +195,6 @@ public class LateMembarInsertionTest extends GraalCompilerTest { verifyMembars("volatileFieldStore", membarsExpected()); } - // Unused field load should be optimized out and leave no barrier behind - @SuppressWarnings("unused") - public static void volatileFieldStoreUnusedVolatileFieldLoadVolatileFieldStore(int v2) { - VolatileAccess2.field = v2; - int v1 = VolatileAccess.field; - VolatileAccess2.field = v2; - } - - @Test - public void test09() { - StructuredGraph graph = getFinalGraph(getResolvedJavaMethod("volatileFieldStoreUnusedVolatileFieldLoadVolatileFieldStore")); - List accesses = getAccesses(graph); - - Assert.assertEquals(accesses.size(), 2); - Assert.assertEquals(accesses.get(0).getType(), volatileAccess2Type); - Assert.assertEquals(accesses.get(1).getType(), volatileAccess2Type); - Assert.assertTrue(accesses.get(0).isWrite()); - Assert.assertTrue(accesses.get(1).isWrite()); - Assert.assertEquals(membarsExpected() ? 4 : 0, getMembars(graph).size()); - } - - // Unused field load should be optimized out and leave no barrier behind - @SuppressWarnings("unused") - public static void unusedVolatileFieldLoadVolatileFieldStore(int v2) { - int v1 = VolatileAccess.field; - VolatileAccess2.field = v2; - } - - @Test - public void test10() { - StructuredGraph graph = getFinalGraph(getResolvedJavaMethod("unusedVolatileFieldLoadVolatileFieldStore")); - List accesses = getAccesses(graph); - - Assert.assertEquals(accesses.size(), 1); - Assert.assertEquals(accesses.get(0).getType(), volatileAccess2Type); - Assert.assertTrue(accesses.get(0).isWrite()); - Assert.assertEquals(membarsExpected() ? 2 : 0, getMembars(graph).size()); - } - public static int unsafeVolatileFieldLoad(Object o, long offset) { return UNSAFE.getIntVolatile(o, offset); } @@ -346,11 +307,4 @@ public class LateMembarInsertionTest extends GraalCompilerTest { return javaType; } - private static List getMembars(StructuredGraph graph) { - StructuredGraph.ScheduleResult schedule = graph.getLastSchedule(); - ControlFlowGraph cfg = schedule.getCFG(); - Block[] blocks = cfg.getBlocks(); - - return Arrays.stream(blocks).flatMap(b -> schedule.nodesFor(b).stream()).filter(n -> n instanceof MembarNode).collect(Collectors.toList()); - } } diff --git a/src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.core.test/src/org/graalvm/compiler/core/test/VolatileReadEliminateWrongMemoryStateTest.java b/src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.core.test/src/org/graalvm/compiler/core/test/VolatileReadEliminateWrongMemoryStateTest.java new file mode 100644 index 00000000000..6e9ed0edb28 --- /dev/null +++ b/src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.core.test/src/org/graalvm/compiler/core/test/VolatileReadEliminateWrongMemoryStateTest.java @@ -0,0 +1,61 @@ +/* + * Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2020, Red Hat Inc. 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 org.graalvm.compiler.core.test; + +import org.junit.Test; + +// See https://bugs.openjdk.java.net/browse/JDK-8247832 +public class VolatileReadEliminateWrongMemoryStateTest extends GraalCompilerTest { + + private static volatile int volatileField; + private static int field; + + @SuppressWarnings("unused") + public static int testMethod() { + field = 0; + int v = volatileField; + field += 1; + v = volatileField; + field += 1; + return field; + } + + @Test + public void test1() { + test("testMethod"); + } + + public static void testMethod2(Object obj) { + synchronized (obj) { + volatileField++; + } + } + + @Test + public void test2() { + test("testMethod2", new Object()); + } +} diff --git a/src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/memory/VolatileReadNode.java b/src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/memory/VolatileReadNode.java index e42e59f7e1e..8e520804a86 100644 --- a/src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/memory/VolatileReadNode.java +++ b/src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/memory/VolatileReadNode.java @@ -33,8 +33,6 @@ import static org.graalvm.compiler.nodeinfo.NodeSize.SIZE_1; import org.graalvm.compiler.core.common.LIRKind; import org.graalvm.compiler.core.common.type.Stamp; import org.graalvm.compiler.graph.NodeClass; -import org.graalvm.compiler.graph.spi.Simplifiable; -import org.graalvm.compiler.graph.spi.SimplifierTool; import org.graalvm.compiler.nodeinfo.NodeInfo; import org.graalvm.compiler.nodes.NodeView; import org.graalvm.compiler.nodes.memory.address.AddressNode; @@ -43,20 +41,11 @@ import org.graalvm.compiler.nodes.spi.NodeLIRBuilderTool; import jdk.internal.vm.compiler.word.LocationIdentity; @NodeInfo(nameTemplate = "VolatileRead#{p#location/s}", allowedUsageTypes = Memory, cycles = CYCLES_2, size = SIZE_1) -public class VolatileReadNode extends ReadNode implements SingleMemoryKill, Lowerable, Simplifiable { +public class VolatileReadNode extends ReadNode implements SingleMemoryKill, Lowerable { public static final NodeClass TYPE = NodeClass.create(VolatileReadNode.class); - public VolatileReadNode(AddressNode address, LocationIdentity location, Stamp stamp, BarrierType barrierType) { - super(TYPE, address, location, stamp, null, barrierType, false, null); - } - - @Override - public void simplify(SimplifierTool tool) { - if (lastLocationAccess != null && hasOnlyUsagesOfType(Memory)) { - replaceAtUsages(lastLocationAccess.asNode(), Memory); - assert hasNoUsages(); - graph().removeFixed(this); - } + public VolatileReadNode(AddressNode address, Stamp stamp, BarrierType barrierType) { + super(TYPE, address, LocationIdentity.any(), stamp, null, barrierType, false, null); } @Override diff --git a/src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.replacements/src/org/graalvm/compiler/replacements/DefaultJavaLoweringProvider.java b/src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.replacements/src/org/graalvm/compiler/replacements/DefaultJavaLoweringProvider.java index b4c784ca4d2..c2b56e0fc07 100644 --- a/src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.replacements/src/org/graalvm/compiler/replacements/DefaultJavaLoweringProvider.java +++ b/src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.replacements/src/org/graalvm/compiler/replacements/DefaultJavaLoweringProvider.java @@ -475,7 +475,7 @@ public abstract class DefaultJavaLoweringProvider implements LoweringProvider { ReadNode memoryRead = null; BarrierType barrierType = barrierSet.fieldLoadBarrierType(field, getStorageKind(field)); if (loadField.isVolatile()) { - memoryRead = graph.add(new VolatileReadNode(address, fieldLocationIdentity(field), loadStamp, barrierType)); + memoryRead = graph.add(new VolatileReadNode(address, loadStamp, barrierType)); } else { memoryRead = graph.add(new ReadNode(address, fieldLocationIdentity(field), loadStamp, barrierType)); } @@ -767,7 +767,7 @@ public abstract class DefaultJavaLoweringProvider implements LoweringProvider { AddressNode address = createUnsafeAddress(graph, load.object(), load.offset()); ReadNode memoryRead = null; if (load.isVolatile()) { - memoryRead = new VolatileReadNode(address, load.getLocationIdentity(), loadStamp, barrierSet.readBarrierType(load)); + memoryRead = new VolatileReadNode(address, loadStamp, barrierSet.readBarrierType(load)); } else { memoryRead = new ReadNode(address, load.getLocationIdentity(), loadStamp, barrierSet.readBarrierType(load)); } From d16ea55b627ffbce82044024f791b1a8f1db7f80 Mon Sep 17 00:00:00 2001 From: Bob Vandette Date: Mon, 29 Jun 2020 15:25:16 +0000 Subject: [PATCH 02/13] 8236647: Correct Fix for 8236647: java/lang/invoke/CallSiteTest.java failed with InvocationTargetException in Graal mode Reviewed-by: kvn, never --- .../jdk/vm/ci/hotspot/HotSpotObjectConstantImpl.java | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/HotSpotObjectConstantImpl.java b/src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/HotSpotObjectConstantImpl.java index b1139f9e42f..4c6c647f264 100644 --- a/src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/HotSpotObjectConstantImpl.java +++ b/src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/HotSpotObjectConstantImpl.java @@ -65,25 +65,21 @@ abstract class HotSpotObjectConstantImpl implements HotSpotObjectConstant { @Override public abstract int getIdentityHashCode(); - static class Fields { - // Initializing these too early causes a hang, so do it here in a subclass - static final HotSpotResolvedJavaField callSiteTargetField = HotSpotMethodHandleAccessProvider.Internals.instance().callSiteTargetField; - static final HotSpotResolvedJavaField constantCallSiteFrozenField = HotSpotMethodHandleAccessProvider.Internals.instance().constantCallSiteFrozenField; - } - private boolean isFullyInitializedConstantCallSite() { if (!runtime().getConstantCallSite().isInstance(this)) { return false; } // read ConstantCallSite.isFrozen as a volatile field - boolean isFrozen = readFieldValue(Fields.constantCallSiteFrozenField, true /* volatile */).asBoolean(); + HotSpotResolvedJavaField field = HotSpotMethodHandleAccessProvider.Internals.instance().constantCallSiteFrozenField; + boolean isFrozen = readFieldValue(field, true /* volatile */).asBoolean(); // isFrozen true implies fully-initialized return isFrozen; } private HotSpotObjectConstantImpl readTarget() { // read CallSite.target as a volatile field - return (HotSpotObjectConstantImpl) readFieldValue(Fields.callSiteTargetField, true /* volatile */); + HotSpotResolvedJavaField field = HotSpotMethodHandleAccessProvider.Internals.instance().callSiteTargetField; + return (HotSpotObjectConstantImpl) readFieldValue(field, true /* volatile */); } @Override From 1a4f31409aed773cd41cd4821fb94f496a4a5867 Mon Sep 17 00:00:00 2001 From: Alexey Semenyuk Date: Mon, 29 Jun 2020 10:51:39 -0400 Subject: [PATCH 03/13] 8248427: jpackage jtreg BasicTest.testTemp() test fails on Windows Reviewed-by: herrick, almatvee --- .../classes/jdk/incubator/jpackage/internal/WinMsiBundler.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/jdk.incubator.jpackage/windows/classes/jdk/incubator/jpackage/internal/WinMsiBundler.java b/src/jdk.incubator.jpackage/windows/classes/jdk/incubator/jpackage/internal/WinMsiBundler.java index f7a8ab72b89..774bb758b08 100644 --- a/src/jdk.incubator.jpackage/windows/classes/jdk/incubator/jpackage/internal/WinMsiBundler.java +++ b/src/jdk.incubator.jpackage/windows/classes/jdk/incubator/jpackage/internal/WinMsiBundler.java @@ -316,6 +316,7 @@ public class WinMsiBundler extends AbstractBundler { .launchersDirectory() .resolve(APP_NAME.fetchFrom(params) + ".exe"); } + installerIcon = installerIcon.toAbsolutePath(); params.put(WIN_APP_IMAGE.getID(), appDir); From d180fb30447283cd131940e2e7be7fba8399d4c8 Mon Sep 17 00:00:00 2001 From: Alexey Semenyuk Date: Mon, 29 Jun 2020 10:52:24 -0400 Subject: [PATCH 04/13] 8248254: jpackage fails if app module is in external runtime Reviewed-by: herrick, almatvee --- .../jpackage/internal/LauncherData.java | 157 ++++++++++++++--- test/jdk/ProblemList.txt | 11 +- .../jdk/jpackage/test/JPackageCommand.java | 2 +- .../jdk/jpackage/tests/ModulePathTest3.java | 161 ++++++++++++++++++ 4 files changed, 302 insertions(+), 29 deletions(-) create mode 100644 test/jdk/tools/jpackage/share/jdk/jpackage/tests/ModulePathTest3.java diff --git a/src/jdk.incubator.jpackage/share/classes/jdk/incubator/jpackage/internal/LauncherData.java b/src/jdk.incubator.jpackage/share/classes/jdk/incubator/jpackage/internal/LauncherData.java index 87268b0cb9c..752509444a9 100644 --- a/src/jdk.incubator.jpackage/share/classes/jdk/incubator/jpackage/internal/LauncherData.java +++ b/src/jdk.incubator.jpackage/share/classes/jdk/incubator/jpackage/internal/LauncherData.java @@ -26,25 +26,34 @@ package jdk.incubator.jpackage.internal; import java.io.File; import java.io.IOException; +import java.io.Reader; import java.lang.module.ModuleDescriptor; +import java.lang.module.ModuleReference; import java.nio.file.Files; import java.nio.file.InvalidPathException; import java.nio.file.Path; import java.text.MessageFormat; -import java.util.*; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Optional; +import java.util.Properties; +import java.util.Set; import java.util.function.Supplier; import java.util.jar.Attributes; import java.util.jar.JarFile; import java.util.jar.Manifest; import java.util.stream.Collectors; import java.util.stream.Stream; +import static jdk.incubator.jpackage.internal.StandardBundlerParam.PREDEFINED_RUNTIME_IMAGE; /** * Extracts data needed to run application from parameters. */ final class LauncherData { boolean isModular() { - return moduleDescriptor != null; + return moduleInfo != null; } String qualifiedClassName() { @@ -61,7 +70,7 @@ final class LauncherData { String moduleName() { verifyIsModular(true); - return moduleDescriptor.name(); + return moduleInfo.name; } List modulePath() { @@ -80,11 +89,7 @@ final class LauncherData { String getAppVersion() { if (isModular()) { - ModuleDescriptor.Version ver = moduleDescriptor.version().orElse(null); - if (ver != null) { - return ver.toString(); - } - return moduleDescriptor.rawVersion().orElse(null); + return moduleInfo.version; } return null; @@ -94,7 +99,7 @@ final class LauncherData { } private void verifyIsModular(boolean isModular) { - if ((moduleDescriptor != null) != isModular) { + if ((moduleInfo != null) != isModular) { throw new IllegalStateException(); } } @@ -103,9 +108,19 @@ final class LauncherData { ConfigException, IOException { final String mainModule = getMainModule(params); + final LauncherData result; if (mainModule == null) { - return createNonModular(params); + result = createNonModular(params); + } else { + result = createModular(mainModule, params); } + result.initClasspath(params); + return result; + } + + private static LauncherData createModular(String mainModule, + Map params) throws ConfigException, + IOException { LauncherData launcherData = new LauncherData(); @@ -119,18 +134,34 @@ final class LauncherData { } launcherData.modulePath = getModulePath(params); - launcherData.moduleDescriptor = JLinkBundlerHelper.createModuleFinder( - launcherData.modulePath).find(moduleName).orElseThrow( - () -> new ConfigException(MessageFormat.format(I18N.getString( - "error.no-module-in-path"), moduleName), null)).descriptor(); + // Try to find module in the specified module path list. + ModuleReference moduleRef = JLinkBundlerHelper.createModuleFinder( + launcherData.modulePath).find(moduleName).orElse(null); - if (launcherData.qualifiedClassName == null) { - launcherData.qualifiedClassName = launcherData.moduleDescriptor.mainClass().orElseThrow( - () -> new ConfigException(I18N.getString("ERR_NoMainClass"), - null)); + if (moduleRef != null) { + launcherData.moduleInfo = ModuleInfo.fromModuleDescriptor( + moduleRef.descriptor()); + } else if (params.containsKey(PREDEFINED_RUNTIME_IMAGE.getID())) { + // Failed to find module in the specified module path list and + // there is external runtime given to jpackage. + // Lookup module in this runtime. + Path cookedRuntime = PREDEFINED_RUNTIME_IMAGE.fetchFrom(params).toPath(); + launcherData.moduleInfo = ModuleInfo.fromCookedRuntime(moduleName, + cookedRuntime); + } + + if (launcherData.moduleInfo == null) { + throw new ConfigException(MessageFormat.format(I18N.getString( + "error.no-module-in-path"), moduleName), null); + } + + if (launcherData.qualifiedClassName == null) { + launcherData.qualifiedClassName = launcherData.moduleInfo.mainClass; + if (launcherData.qualifiedClassName == null) { + throw new ConfigException(I18N.getString("ERR_NoMainClass"), null); + } } - launcherData.initClasspath(params); return launcherData; } @@ -191,7 +222,6 @@ final class LauncherData { launcherData.mainJarName)); } - launcherData.initClasspath(params); return launcherData; } @@ -259,8 +289,17 @@ final class LauncherData { private static List getModulePath(Map params) throws ConfigException { - return getPathListParameter(Arguments.CLIOptions.MODULE_PATH.getId(), - params); + List modulePath = getPathListParameter(Arguments.CLIOptions.MODULE_PATH.getId(), params); + + if (params.containsKey(PREDEFINED_RUNTIME_IMAGE.getID())) { + Path runtimePath = PREDEFINED_RUNTIME_IMAGE.fetchFrom(params).toPath(); + runtimePath = runtimePath.resolve("lib"); + modulePath = Stream.of(modulePath, List.of(runtimePath)) + .flatMap(List::stream) + .collect(Collectors.toUnmodifiableList()); + } + + return modulePath; } private static List getPathListParameter(String paramName, @@ -277,5 +316,77 @@ final class LauncherData { private Path mainJarName; private List classPath; private List modulePath; - private ModuleDescriptor moduleDescriptor; + private ModuleInfo moduleInfo; + + private static final class ModuleInfo { + String name; + String version; + String mainClass; + + static ModuleInfo fromModuleDescriptor(ModuleDescriptor md) { + ModuleInfo result = new ModuleInfo(); + result.name = md.name(); + result.mainClass = md.mainClass().orElse(null); + + ModuleDescriptor.Version ver = md.version().orElse(null); + if (ver != null) { + result.version = ver.toString(); + } else { + result.version = md.rawVersion().orElse(null); + } + + return result; + } + + static ModuleInfo fromCookedRuntime(String moduleName, + Path cookedRuntime) { + Objects.requireNonNull(moduleName); + + // We can't extract info about version and main class of a module + // linked in external runtime without running ModuleFinder in that + // runtime. But this is too much work as the runtime might have been + // coocked without native launchers. So just make sure the module + // is linked in the runtime by simply analysing the data + // of `release` file. + + final Path releaseFile; + if (!Platform.isMac()) { + releaseFile = cookedRuntime.resolve("release"); + } else { + // On Mac `cookedRuntime` can be runtime root or runtime home. + Path runtimeHome = cookedRuntime.resolve("Contents/Home"); + if (!Files.isDirectory(runtimeHome)) { + runtimeHome = cookedRuntime; + } + releaseFile = runtimeHome.resolve("release"); + } + + try (Reader reader = Files.newBufferedReader(releaseFile)) { + Properties props = new Properties(); + props.load(reader); + String moduleList = props.getProperty("MODULES"); + if (moduleList == null) { + return null; + } + + if ((moduleList.startsWith("\"") && moduleList.endsWith("\"")) + || (moduleList.startsWith("\'") && moduleList.endsWith( + "\'"))) { + moduleList = moduleList.substring(1, moduleList.length() - 1); + } + + if (!List.of(moduleList.split("\\s+")).contains(moduleName)) { + return null; + } + } catch (IOException|IllegalArgumentException ex) { + Log.verbose(ex); + return null; + } + + ModuleInfo result = new ModuleInfo(); + result.name = moduleName; + + return result; + } + } } diff --git a/test/jdk/ProblemList.txt b/test/jdk/ProblemList.txt index 34bf7313661..d1dc3124aaa 100644 --- a/test/jdk/ProblemList.txt +++ b/test/jdk/ProblemList.txt @@ -924,10 +924,11 @@ jdk/jfr/event/os/TestThreadContextSwitches.java 8247776 windows- # jdk_jpackage -tools/jpackage/share/EmptyFolderPackageTest.java 8248059 macosx-all -tools/jpackage/share/IconTest.java 8248059 macosx-all -tools/jpackage/share/AppImagePackageTest.java 8248059 macosx-all -tools/jpackage/share/SimplePackageTest.java 8248059 macosx-all -tools/jpackage/share/jdk/jpackage/tests/BasicTest.java 8248059 macosx-all +tools/jpackage/share/EmptyFolderPackageTest.java 8248059 macosx-all +tools/jpackage/share/IconTest.java 8248059 macosx-all +tools/jpackage/share/AppImagePackageTest.java 8248059 macosx-all +tools/jpackage/share/SimplePackageTest.java 8248059 macosx-all +tools/jpackage/share/jdk/jpackage/tests/BasicTest.java 8248059 macosx-all +tools/jpackage/share/jdk/jpackage/tests/ModulePathTest3.java#id0 8248418 generic-all ############################################################################ diff --git a/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/JPackageCommand.java b/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/JPackageCommand.java index 58407fd8279..d404fb7a7e1 100644 --- a/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/JPackageCommand.java +++ b/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/JPackageCommand.java @@ -300,7 +300,7 @@ public final class JPackageCommand extends CommandArguments { public static JPackageCommand helloAppImage(JavaAppDesc javaAppDesc) { JPackageCommand cmd = new JPackageCommand(); cmd.setDefaultInputOutput().setDefaultAppName(); - PackageType.IMAGE.applyTo(cmd); + cmd.setPackageType(PackageType.IMAGE); new HelloApp(javaAppDesc).addTo(cmd); return cmd; } diff --git a/test/jdk/tools/jpackage/share/jdk/jpackage/tests/ModulePathTest3.java b/test/jdk/tools/jpackage/share/jdk/jpackage/tests/ModulePathTest3.java new file mode 100644 index 00000000000..d916fe5c7de --- /dev/null +++ b/test/jdk/tools/jpackage/share/jdk/jpackage/tests/ModulePathTest3.java @@ -0,0 +1,161 @@ +/* + * Copyright (c) 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 + * 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 jdk.jpackage.tests; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; +import javax.xml.xpath.XPathExpressionException; +import javax.xml.xpath.XPathConstants; +import javax.xml.xpath.XPathFactory; +import jdk.incubator.jpackage.internal.AppImageFile; +import jdk.jpackage.test.HelloApp; +import jdk.jpackage.test.JavaAppDesc; +import jdk.jpackage.test.Annotations.Test; +import jdk.jpackage.test.Annotations.Parameter; +import jdk.jpackage.test.Annotations.Parameters; +import jdk.jpackage.test.Executor; +import jdk.jpackage.test.JPackageCommand; +import jdk.jpackage.test.JavaTool; +import jdk.jpackage.test.PackageType; +import jdk.jpackage.test.TKit; +import org.w3c.dom.Document; + + +/* + * @test + * @summary jpackage for app's module linked in external runtime + * @library ../../../../helpers + * @build jdk.jpackage.test.* + * @modules jdk.incubator.jpackage/jdk.incubator.jpackage.internal + * @compile ModulePathTest3.java + * @run main/othervm/timeout=360 -Xmx512m jdk.jpackage.test.Main + * --jpt-run=jdk.jpackage.tests.ModulePathTest3 + */ + +/* + * @test + * @summary jpackage for app's module linked in external runtime + * @library ../../../../helpers + * @build jdk.jpackage.test.* + * @modules jdk.incubator.jpackage/jdk.incubator.jpackage.internal + * @compile ModulePathTest3.java + * @run main/othervm/timeout=360 -Xmx512m jdk.jpackage.test.Main + * --jpt-run=jdk.jpackage.tests.ModulePathTest3 + * --jpt-exclude=test8248418 + */ + +public final class ModulePathTest3 { + + public ModulePathTest3(String jlinkOutputSubdir, String runtimeSubdir) { + this.jlinkOutputSubdir = Path.of(jlinkOutputSubdir); + this.runtimeSubdir = Path.of(runtimeSubdir); + } + + /** + * Test case for JDK-8248254. + * App's module in runtime directory. + */ + @Test + public void test8248254() throws XPathExpressionException, IOException { + testIt("me.mymodule/me.mymodule.Main"); + } + + /** + * Test case for JDK-8248418. + * App's module with version specified in runtime directory. + */ + @Test + public void test8248418() throws XPathExpressionException, IOException { + testIt("me.mymodule/me.mymodule.Main@3.7"); + } + + private void testIt(String mainAppDesc) throws XPathExpressionException, + IOException { + final JavaAppDesc appDesc = JavaAppDesc.parse(mainAppDesc); + final Path moduleOutputDir = TKit.createTempDirectory("modules"); + HelloApp.createBundle(appDesc, moduleOutputDir); + + final Path workDir = TKit.createTempDirectory("runtime").resolve("data"); + final Path jlinkOutputDir = workDir.resolve(jlinkOutputSubdir); + Files.createDirectories(jlinkOutputDir.getParent()); + + new Executor() + .setToolProvider(JavaTool.JLINK) + .dumpOutput() + .addArguments( + "--add-modules", appDesc.moduleName(), + "--output", jlinkOutputDir.toString(), + "--module-path", moduleOutputDir.resolve(appDesc.jarFileName()).toString(), + "--strip-debug", + "--no-header-files", + "--no-man-pages", + "--strip-native-commands") + .execute(); + + JPackageCommand cmd = new JPackageCommand() + .setDefaultAppName() + .setPackageType(PackageType.IMAGE) + .setDefaultInputOutput() + .removeArgumentWithValue("--input") + .addArguments("--module", appDesc.moduleName() + "/" + appDesc.className()) + .setArgumentValue("--runtime-image", workDir.resolve(runtimeSubdir)); + + cmd.executeAndAssertHelloAppImageCreated(); + + if (appDesc.moduleVersion() != null) { + Document xml = AppImageFile.readXml(cmd.outputBundle()); + String actualVersion = XPathFactory.newInstance().newXPath().evaluate( + "/jpackage-state/app-version/text()", xml, + XPathConstants.STRING).toString(); + + TKit.assertEquals(appDesc.moduleVersion(), actualVersion, + "Check application version"); + } + } + + @Parameters + public static Collection data() { + final List paths = new ArrayList<>(); + paths.add(new String[] { "", "" }); + if (TKit.isOSX()) { + // On OSX jpackage should accept both runtime root and runtime home + // directories. + paths.add(new String[] { "Contents/Home", "" }); + } + + List data = new ArrayList<>(); + for (var pathCfg : paths) { + data.add(new Object[] { pathCfg[0], pathCfg[1] }); + } + + return data; + } + + private final Path jlinkOutputSubdir; + private final Path runtimeSubdir; +} From 320af9b34b2cba78a5b5b262719b8d1a727a3f82 Mon Sep 17 00:00:00 2001 From: Alexey Semenyuk Date: Mon, 29 Jun 2020 10:52:24 -0400 Subject: [PATCH 05/13] 8248264: WinUpgradeUUIDTest application is missing in downgrade scenario Reviewed-by: herrick, almatvee --- .../jpackage/internal/WinMsiBundler.java | 2 + .../jpackage/internal/resources/main.wxs | 69 ++++++++++--------- 2 files changed, 37 insertions(+), 34 deletions(-) diff --git a/src/jdk.incubator.jpackage/windows/classes/jdk/incubator/jpackage/internal/WinMsiBundler.java b/src/jdk.incubator.jpackage/windows/classes/jdk/incubator/jpackage/internal/WinMsiBundler.java index 774bb758b08..f7667ad991e 100644 --- a/src/jdk.incubator.jpackage/windows/classes/jdk/incubator/jpackage/internal/WinMsiBundler.java +++ b/src/jdk.incubator.jpackage/windows/classes/jdk/incubator/jpackage/internal/WinMsiBundler.java @@ -464,6 +464,8 @@ public class WinMsiBundler extends AbstractBundler { boolean enableLicenseUI = (LICENSE_FILE.fetchFrom(params) != null); boolean enableInstalldirUI = INSTALLDIR_CHOOSER.fetchFrom(params); + wixPipeline.addLightOptions("-sice:ICE27"); + if (!MSI_SYSTEM_WIDE.fetchFrom(params)) { wixPipeline.addLightOptions("-sice:ICE91"); } diff --git a/src/jdk.incubator.jpackage/windows/classes/jdk/incubator/jpackage/internal/resources/main.wxs b/src/jdk.incubator.jpackage/windows/classes/jdk/incubator/jpackage/internal/resources/main.wxs index 6b0f9b09f9b..b7b65d7b61b 100644 --- a/src/jdk.incubator.jpackage/windows/classes/jdk/incubator/jpackage/internal/resources/main.wxs +++ b/src/jdk.incubator.jpackage/windows/classes/jdk/incubator/jpackage/internal/resources/main.wxs @@ -12,6 +12,17 @@ + + + + + + + + + + + - - - - - + + + + - - + - - - - - - - - - - - + @@ -142,6 +135,14 @@ Not Installed + + JP_UPGRADABLE_FOUND + + + JP_DOWNGRADABLE_FOUND + + + From d19f2bdec01eb324d200f2144293d05ea310f9b8 Mon Sep 17 00:00:00 2001 From: Patric Hedlin Date: Mon, 29 Jun 2020 19:33:35 +0200 Subject: [PATCH 06/13] 8234605: C2 failed "assert(C->live_nodes() - live_at_begin <= 2 * _nodes_required) failed: Bad node estimate: actual = 208 >> request = 101" Reviewed-by: neliasso, kvn --- src/hotspot/share/opto/loopnode.hpp | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/hotspot/share/opto/loopnode.hpp b/src/hotspot/share/opto/loopnode.hpp index 4be52881c56..a7333ea0982 100644 --- a/src/hotspot/share/opto/loopnode.hpp +++ b/src/hotspot/share/opto/loopnode.hpp @@ -1405,14 +1405,17 @@ private: void require_nodes_final(uint live_at_begin, bool check_estimate) { assert(_nodes_required < UINT_MAX, "Bad state (final)."); +#ifdef ASSERT if (check_estimate) { - // Assert that the node budget request was not off by too much (x2). + // Check that the node budget request was not off by too much (x2). // Should this be the case we _surely_ need to improve the estimates // used in our budget calculations. - assert(C->live_nodes() - live_at_begin <= 2 * _nodes_required, - "Bad node estimate: actual = %d >> request = %d", - C->live_nodes() - live_at_begin, _nodes_required); + if (C->live_nodes() - live_at_begin > 2 * _nodes_required) { + log_info(compilation)("Bad node estimate: actual = %d >> request = %d", + C->live_nodes() - live_at_begin, _nodes_required); + } } +#endif // Assert that we have stayed within the node budget limit. assert(C->live_nodes() < C->max_node_limit(), "Exceeding node budget limit: %d + %d > %d (request = %d)", From 144267d30fc0a64688cc37d2c93af10fd0241860 Mon Sep 17 00:00:00 2001 From: Chris Plummer Date: Mon, 29 Jun 2020 14:22:01 -0700 Subject: [PATCH 07/13] 7107012: sun.jvm.hostspot.code.CompressedReadStream readDouble() conversion to long mishandled Reviewed-by: sspitsyn, dcubed --- .../classes/sun/jvm/hotspot/code/CompressedReadStream.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/code/CompressedReadStream.java b/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/code/CompressedReadStream.java index 671bd4362db..49f1d2b3124 100644 --- a/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/code/CompressedReadStream.java +++ b/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/code/CompressedReadStream.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2000, 2005, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2000, 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 @@ -75,7 +75,7 @@ public class CompressedReadStream extends CompressedStream { int rl = readInt(); int h = reverseInt(rh); int l = reverseInt(rl); - return Double.longBitsToDouble((h << 32) | ((long)l & 0x00000000FFFFFFFFL)); + return Double.longBitsToDouble(((long)h << 32) | ((long)l & 0x00000000FFFFFFFFL)); } public long readLong() { From a7e352b554078a11195b434ea9a601210b5b33cd Mon Sep 17 00:00:00 2001 From: Zhuo Wang Date: Mon, 29 Jun 2020 10:15:45 -0400 Subject: [PATCH 08/13] 8246051: SIGBUS by unaligned Unsafe compare_and_swap Reviewed-by: aph --- src/hotspot/share/prims/unsafe.cpp | 2 + .../unsafe/TestUnsafeUnalignedSwap.java | 80 +++++++++++++++++++ 2 files changed, 82 insertions(+) create mode 100644 test/hotspot/jtreg/compiler/unsafe/TestUnsafeUnalignedSwap.java diff --git a/src/hotspot/share/prims/unsafe.cpp b/src/hotspot/share/prims/unsafe.cpp index 003b73e6785..72d81ec9d6c 100644 --- a/src/hotspot/share/prims/unsafe.cpp +++ b/src/hotspot/share/prims/unsafe.cpp @@ -950,6 +950,7 @@ UNSAFE_ENTRY(jboolean, Unsafe_CompareAndSetReference(JNIEnv *env, jobject unsafe UNSAFE_ENTRY(jboolean, Unsafe_CompareAndSetInt(JNIEnv *env, jobject unsafe, jobject obj, jlong offset, jint e, jint x)) { oop p = JNIHandles::resolve(obj); + GuardUnsafeAccess guard(thread); if (p == NULL) { volatile jint* addr = (volatile jint*)index_oop_from_field_offset_long(p, offset); return RawAccess<>::atomic_cmpxchg(addr, e, x) == e; @@ -961,6 +962,7 @@ UNSAFE_ENTRY(jboolean, Unsafe_CompareAndSetInt(JNIEnv *env, jobject unsafe, jobj UNSAFE_ENTRY(jboolean, Unsafe_CompareAndSetLong(JNIEnv *env, jobject unsafe, jobject obj, jlong offset, jlong e, jlong x)) { oop p = JNIHandles::resolve(obj); + GuardUnsafeAccess guard(thread); if (p == NULL) { volatile jlong* addr = (volatile jlong*)index_oop_from_field_offset_long(p, offset); return RawAccess<>::atomic_cmpxchg(addr, e, x) == e; diff --git a/test/hotspot/jtreg/compiler/unsafe/TestUnsafeUnalignedSwap.java b/test/hotspot/jtreg/compiler/unsafe/TestUnsafeUnalignedSwap.java new file mode 100644 index 00000000000..1b345894e7c --- /dev/null +++ b/test/hotspot/jtreg/compiler/unsafe/TestUnsafeUnalignedSwap.java @@ -0,0 +1,80 @@ +/* + * Copyright (c) 2020 Alibaba Group Holding Limited. 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. Alibaba designates this + * particular file as subject to the "Classpath" exception as provided + * by Oracle in the LICENSE file that accompanied this code. + * + * 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. + * + */ + +/* @test + * @library / /test/lib + * @bug 8246051 + * @summary A test for SIGBUS in aarch64 by unalgined unsafe access + * @requires os.arch=="aarch64" + * @run main/othervm/timeout=200 -XX:-Inline TestUnsafeUnalignedSwap + */ + +import sun.misc.Unsafe; +import java.lang.reflect.Field; +import java.util.*; +import jdk.test.lib.Asserts; + +public class TestUnsafeUnalignedSwap { + private final static Unsafe U; + private static long sum = 4; + static volatile long[] arrayLong = new long[1001]; + static volatile int[] arrayInt = new int[1001]; + static { + try { + Field f = Unsafe.class.getDeclaredField("theUnsafe"); + f.setAccessible(true); + U = (Unsafe) f.get(null); + } catch (ReflectiveOperationException e) { + throw new InternalError(e); + } + } + // Bug 8246051 : Unsafe.compareAndSwapLong should not crash + public static void testCompareAndSwapLong() { + try { + if (U.compareAndSwapLong(arrayLong, Unsafe.ARRAY_LONG_BASE_OFFSET + 1, 3243, 2334)) { + sum++; + } else { + sum--; + } + } catch (InternalError e) { + System.out.println(e.getMessage()); + } + } + public static void testCompareAndSwapInt() { + try { + if (U.compareAndSwapInt(arrayInt, Unsafe.ARRAY_INT_BASE_OFFSET + 1, 3243, 2334)) { + sum++; + } else { + sum--; + } + } catch (InternalError e) { + System.out.println(e.getMessage()); + } + } + public static void test() { + testCompareAndSwapLong(); + testCompareAndSwapInt(); + } + public static void main(String[] args) { + test(); + } +} From d5ae932b3fb80fb96b808db1cead20adea8dfb00 Mon Sep 17 00:00:00 2001 From: Tobias Hartmann Date: Tue, 30 Jun 2020 15:08:40 +0200 Subject: [PATCH 09/13] 8248265: compiler/ciReplay tests fail with AOT compiled java.base The test should use a non-empty method to trigger compilation. Reviewed-by: kvn, neliasso, iignatyev --- test/hotspot/jtreg/ProblemList-aot.txt | 3 --- test/hotspot/jtreg/compiler/ciReplay/CiReplayBase.java | 7 +++++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/test/hotspot/jtreg/ProblemList-aot.txt b/test/hotspot/jtreg/ProblemList-aot.txt index a2fcf21e18c..e584c647175 100644 --- a/test/hotspot/jtreg/ProblemList-aot.txt +++ b/test/hotspot/jtreg/ProblemList-aot.txt @@ -85,8 +85,5 @@ compiler/intrinsics/sha/sanity/TestSHA256Intrinsics.java 8167430 gene compiler/intrinsics/sha/sanity/TestSHA1MultiBlockIntrinsics.java 8167430 generic-all compiler/intrinsics/sha/sanity/TestSHA512MultiBlockIntrinsics.java 8167430 generic-all -compiler/ciReplay/TestServerVM.java 8248265 generic-all -compiler/ciReplay/TestVMNoCompLevel.java 8248265 generic-all - vmTestbase/vm/mlvm/indy/stress/java/relinkMutableCallSiteFreq/Test.java 8226689 generic-all vmTestbase/vm/mlvm/indy/stress/java/relinkVolatileCallSiteFreq/Test.java 8226689 generic-all diff --git a/test/hotspot/jtreg/compiler/ciReplay/CiReplayBase.java b/test/hotspot/jtreg/compiler/ciReplay/CiReplayBase.java index 818ab4ee5d3..7d39496831a 100644 --- a/test/hotspot/jtreg/compiler/ciReplay/CiReplayBase.java +++ b/test/hotspot/jtreg/compiler/ciReplay/CiReplayBase.java @@ -70,9 +70,12 @@ public abstract class CiReplayBase { private static final String[] REPLAY_OPTIONS = new String[]{DISABLE_COREDUMP_ON_CRASH, "-XX:+ReplayCompiles", REPLAY_FILE_OPTION}; protected final Optional runServer; + private static int dummy; - public static class EmptyMain { + public static class TestMain { public static void main(String[] args) { + // Do something because empty methods might not be called/compiled. + dummy = 42; } } @@ -140,7 +143,7 @@ public abstract class CiReplayBase { options.addAll(Arrays.asList(REPLAY_GENERATION_OPTIONS)); options.addAll(Arrays.asList(vmopts)); options.add(needCoreDump ? ENABLE_COREDUMP_ON_CRASH : DISABLE_COREDUMP_ON_CRASH); - options.add(EmptyMain.class.getName()); + options.add(TestMain.class.getName()); if (needCoreDump) { crashOut = ProcessTools.executeProcess(getTestJvmCommandlineWithPrefix( RUN_SHELL_NO_LIMIT, options.toArray(new String[0]))); From 05dc2af21fb0afffee6c42443f5c960d84df28ac Mon Sep 17 00:00:00 2001 From: Roland Westrelin Date: Thu, 18 Jun 2020 13:51:40 +0200 Subject: [PATCH 10/13] 8247824: CTW: C2 (Shenandoah) compilation fails with SEGV in SBC2Support::pin_and_expand Reviewed-by: rkennke, thartmann --- .../gc/shenandoah/c2/shenandoahSupport.cpp | 10 +-- src/hotspot/share/opto/loopnode.hpp | 8 +++ ...estShenandoahLRBInOuterStripMinedLoop.java | 61 +++++++++++++++++++ 3 files changed, 75 insertions(+), 4 deletions(-) create mode 100644 test/hotspot/jtreg/gc/shenandoah/compiler/TestShenandoahLRBInOuterStripMinedLoop.java diff --git a/src/hotspot/share/gc/shenandoah/c2/shenandoahSupport.cpp b/src/hotspot/share/gc/shenandoah/c2/shenandoahSupport.cpp index fe11e33c069..d7f1cd7bfff 100644 --- a/src/hotspot/share/gc/shenandoah/c2/shenandoahSupport.cpp +++ b/src/hotspot/share/gc/shenandoah/c2/shenandoahSupport.cpp @@ -1083,11 +1083,12 @@ void ShenandoahBarrierC2Support::pin_and_expand(PhaseIdealLoop* phase) { Node* barrier = state->enqueue_barrier(i); Node* ctrl = phase->get_ctrl(barrier); IdealLoopTree* loop = phase->get_loop(ctrl); - if (loop->_head->is_OuterStripMinedLoop()) { + Node* head = loop->head(); + if (head->is_OuterStripMinedLoop()) { // Expanding a barrier here will break loop strip mining // verification. Transform the loop so the loop nest doesn't // appear as strip mined. - OuterStripMinedLoopNode* outer = loop->_head->as_OuterStripMinedLoop(); + OuterStripMinedLoopNode* outer = head->as_OuterStripMinedLoop(); hide_strip_mined_loop(outer, outer->unique_ctrl_out()->as_CountedLoop(), phase); } } @@ -1289,11 +1290,12 @@ void ShenandoahBarrierC2Support::pin_and_expand(PhaseIdealLoop* phase) { ShenandoahLoadReferenceBarrierNode* lrb = state->load_reference_barrier(i); Node* ctrl = phase->get_ctrl(lrb); IdealLoopTree* loop = phase->get_loop(ctrl); - if (loop->_head->is_OuterStripMinedLoop()) { + Node* head = loop->head(); + if (head->is_OuterStripMinedLoop()) { // Expanding a barrier here will break loop strip mining // verification. Transform the loop so the loop nest doesn't // appear as strip mined. - OuterStripMinedLoopNode* outer = loop->_head->as_OuterStripMinedLoop(); + OuterStripMinedLoopNode* outer = head->as_OuterStripMinedLoop(); hide_strip_mined_loop(outer, outer->unique_ctrl_out()->as_CountedLoop(), phase); } } diff --git a/src/hotspot/share/opto/loopnode.hpp b/src/hotspot/share/opto/loopnode.hpp index a7333ea0982..97a97ff8290 100644 --- a/src/hotspot/share/opto/loopnode.hpp +++ b/src/hotspot/share/opto/loopnode.hpp @@ -485,6 +485,7 @@ public: Node *_head; // Head of loop Node *_tail; // Tail of loop inline Node *tail(); // Handle lazy update of _tail field + inline Node *head(); // Handle lazy update of _head field PhaseIdealLoop* _phase; int _local_loop_unroll_limit; int _local_loop_unroll_factor; @@ -1579,6 +1580,13 @@ inline Node* IdealLoopTree::tail() { return _tail; } +inline Node* IdealLoopTree::head() { + // Handle lazy update of _head field. + if (_head->in(0) == NULL) { + _head = _phase->get_ctrl(_head); + } + return _head; +} // Iterate over the loop tree using a preorder, left-to-right traversal. // diff --git a/test/hotspot/jtreg/gc/shenandoah/compiler/TestShenandoahLRBInOuterStripMinedLoop.java b/test/hotspot/jtreg/gc/shenandoah/compiler/TestShenandoahLRBInOuterStripMinedLoop.java new file mode 100644 index 00000000000..774d3cd3e6f --- /dev/null +++ b/test/hotspot/jtreg/gc/shenandoah/compiler/TestShenandoahLRBInOuterStripMinedLoop.java @@ -0,0 +1,61 @@ +/* + * Copyright (c) 2020, Red Hat, Inc. 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. + */ + +/** + * @test + * @bug 8247824 + * @summary CTW: C2 (Shenandoah) compilation fails with SEGV in SBC2Support::pin_and_expand + * @requires vm.flavor == "server" + * @requires vm.gc.Shenandoah & !vm.graal.enabled + * + * @run main/othervm -XX:-BackgroundCompilation -XX:+UseShenandoahGC -XX:LoopMaxUnroll=0 TestShenandoahLRBInOuterStripMinedLoop + * + */ + +import java.util.Arrays; + +public class TestShenandoahLRBInOuterStripMinedLoop { + public static void main(String[] args) { + A[] array = new A[4000]; + Arrays.fill(array, new A()); + for (int i = 0; i < 20_0000; i++) { + test(array); + } + } + + private static int test(A[] array) { + A a = null; + int v = 1; + A b = null; + for (int i = 0; i < 2000; i++) { + a = array[i]; + b = array[2*i]; + v *= 2; + } + return a.f + b.f + v; + } + + private static class A { + public int f; + } +} From eb1bacc71b60ecf717a4549ef46ecd0ed877c75f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20Gr=C3=B6nlund?= Date: Tue, 30 Jun 2020 18:10:44 +0200 Subject: [PATCH 11/13] 8248475: Suppress unconditional warning "JFR will be disabled during CDS dumping" Reviewed-by: redestad --- src/hotspot/share/jfr/recorder/jfrRecorder.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hotspot/share/jfr/recorder/jfrRecorder.cpp b/src/hotspot/share/jfr/recorder/jfrRecorder.cpp index 257c17d2d29..ee6f185dcd0 100644 --- a/src/hotspot/share/jfr/recorder/jfrRecorder.cpp +++ b/src/hotspot/share/jfr/recorder/jfrRecorder.cpp @@ -173,7 +173,7 @@ static void log_jdk_jfr_module_resolution_error(TRAPS) { static bool is_cds_dump_requested() { // we will not be able to launch recordings on startup if a cds dump is being requested - if (Arguments::is_dumping_archive()) { + if (Arguments::is_dumping_archive() && JfrOptionSet::start_flight_recording_options() != NULL) { warning("JFR will be disabled during CDS dumping"); teardown_startup_support(); return true; From abc55dea7eb55ec2258867187d97dd0a14c5934d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20Gr=C3=B6nlund?= Date: Tue, 30 Jun 2020 19:00:14 +0200 Subject: [PATCH 12/13] 8248485: Poor scalability in JfrCheckpointManager when using many threads after JDK-8242008 Reviewed-by: egahlin --- .../checkpoint/jfrCheckpointManager.cpp | 157 ++++++++++-------- .../checkpoint/jfrCheckpointManager.hpp | 16 +- .../checkpoint/jfrCheckpointWriter.cpp | 4 +- .../checkpoint/jfrCheckpointWriter.hpp | 2 +- .../checkpoint/types/jfrTypeManager.cpp | 14 +- .../share/jfr/recorder/storage/jfrBuffer.cpp | 40 +++-- .../share/jfr/recorder/storage/jfrBuffer.hpp | 13 +- .../jfr/recorder/storage/jfrEpochStorage.hpp | 24 ++- .../storage/jfrEpochStorage.inline.hpp | 52 +++--- .../storage/jfrMemorySpace.inline.hpp | 41 ++++- .../jfr/recorder/stringpool/jfrStringPool.cpp | 2 +- .../jfrConcurrentLinkedListHost.inline.hpp | 4 +- 12 files changed, 234 insertions(+), 135 deletions(-) diff --git a/src/hotspot/share/jfr/recorder/checkpoint/jfrCheckpointManager.cpp b/src/hotspot/share/jfr/recorder/checkpoint/jfrCheckpointManager.cpp index 9c606df6c24..3ba27e40386 100644 --- a/src/hotspot/share/jfr/recorder/checkpoint/jfrCheckpointManager.cpp +++ b/src/hotspot/share/jfr/recorder/checkpoint/jfrCheckpointManager.cpp @@ -35,6 +35,7 @@ #include "jfr/recorder/jfrRecorder.hpp" #include "jfr/recorder/repository/jfrChunkWriter.hpp" #include "jfr/recorder/service/jfrOptionSet.hpp" +#include "jfr/recorder/storage/jfrEpochStorage.inline.hpp" #include "jfr/recorder/storage/jfrMemorySpace.inline.hpp" #include "jfr/recorder/storage/jfrStorageUtils.inline.hpp" #include "jfr/support/jfrKlassUnloading.hpp" @@ -91,50 +92,44 @@ void JfrCheckpointManager::destroy() { } JfrCheckpointManager::JfrCheckpointManager(JfrChunkWriter& cw) : - _mspace(NULL), + _global_mspace(NULL), + _thread_local_mspace(NULL), _chunkwriter(cw) {} JfrCheckpointManager::~JfrCheckpointManager() { JfrTraceIdLoadBarrier::destroy(); JfrTypeManager::destroy(); - delete _mspace; + delete _global_mspace; + delete _thread_local_mspace; } -static const size_t buffer_count = 2; -static const size_t buffer_size = 512 * K; +static const size_t global_buffer_prealloc_count = 2; +static const size_t global_buffer_size = 512 * K; -static JfrCheckpointMspace* allocate_mspace(size_t min_elem_size, - size_t free_list_cache_count_limit, - size_t cache_prealloc_count, - bool prealloc_to_free_list, - JfrCheckpointManager* mgr) { - return create_mspace(min_elem_size, - free_list_cache_count_limit, - cache_prealloc_count, - prealloc_to_free_list, - mgr); -} +static const size_t thread_local_buffer_prealloc_count = 16; +static const size_t thread_local_buffer_size = 128; bool JfrCheckpointManager::initialize() { - assert(_mspace == NULL, "invariant"); - _mspace = allocate_mspace(buffer_size, 0, 0, false, this); // post-pone preallocation - if (_mspace == NULL) { + assert(_global_mspace == NULL, "invariant"); + _global_mspace = create_mspace(global_buffer_size, 0, 0, false, this); // post-pone preallocation + if (_global_mspace == NULL) { return false; } // preallocate buffer count to each of the epoch live lists - for (size_t i = 0; i < buffer_count * 2; ++i) { - Buffer* const buffer = mspace_allocate(buffer_size, _mspace); - _mspace->add_to_live_list(buffer, i % 2 == 0); + for (size_t i = 0; i < global_buffer_prealloc_count * 2; ++i) { + Buffer* const buffer = mspace_allocate(global_buffer_size, _global_mspace); + _global_mspace->add_to_live_list(buffer, i % 2 == 0); } - assert(_mspace->free_list_is_empty(), "invariant"); - return JfrTypeManager::initialize() && JfrTraceIdLoadBarrier::initialize(); -} + assert(_global_mspace->free_list_is_empty(), "invariant"); -void JfrCheckpointManager::register_full(BufferPtr buffer, Thread* thread) { - // nothing here at the moment - assert(buffer != NULL, "invariant"); - assert(buffer->acquired_by(thread), "invariant"); - assert(buffer->retired(), "invariant"); + assert(_thread_local_mspace == NULL, "invariant"); + _thread_local_mspace = new JfrThreadLocalCheckpointMspace(); + if (_thread_local_mspace == NULL || !_thread_local_mspace->initialize(thread_local_buffer_size, + JFR_MSPACE_UNLIMITED_CACHE_SIZE, + thread_local_buffer_prealloc_count)) { + return false; + } + return JfrTypeManager::initialize() && JfrTraceIdLoadBarrier::initialize(); } #ifdef ASSERT @@ -149,15 +144,28 @@ static void assert_release(const BufferPtr buffer) { assert(buffer->lease(), "invariant"); assert(buffer->acquired_by_self(), "invariant"); } + +static void assert_retired(const BufferPtr buffer, Thread* thread) { + assert(buffer != NULL, "invariant"); + assert(buffer->acquired_by(thread), "invariant"); + assert(buffer->retired(), "invariant"); +} #endif // ASSERT -static BufferPtr lease(size_t size, JfrCheckpointMspace* mspace, size_t retry_count, Thread* thread, bool previous_epoch) { +void JfrCheckpointManager::register_full(BufferPtr buffer, Thread* thread) { + DEBUG_ONLY(assert_retired(buffer, thread);) + // nothing here at the moment +} + +BufferPtr JfrCheckpointManager::lease(Thread* thread, bool previous_epoch /* false */, size_t size /* 0 */) { + JfrCheckpointMspace* const mspace = instance()._global_mspace; assert(mspace != NULL, "invariant"); static const size_t max_elem_size = mspace->min_element_size(); // min is max BufferPtr buffer; if (size <= max_elem_size) { - buffer = mspace_acquire_lease_with_retry(size, mspace, retry_count, thread, previous_epoch); + buffer = mspace_acquire_live(size, mspace, thread, previous_epoch); if (buffer != NULL) { + buffer->set_lease(); DEBUG_ONLY(assert_lease(buffer);) return buffer; } @@ -167,54 +175,70 @@ static BufferPtr lease(size_t size, JfrCheckpointMspace* mspace, size_t retry_co return buffer; } -static const size_t lease_retry = 100; +const u1 thread_local_context = 1; -BufferPtr JfrCheckpointManager::lease(Thread* thread, bool previous_epoch /* false */, size_t size /* 0 */) { - return ::lease(size, instance()._mspace, lease_retry, thread, previous_epoch); +static bool is_thread_local(JfrBuffer* buffer) { + assert(buffer != NULL, "invariant"); + return buffer->context() == thread_local_context; } -bool JfrCheckpointManager::lookup(BufferPtr old) const { - assert(old != NULL, "invariant"); - return !_mspace->in_current_epoch_list(old); -} - -BufferPtr JfrCheckpointManager::lease(BufferPtr old, Thread* thread, size_t size /* 0 */) { - assert(old != NULL, "invariant"); - return ::lease(size, instance()._mspace, lease_retry, thread, instance().lookup(old)); +static void retire(JfrBuffer* buffer) { + DEBUG_ONLY(assert_release(buffer);) + buffer->clear_lease(); + buffer->set_retired(); } /* - * If the buffer was a lease, release back. - * * The buffer is effectively invalidated for the thread post-return, * and the caller should take means to ensure that it is not referenced. */ -static void release(BufferPtr buffer, Thread* thread) { +static void release(JfrBuffer* buffer) { DEBUG_ONLY(assert_release(buffer);) - buffer->clear_lease(); - if (buffer->transient()) { - buffer->set_retired(); + if (is_thread_local(buffer)) { + retire(buffer); } else { + buffer->clear_lease(); buffer->release(); } } +BufferPtr JfrCheckpointManager::acquire_thread_local(size_t size, Thread* thread) { + assert(thread != NULL, "invariant"); + JfrBuffer* const buffer = instance()._thread_local_mspace->acquire(size, thread); + assert(buffer != NULL, "invariant"); + assert(buffer->free_size() >= size, "invariant"); + buffer->set_context(thread_local_context); + assert(is_thread_local(buffer), "invariant"); + buffer->set_lease(); + return buffer; +} + +BufferPtr JfrCheckpointManager::lease_thread_local(Thread* thread, size_t size /* 0 */) { + JfrBuffer* const buffer = acquire_thread_local(size, thread); + DEBUG_ONLY(assert_lease(buffer);) + return buffer; +} + +BufferPtr JfrCheckpointManager::lease(BufferPtr old, Thread* thread, size_t size) { + assert(old != NULL, "invariant"); + return is_thread_local(old) ? acquire_thread_local(size, thread) : + lease(thread, instance()._global_mspace->in_previous_epoch_list(old), size); +} BufferPtr JfrCheckpointManager::flush(BufferPtr old, size_t used, size_t requested, Thread* thread) { assert(old != NULL, "invariant"); assert(old->lease(), "invariant"); if (0 == requested) { // indicates a lease is being returned - release(old, thread); + release(old); + // signal completion of a new checkpoint set_constant_pending(); return NULL; } - // migration of in-flight information - BufferPtr const new_buffer = lease(old, thread, used + requested); - if (new_buffer != NULL) { - migrate_outstanding_writes(old, new_buffer, used, requested); - } - release(old, thread); - return new_buffer; // might be NULL + BufferPtr new_buffer = lease(old, thread, used + requested); + assert(new_buffer != NULL, "invariant"); + migrate_outstanding_writes(old, new_buffer, used, requested); + retire(old); + return new_buffer; } // offsets into the JfrCheckpointEntry @@ -311,7 +335,7 @@ class CheckpointWriteOp { typedef CheckpointWriteOp WriteOperation; typedef MutexedWriteOp MutexedWriteOperation; -typedef ReleaseOpWithExcision ReleaseOperation; +typedef ReleaseWithExcisionOp ReleaseOperation; typedef CompositeOperation WriteReleaseOperation; void JfrCheckpointManager::begin_epoch_shift() { @@ -328,12 +352,13 @@ void JfrCheckpointManager::end_epoch_shift() { size_t JfrCheckpointManager::write() { DEBUG_ONLY(JfrJavaSupport::check_java_thread_in_native(Thread::current())); - assert(_mspace->free_list_is_empty(), "invariant"); WriteOperation wo(_chunkwriter); MutexedWriteOperation mwo(wo); - ReleaseOperation ro(_mspace, _mspace->live_list(true)); + _thread_local_mspace->iterate(mwo, true); // previous epoch list + assert(_global_mspace->free_list_is_empty(), "invariant"); + ReleaseOperation ro(_global_mspace, _global_mspace->live_list(true)); WriteReleaseOperation wro(&mwo, &ro); - process_live_list(wro, _mspace, true); + process_live_list(wro, _global_mspace, true); // previous epoch list return wo.processed(); } @@ -344,10 +369,11 @@ size_t JfrCheckpointManager::clear() { JfrTraceIdLoadBarrier::clear(); clear_type_set(); DiscardOperation discard_operation(mutexed); // mutexed discard mode - ReleaseOperation ro(_mspace, _mspace->live_list(true)); + _thread_local_mspace->iterate(discard_operation, true); // previous epoch list + ReleaseOperation ro(_global_mspace, _global_mspace->live_list(true)); DiscardReleaseOperation discard_op(&discard_operation, &ro); - assert(_mspace->free_list_is_empty(), "invariant"); - process_live_list(discard_op, _mspace, true); // previous epoch list + assert(_global_mspace->free_list_is_empty(), "invariant"); + process_live_list(discard_op, _global_mspace, true); // previous epoch list return discard_operation.elements(); } @@ -451,8 +477,9 @@ size_t JfrCheckpointManager::flush_type_set() { if (is_constant_pending()) { WriteOperation wo(_chunkwriter); MutexedWriteOperation mwo(wo); - assert(_mspace->live_list_is_nonempty(), "invariant"); - process_live_list(mwo, _mspace); + _thread_local_mspace->iterate(mwo); // current epoch list + assert(_global_mspace->live_list_is_nonempty(), "invariant"); + process_live_list(mwo, _global_mspace); // current epoch list } return elements; } diff --git a/src/hotspot/share/jfr/recorder/checkpoint/jfrCheckpointManager.hpp b/src/hotspot/share/jfr/recorder/checkpoint/jfrCheckpointManager.hpp index d9c2ad8391a..428b6e6fe2a 100644 --- a/src/hotspot/share/jfr/recorder/checkpoint/jfrCheckpointManager.hpp +++ b/src/hotspot/share/jfr/recorder/checkpoint/jfrCheckpointManager.hpp @@ -26,14 +26,13 @@ #define SHARE_JFR_RECORDER_CHECKPOINT_JFRCHECKPOINTMANAGER_HPP #include "jfr/recorder/storage/jfrBuffer.hpp" +#include "jfr/recorder/storage/jfrEpochStorage.hpp" #include "jfr/recorder/storage/jfrMemorySpace.hpp" #include "jfr/recorder/storage/jfrMemorySpaceRetrieval.hpp" #include "jfr/utilities/jfrLinkedList.hpp" class JfrCheckpointManager; class JfrChunkWriter; -class JfrSerializer; -class JfrTypeManager; class Thread; struct JfrCheckpointEntry { @@ -45,6 +44,7 @@ struct JfrCheckpointEntry { }; typedef JfrMemorySpace, JfrLinkedList, true > JfrCheckpointMspace; +typedef JfrEpochStorageHost JfrThreadLocalCheckpointMspace; // // Responsible for maintaining checkpoints and by implication types. @@ -56,7 +56,8 @@ class JfrCheckpointManager : public JfrCHeapObj { typedef JfrCheckpointMspace::Node Buffer; typedef JfrCheckpointMspace::NodePtr BufferPtr; private: - JfrCheckpointMspace* _mspace; + JfrCheckpointMspace* _global_mspace; + JfrThreadLocalCheckpointMspace* _thread_local_mspace; JfrChunkWriter& _chunkwriter; JfrCheckpointManager(JfrChunkWriter& cw); @@ -66,14 +67,16 @@ class JfrCheckpointManager : public JfrCHeapObj { bool initialize(); static void destroy(); - bool lookup(Buffer* old) const; static BufferPtr lease(Thread* thread, bool previous_epoch = false, size_t size = 0); - static BufferPtr lease(BufferPtr old, Thread* thread, size_t size = 0); + static BufferPtr lease(BufferPtr old, Thread* thread, size_t size); + + static BufferPtr acquire_thread_local(size_t size, Thread* thread); + static BufferPtr lease_thread_local(Thread* thread, size_t size = 0); + static BufferPtr flush(BufferPtr old, size_t used, size_t requested, Thread* thread); size_t clear(); size_t write(); - size_t flush(); void notify_threads(); size_t write_static_type_set(Thread* thread); @@ -102,7 +105,6 @@ class JfrCheckpointManager : public JfrCHeapObj { friend class JfrCheckpointFlush; friend class JfrCheckpointWriter; friend class JfrSerializer; - friend class JfrStackTraceRepository; template class, typename, typename, bool> friend class JfrMemorySpace; }; diff --git a/src/hotspot/share/jfr/recorder/checkpoint/jfrCheckpointWriter.cpp b/src/hotspot/share/jfr/recorder/checkpoint/jfrCheckpointWriter.cpp index 604dce389ba..12497802cf7 100644 --- a/src/hotspot/share/jfr/recorder/checkpoint/jfrCheckpointWriter.cpp +++ b/src/hotspot/share/jfr/recorder/checkpoint/jfrCheckpointWriter.cpp @@ -45,8 +45,8 @@ JfrCheckpointWriter::JfrCheckpointWriter(JfrCheckpointType type /* GENERIC */) : } } -JfrCheckpointWriter::JfrCheckpointWriter(Thread* thread, bool header /* true */, JfrCheckpointType type /* GENERIC */) : - JfrCheckpointWriterBase(JfrCheckpointManager::lease(thread), thread), +JfrCheckpointWriter::JfrCheckpointWriter(Thread* thread, bool header /* true */, JfrCheckpointType type /* GENERIC */, bool global_lease /* true */) : + JfrCheckpointWriterBase(global_lease ? JfrCheckpointManager::lease(thread) : JfrCheckpointManager::lease_thread_local(thread), thread), _time(JfrTicks::now()), _offset(0), _count(0), diff --git a/src/hotspot/share/jfr/recorder/checkpoint/jfrCheckpointWriter.hpp b/src/hotspot/share/jfr/recorder/checkpoint/jfrCheckpointWriter.hpp index 2a2e0f14797..3666d7f747b 100644 --- a/src/hotspot/share/jfr/recorder/checkpoint/jfrCheckpointWriter.hpp +++ b/src/hotspot/share/jfr/recorder/checkpoint/jfrCheckpointWriter.hpp @@ -72,7 +72,7 @@ class JfrCheckpointWriter : public JfrCheckpointWriterBase { JfrCheckpointWriter(bool previous_epoch, Thread* thread, JfrCheckpointType type = GENERIC); public: JfrCheckpointWriter(JfrCheckpointType type = GENERIC); - JfrCheckpointWriter(Thread* thread, bool header = true, JfrCheckpointType mode = GENERIC); + JfrCheckpointWriter(Thread* thread, bool header = true, JfrCheckpointType mode = GENERIC, bool global_lease = true); ~JfrCheckpointWriter(); void write_type(JfrTypeId type_id); void write_count(u4 nof_entries); diff --git a/src/hotspot/share/jfr/recorder/checkpoint/types/jfrTypeManager.cpp b/src/hotspot/share/jfr/recorder/checkpoint/types/jfrTypeManager.cpp index 5c30e1699b2..40525969efe 100644 --- a/src/hotspot/share/jfr/recorder/checkpoint/types/jfrTypeManager.cpp +++ b/src/hotspot/share/jfr/recorder/checkpoint/types/jfrTypeManager.cpp @@ -28,6 +28,7 @@ #include "jfr/recorder/checkpoint/types/jfrType.hpp" #include "jfr/recorder/checkpoint/types/jfrTypeManager.hpp" #include "jfr/recorder/jfrRecorder.hpp" +#include "jfr/support/jfrThreadLocal.hpp" #include "jfr/utilities/jfrIterator.hpp" #include "jfr/utilities/jfrLinkedList.inline.hpp" #include "memory/resourceArea.hpp" @@ -105,7 +106,7 @@ void JfrTypeManager::create_thread_blob(Thread* t) { ResourceMark rm(t); HandleMark hm(t); JfrThreadConstant type_thread(t); - JfrCheckpointWriter writer(t, true, THREADS); + JfrCheckpointWriter writer(t, true, THREADS, false); writer.write_type(TYPE_THREAD); type_thread.serialize(writer); // create and install a checkpoint blob @@ -115,12 +116,11 @@ void JfrTypeManager::create_thread_blob(Thread* t) { void JfrTypeManager::write_thread_checkpoint(Thread* t) { assert(t != NULL, "invariant"); - ResourceMark rm(t); - HandleMark hm(t); - JfrThreadConstant type_thread(t); - JfrCheckpointWriter writer(t, true, THREADS); - writer.write_type(TYPE_THREAD); - type_thread.serialize(writer); + if (!t->jfr_thread_local()->has_thread_blob()) { + create_thread_blob(t); + } + JfrCheckpointWriter writer(t, false, THREADS, false); + t->jfr_thread_local()->thread_blob()->write(writer); } class SerializerRegistrationGuard : public StackObj { diff --git a/src/hotspot/share/jfr/recorder/storage/jfrBuffer.cpp b/src/hotspot/share/jfr/recorder/storage/jfrBuffer.cpp index e908bfef699..838d126c00a 100644 --- a/src/hotspot/share/jfr/recorder/storage/jfrBuffer.cpp +++ b/src/hotspot/share/jfr/recorder/storage/jfrBuffer.cpp @@ -32,9 +32,10 @@ JfrBuffer::JfrBuffer() : _next(NULL), _identity(NULL), _pos(NULL), _top(NULL), - _flags(0), + _size(0), _header_size(0), - _size(0) {} + _flags(0), + _context(0) {} bool JfrBuffer::initialize(size_t header_size, size_t size) { assert(_next == NULL, "invariant"); @@ -52,7 +53,6 @@ bool JfrBuffer::initialize(size_t header_size, size_t size) { void JfrBuffer::reinitialize(bool exclusion /* false */) { acquire_critical_section_top(); - assert(!lease(), "invariant"); if (exclusion != excluded()) { // update if (exclusion) { @@ -185,25 +185,25 @@ enum FLAG { EXCLUDED = 8 }; -inline u2 load(const volatile u2* flags) { - assert(flags != NULL, "invariant"); - return Atomic::load_acquire(flags); +inline u1 load(const volatile u1* dest) { + assert(dest != NULL, "invariant"); + return Atomic::load_acquire(dest); } -inline void set(u2* flags, FLAG flag) { - assert(flags != NULL, "invariant"); +inline void set(u1* dest, u1 data) { + assert(dest != NULL, "invariant"); OrderAccess::storestore(); - *flags |= (u1)flag; + *dest |= data; } -inline void clear(u2* flags, FLAG flag) { - assert(flags != NULL, "invariant"); +inline void clear(u1* dest, u1 data) { + assert(dest != NULL, "invariant"); OrderAccess::storestore(); - *flags ^= (u1)flag; + *dest ^= data; } -inline bool test(const u2* flags, FLAG flag) { - return (u1)flag == (load(flags) & (u1)flag); +inline bool test(const u1* dest, u1 data) { + return data == (load(dest) & data); } bool JfrBuffer::transient() const { @@ -273,3 +273,15 @@ void JfrBuffer::clear_retired() { clear(&_flags, RETIRED); } } + +u1 JfrBuffer::context() const { + return load(&_context); +} + +void JfrBuffer::set_context(u1 context) { + set(&_context, context); +} + +void JfrBuffer::clear_context() { + set(&_context, 0); +} diff --git a/src/hotspot/share/jfr/recorder/storage/jfrBuffer.hpp b/src/hotspot/share/jfr/recorder/storage/jfrBuffer.hpp index f26eef06945..120c7311321 100644 --- a/src/hotspot/share/jfr/recorder/storage/jfrBuffer.hpp +++ b/src/hotspot/share/jfr/recorder/storage/jfrBuffer.hpp @@ -44,6 +44,10 @@ // e.g. the delta must always be fully parsable. // _top can move concurrently by other threads but is always <= _pos. // +// The _flags field holds generic tags applicable to all subsystems. +// +// The _context field can be used to set subsystem specific tags onto a buffer. +// // Memory ordering: // // Method Owner thread Other threads @@ -66,9 +70,10 @@ class JfrBuffer { const void* _identity; u1* _pos; mutable const u1* _top; - u2 _flags; - u2 _header_size; u4 _size; + u2 _header_size; + u1 _flags; + u1 _context; const u1* stable_top() const; @@ -168,6 +173,10 @@ class JfrBuffer { bool excluded() const; void set_excluded(); void clear_excluded(); + + u1 context() const; + void set_context(u1 context); + void clear_context(); }; #endif // SHARE_JFR_RECORDER_STORAGE_JFRBUFFER_HPP diff --git a/src/hotspot/share/jfr/recorder/storage/jfrEpochStorage.hpp b/src/hotspot/share/jfr/recorder/storage/jfrEpochStorage.hpp index 7592cbd6917..02f6a0ca774 100644 --- a/src/hotspot/share/jfr/recorder/storage/jfrEpochStorage.hpp +++ b/src/hotspot/share/jfr/recorder/storage/jfrEpochStorage.hpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2019, 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 @@ -33,9 +33,19 @@ /* * Provides storage as a function of an epoch, with iteration capabilities for the current and previous epoch. - * Iteration over the current epoch is incremental while iteration over the previous epoch is complete, - * including storage reclamation. The design caters to use cases having multiple incremental iterations - * over the current epoch, and a single, complete, iteration over the previous epoch. + * + * When iterating the previous epoch, where exclusive access to buffers is assumed, + * all buffers will be reinitialized post-callback, with retired buffers reclaimed + * and moved onto the free list and non-retired buffers left in-place. + * + * When iterating the current epoch, where concurrent access to buffers is assumed, + * there exist two modes, controlled by the EagerReclaim parameter. + * By default, EagerReclaim is false, meaning no retired buffers are reclaimed during the current epoch. + * Setting EagerReclaim to true, retired buffers will be reclaimed post-callback, by reinitialization + * and by moving them onto the free list, just like is done when iterating the previous epoch. + * + * The design caters to use cases having multiple incremental iterations over the current epoch, + * and a single iteration over the previous epoch. * * The JfrEpochStorage can be specialized by the following policies: * @@ -43,10 +53,12 @@ * * RetrievalPolicy see jfrMemorySpace.hpp for a description. * + * EagerReclaim should retired buffers be reclaimed also during the current epoch (i.e. eagerly) + * */ -template class RetrievalPolicy> +template class RetrievalPolicy, bool EagerReclaim = false> class JfrEpochStorageHost : public JfrCHeapObj { - typedef JfrMemorySpace, + typedef JfrMemorySpace, RetrievalPolicy, JfrConcurrentQueue, JfrLinkedList, diff --git a/src/hotspot/share/jfr/recorder/storage/jfrEpochStorage.inline.hpp b/src/hotspot/share/jfr/recorder/storage/jfrEpochStorage.inline.hpp index 725a08a92c4..464d393adc1 100644 --- a/src/hotspot/share/jfr/recorder/storage/jfrEpochStorage.inline.hpp +++ b/src/hotspot/share/jfr/recorder/storage/jfrEpochStorage.inline.hpp @@ -32,23 +32,23 @@ #include "jfr/utilities/jfrLinkedList.inline.hpp" #include "logging/log.hpp" -template class RetrievalPolicy> -JfrEpochStorageHost::JfrEpochStorageHost() : _mspace(NULL) {} +template class RetrievalPolicy, bool EagerReclaim> +JfrEpochStorageHost::JfrEpochStorageHost() : _mspace(NULL) {} -template class RetrievalPolicy> -JfrEpochStorageHost::~JfrEpochStorageHost() { +template class RetrievalPolicy, bool EagerReclaim> +JfrEpochStorageHost::~JfrEpochStorageHost() { delete _mspace; } -template class RetrievalPolicy> -bool JfrEpochStorageHost::initialize(size_t min_elem_size, size_t free_list_cache_count_limit, size_t cache_prealloc_count) { +template class RetrievalPolicy, bool EagerReclaim> +bool JfrEpochStorageHost::initialize(size_t min_elem_size, size_t free_list_cache_count_limit, size_t cache_prealloc_count) { assert(_mspace == NULL, "invariant"); _mspace = new EpochMspace(min_elem_size, free_list_cache_count_limit, this); return _mspace != NULL && _mspace->initialize(cache_prealloc_count); } -template class RetrievalPolicy> -inline NodeType* JfrEpochStorageHost::acquire(size_t size, Thread* thread) { +template class RetrievalPolicy, bool EagerReclaim> +inline NodeType* JfrEpochStorageHost::acquire(size_t size, Thread* thread) { BufferPtr buffer = mspace_acquire_to_live_list(size, _mspace, thread); if (buffer == NULL) { log_warning(jfr)("Unable to allocate " SIZE_FORMAT " bytes of %s.", _mspace->min_element_size(), "epoch storage"); @@ -58,29 +58,37 @@ inline NodeType* JfrEpochStorageHost::acquire(size_t return buffer; } -template class RetrievalPolicy> -void JfrEpochStorageHost::release(NodeType* buffer) { +template class RetrievalPolicy, bool EagerReclaim> +void JfrEpochStorageHost::release(NodeType* buffer) { assert(buffer != NULL, "invariant"); buffer->set_retired(); } -template class RetrievalPolicy> -void JfrEpochStorageHost::register_full(NodeType* buffer, Thread* thread) { +template class RetrievalPolicy, bool EagerReclaim> +void JfrEpochStorageHost::register_full(NodeType* buffer, Thread* thread) { // nothing here at the moment } -template class RetrievalPolicy> +template class RetrievalPolicy, bool EagerReclaim> template -void JfrEpochStorageHost::iterate(Functor& functor, bool previous_epoch) { - typedef ReleaseRetiredToFreeListOp ReleaseStorage; - typedef CompositeOperation PreviousEpochOperation; +void JfrEpochStorageHost::iterate(Functor& functor, bool previous_epoch) { + typedef ReinitializeAllReleaseRetiredOp PreviousEpochReleaseOperation; + typedef CompositeOperation PreviousEpochOperation; + typedef ReleaseRetiredOp CurrentEpochReleaseOperation; + typedef CompositeOperation CurrentEpochOperation; if (previous_epoch) { - ReleaseStorage rs(_mspace, _mspace->live_list(true)); - PreviousEpochOperation peo(&functor, &rs); - process_live_list(peo, _mspace, true); + PreviousEpochReleaseOperation pero(_mspace, _mspace->live_list(true)); + PreviousEpochOperation peo(&functor, &pero); + process_live_list(peo, _mspace, true); // previous epoch list return; } - process_live_list(functor, _mspace, false); + if (EagerReclaim) { + CurrentEpochReleaseOperation cero(_mspace, _mspace->live_list()); + CurrentEpochOperation ceo(&functor, &cero); + process_live_list(ceo, _mspace, false); // current epoch list + return; + } + process_live_list(functor, _mspace, false); // current epoch list } #ifdef ASSERT @@ -100,8 +108,8 @@ class EmptyVerifier { } }; -template class RetrievalPolicy> -void JfrEpochStorageHost::verify_previous_empty() const { +template class RetrievalPolicy, bool EagerReclaim> +void JfrEpochStorageHost::verify_previous_empty() const { typedef EmptyVerifier VerifyEmptyMspace; VerifyEmptyMspace vem(_mspace); process_live_list(vem, _mspace, true); diff --git a/src/hotspot/share/jfr/recorder/storage/jfrMemorySpace.inline.hpp b/src/hotspot/share/jfr/recorder/storage/jfrMemorySpace.inline.hpp index 40d585f42b0..47ae658b7b2 100644 --- a/src/hotspot/share/jfr/recorder/storage/jfrMemorySpace.inline.hpp +++ b/src/hotspot/share/jfr/recorder/storage/jfrMemorySpace.inline.hpp @@ -493,14 +493,14 @@ inline bool ReleaseOp::process(typename Mspace::NodePtr node) { } template -class ReleaseOpWithExcision : public ReleaseOp { +class ReleaseWithExcisionOp : public ReleaseOp { private: List& _list; typename List::NodePtr _prev; size_t _count; size_t _amount; public: - ReleaseOpWithExcision(Mspace* mspace, List& list) : + ReleaseWithExcisionOp(Mspace* mspace, List& list) : ReleaseOp(mspace), _list(list), _prev(NULL), _count(0), _amount(0) {} bool process(typename List::NodePtr node); size_t processed() const { return _count; } @@ -508,7 +508,7 @@ class ReleaseOpWithExcision : public ReleaseOp { }; template -inline bool ReleaseOpWithExcision::process(typename List::NodePtr node) { +inline bool ReleaseWithExcisionOp::process(typename List::NodePtr node) { assert(node != NULL, "invariant"); if (node->transient()) { _prev = _list.excise(_prev, node); @@ -569,20 +569,49 @@ inline bool ScavengingReleaseOp::excise_with_release(typename List } template -class ReleaseRetiredToFreeListOp : public StackObj { +class ReleaseRetiredOp : public StackObj { +private: + Mspace* _mspace; + FromList& _list; + typename Mspace::NodePtr _prev; +public: + typedef typename Mspace::Node Node; + ReleaseRetiredOp(Mspace* mspace, FromList& list) : + _mspace(mspace), _list(list), _prev(NULL) {} + bool process(Node* node); +}; + +template +inline bool ReleaseRetiredOp::process(typename Mspace::Node* node) { + assert(node != NULL, "invariant"); + if (node->retired()) { + _prev = _list.excise(_prev, node); + node->reinitialize(); + assert(node->empty(), "invariant"); + assert(!node->retired(), "invariant"); + node->release(); + mspace_release(node, _mspace); + } else { + _prev = node; + } + return true; +} + +template +class ReinitializeAllReleaseRetiredOp : public StackObj { private: Mspace* _mspace; FromList& _list; typename Mspace::NodePtr _prev; public: typedef typename Mspace::Node Node; - ReleaseRetiredToFreeListOp(Mspace* mspace, FromList& list) : + ReinitializeAllReleaseRetiredOp(Mspace* mspace, FromList& list) : _mspace(mspace), _list(list), _prev(NULL) {} bool process(Node* node); }; template -inline bool ReleaseRetiredToFreeListOp::process(typename Mspace::Node* node) { +inline bool ReinitializeAllReleaseRetiredOp::process(typename Mspace::Node* node) { assert(node != NULL, "invariant"); // assumes some means of exclusive access to node const bool retired = node->retired(); diff --git a/src/hotspot/share/jfr/recorder/stringpool/jfrStringPool.cpp b/src/hotspot/share/jfr/recorder/stringpool/jfrStringPool.cpp index 230baf33a23..de836b00064 100644 --- a/src/hotspot/share/jfr/recorder/stringpool/jfrStringPool.cpp +++ b/src/hotspot/share/jfr/recorder/stringpool/jfrStringPool.cpp @@ -208,7 +208,7 @@ typedef StringPoolOp WriteOperation; typedef StringPoolOp DiscardOperation; typedef ExclusiveOp ExclusiveWriteOperation; typedef ExclusiveOp ExclusiveDiscardOperation; -typedef ReleaseOpWithExcision ReleaseOperation; +typedef ReleaseWithExcisionOp ReleaseOperation; typedef CompositeOperation WriteReleaseOperation; typedef CompositeOperation DiscardReleaseOperation; diff --git a/src/hotspot/share/jfr/utilities/jfrConcurrentLinkedListHost.inline.hpp b/src/hotspot/share/jfr/utilities/jfrConcurrentLinkedListHost.inline.hpp index 48b54fda18a..0f3e8fb55b9 100644 --- a/src/hotspot/share/jfr/utilities/jfrConcurrentLinkedListHost.inline.hpp +++ b/src/hotspot/share/jfr/utilities/jfrConcurrentLinkedListHost.inline.hpp @@ -34,7 +34,7 @@ /* * The removal marker (i.e. the excision bit) is represented by '( )' as part of state description comments: - * node --> next becomes (node) --> next, when node is logically deleted. + * "node --> next" becomes "(node) --> next", when node is logically deleted. */ template inline Node* mark_for_removal(Node* node) { @@ -47,7 +47,7 @@ inline Node* mark_for_removal(Node* node) { /* * The insertion marker (i.e. the insertion bit) is represented by '[ ]' as part of state description comments: - * "node --> next" becomes "[node} --> next", in an attempt to convey node as being exlusively reserved. + * "node --> next" becomes "[node] --> next", in an attempt to convey node as being exlusively reserved. */ template inline bool mark_for_insertion(Node* node, const Node* tail) { From 0f43de9f026d5405dd7a38799e499312ef8df890 Mon Sep 17 00:00:00 2001 From: Daniel Fuchs Date: Tue, 30 Jun 2020 18:52:59 +0100 Subject: [PATCH 13/13] 8246114: java/net/MulticastSocket/Promiscuous.java fails after 8241072 (multi-homed systems) Fixed the test - an IPv4 group cannot be joined from an interface that has no IPv4 address configured Reviewed-by: alanb, amlu --- test/jdk/java/net/MulticastSocket/Promiscuous.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/jdk/java/net/MulticastSocket/Promiscuous.java b/test/jdk/java/net/MulticastSocket/Promiscuous.java index ed99408c754..5f6126ae425 100644 --- a/test/jdk/java/net/MulticastSocket/Promiscuous.java +++ b/test/jdk/java/net/MulticastSocket/Promiscuous.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2013, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2013, 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 @@ -119,7 +119,7 @@ public class Promiscuous { // join groups on all network interfaces NetworkConfiguration.probe() - .multicastInterfaces(false) + .ip4MulticastInterfaces(false) .forEach((nic) -> { try { mc1.joinGroup(toSocketAddress(group1), nic); @@ -155,7 +155,7 @@ public class Promiscuous { // leave groups on all network interfaces NetworkConfiguration.probe() - .multicastInterfaces(false) + .ip4MulticastInterfaces(false) .forEach((nic) -> { try { mc1.leaveGroup(toSocketAddress(group1), nic);