From b55902f065e94ff521648f77b803b0e32b3de7b0 Mon Sep 17 00:00:00 2001 From: David Leopoldseder Date: Tue, 2 Dec 2025 16:20:18 +0100 Subject: [PATCH 01/18] update owners --- substratevm/src/com.oracle.svm.interpreter.metadata/OWNERS.toml | 1 + substratevm/src/com.oracle.svm.interpreter/OWNERS.toml | 1 + 2 files changed, 2 insertions(+) diff --git a/substratevm/src/com.oracle.svm.interpreter.metadata/OWNERS.toml b/substratevm/src/com.oracle.svm.interpreter.metadata/OWNERS.toml index 67c7e4be1111..31d2b48d7cf3 100644 --- a/substratevm/src/com.oracle.svm.interpreter.metadata/OWNERS.toml +++ b/substratevm/src/com.oracle.svm.interpreter.metadata/OWNERS.toml @@ -3,4 +3,5 @@ files = "*" all = [ "bernhard.urban-forster@oracle.com", "gilles.m.duboscq@oracle.com", + "david.leopoldseder@oracle.com" ] diff --git a/substratevm/src/com.oracle.svm.interpreter/OWNERS.toml b/substratevm/src/com.oracle.svm.interpreter/OWNERS.toml index 67c7e4be1111..31d2b48d7cf3 100644 --- a/substratevm/src/com.oracle.svm.interpreter/OWNERS.toml +++ b/substratevm/src/com.oracle.svm.interpreter/OWNERS.toml @@ -3,4 +3,5 @@ files = "*" all = [ "bernhard.urban-forster@oracle.com", "gilles.m.duboscq@oracle.com", + "david.leopoldseder@oracle.com" ] From 9899bc2c1e2970427dec55f90fda28da894b5995 Mon Sep 17 00:00:00 2001 From: David Leopoldseder Date: Fri, 21 Nov 2025 12:24:43 +0100 Subject: [PATCH 02/18] ristretto: expand profiles and add first prototype of branch profiles --- .../profile/InterpreterProfileContainer.java | 335 ++++++++++++++++++ .../oracle/svm/interpreter/Interpreter.java | 18 +- .../ristretto/RistrettoRuntimeOptions.java | 3 + .../interpreter/ristretto/RistrettoUtils.java | 91 +++-- .../ristretto/meta/RistrettoMethod.java | 25 +- .../profile/RistrettoCompilationManager.java | 2 +- .../profile/RistrettoProfileProvider.java} | 30 +- .../profile/RistrettoProfileSupport.java | 86 ++++- .../profile/RistrettoProfilingInfo.java | 119 +++++++ 9 files changed, 662 insertions(+), 47 deletions(-) create mode 100644 substratevm/src/com.oracle.svm.interpreter.metadata/src/com/oracle/svm/interpreter/metadata/profile/InterpreterProfileContainer.java rename substratevm/src/{com.oracle.svm.interpreter.metadata/src/com/oracle/svm/interpreter/metadata/profile/InterpreterProfile.java => com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/profile/RistrettoProfileProvider.java} (58%) create mode 100644 substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/profile/RistrettoProfilingInfo.java diff --git a/substratevm/src/com.oracle.svm.interpreter.metadata/src/com/oracle/svm/interpreter/metadata/profile/InterpreterProfileContainer.java b/substratevm/src/com.oracle.svm.interpreter.metadata/src/com/oracle/svm/interpreter/metadata/profile/InterpreterProfileContainer.java new file mode 100644 index 000000000000..8258c20a5cfd --- /dev/null +++ b/substratevm/src/com.oracle.svm.interpreter.metadata/src/com/oracle/svm/interpreter/metadata/profile/InterpreterProfileContainer.java @@ -0,0 +1,335 @@ +/* + * Copyright (c) 2025, 2025, 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. Oracle 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. + * + * 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 com.oracle.svm.interpreter.metadata.profile; + +import static jdk.graal.compiler.bytecode.Bytecodes.END; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; + +import com.oracle.svm.core.log.Log; +import com.oracle.svm.interpreter.metadata.Bytecodes; + +import jdk.graal.compiler.bytecode.BytecodeStream; +import jdk.vm.ci.meta.ResolvedJavaMethod; + +/** + * Abstract base representation of profile data for crema. + */ +public final class InterpreterProfileContainer { + + /** + * Developer debug only flag to log profile creation and access. + */ + private static final boolean LOG_PROFILE_MACHINERY = false; + + private InterpreterProfile[] profiles; + /* + * Cached entry always starts at 0, if it's a cache miss we will initialize it to the correct + * value. + */ + private int lastIndex = 0; + private final ResolvedJavaMethod method; + private boolean isMature; + + public InterpreterProfileContainer(ResolvedJavaMethod method) { + this.method = method; + buildProfiles(); + } + + private void buildProfiles() { + BytecodeStream stream = new BytecodeStream(method.getCode()); + stream.setBCI(0); + + ArrayList allProfiles = new ArrayList<>(); + // we always add a method entry counting profile + allProfiles.add(new CountingProfile(JVMCI_METHOD_ENTRY_BCI)); + + while (stream.currentBC() != END) { + int bci = stream.currentBCI(); + int bc = stream.currentBC(); + // in theory, we can have multiple profiles for a single BCI, type, exception etc + if (inArray(BRANCH_PROFILED_BYTECODES, bc)) { + // allocate a branch profile for the bci + allProfiles.add(new BranchProfile(bci)); + } + if (inArray(TYPE_PROFILED_BYTECODES, bc)) { + // allocate a type profile + // TODO GR-71567 + } + // TODO - backedge / goto profiles + stream.next(); + } + if (LOG_PROFILE_MACHINERY) { + Log.log().string("Assigning profiles ").string(Arrays.toString(allProfiles.toArray())).string(" to method ").string(method.toString()).newline(); + } + profiles = allProfiles.toArray(new InterpreterProfile[0]); + } + + public ResolvedJavaMethod getMethod() { + return method; + } + + public boolean isMature() { + return isMature; + } + + public void setMature(boolean mature) { + isMature = mature; + } + + private static boolean inArray(int[] arr, int key) { + for (int i : arr) { + if (i == key) { + return true; + } + } + return false; + } + + public long profileMethodEntry() { + return ((CountingProfile) getAtBCI(JVMCI_METHOD_ENTRY_BCI, CountingProfile.class)).counter++; + } + + public long getProfileEntryCount() { + return ((CountingProfile) getAtBCI(JVMCI_METHOD_ENTRY_BCI, CountingProfile.class)).counter; + } + + public void profileBranch(int bci, boolean taken) { + if (taken) { + ((BranchProfile) getAtBCI(bci, BranchProfile.class)).incrementTakenCounter(); + } else { + ((BranchProfile) getAtBCI(bci, BranchProfile.class)).incrementNotTakenCounter(); + } + } + + public double getBranchTakenProbability(int bci) { + return ((BranchProfile) getAtBCI(bci, BranchProfile.class)).takenProfile(); + } + + /** + * Get the given profile at the current {@code bci} that's class equals the {@code clazz} + * given. There can be multiple profiles per bci, we want the one that matches the given clazz. + * Beware: it is not checked nor guaranteed that there is only a single profile which's class + * equals the given one. This is guaranteed by construction. + */ + private InterpreterProfile getAtBCI(int bci, Class clazz) { + if (LOG_PROFILE_MACHINERY) { + Log.log().string("getAtBCI method=").string(method.toString()).string(" bci=").unsigned(bci).string(" clazz=").string(clazz.getName()).string(" profilesArray=") + .string(Arrays.toString(profiles)).string(" lastIndex=").unsigned(lastIndex).newline(); + } + InterpreterProfile lastUsedProfile = profiles[lastIndex]; + if (lastUsedProfile == null || lastUsedProfile.getBci() != bci || lastUsedProfile.getClass() != clazz) { + if (LOG_PROFILE_MACHINERY) { + Log.log().string("\tlastUsedProfile was ").string(lastUsedProfile.toString()).string(" but that is not matching the bci or class").newline(); + } + // search for the correct profile + for (int i = 0; i < profiles.length; i++) { + InterpreterProfile profile = profiles[i]; + // make sure lastUsedProfile == null if we did not find a match + if (profile.getBci() == bci && profile.getClass() == clazz) { + if (LOG_PROFILE_MACHINERY) { + Log.log().string("\tFound profile with bci and class ").string(profile.toString()).string(" setting index to ").unsigned(i).newline(); + } + lastUsedProfile = profile; + lastIndex = i; + break; + } else { + lastUsedProfile = null; + } + } + } else { + if (LOG_PROFILE_MACHINERY) { + Log.log().string("\tReturning cached lastUsedProfile ").string(lastUsedProfile.toString()).newline(); + } + } + // found, probably the last entry used it as well + return lastUsedProfile; + } + + public static class TestingBackdoor { + public static List profilesAtBCI(InterpreterProfileContainer methodProfile, int bci) { + ArrayList profiles = new ArrayList<>(); + for (int i = 0; i < methodProfile.profiles.length; i++) { + InterpreterProfile profile = methodProfile.profiles[i]; + if (profile.getBci() == bci) { + profiles.add(profile); + } + } + return profiles; + } + } + + public abstract static class InterpreterProfile { + protected InterpreterProfile next; + protected final int bci; + + protected InterpreterProfile(int bci) { + this.bci = bci; + } + + public void setNext(InterpreterProfile next) { + this.next = next; + } + + public InterpreterProfile getNext() { + return next; + } + + public int getBci() { + return bci; + } + } + + public static class CountingProfile extends InterpreterProfile { + protected long counter; + + CountingProfile(int bci) { + super(bci); + } + + public long getCounter() { + return counter; + } + + public void incrementCounter() { + counter++; + } + + @Override + public String toString() { + return "{Counting:bci=" + bci + ", counter=" + counter + "}"; + } + } + + public static class BranchProfile extends CountingProfile { + private long takenCounter; + + public BranchProfile(int bci) { + super(bci); + } + + public void incrementTakenCounter() { + takenCounter++; + counter++; + } + + public void incrementNotTakenCounter() { + counter++; + } + + public double takenProfile() { + if (counter == 0) { + return -1; + } + return (double) takenCounter / (double) counter; + } + + public double notTakenProfile() { + if (counter == 0) { + return -1; + } + return 1D - takenProfile(); + } + + @Override + public String toString() { + return "{BranchProfile:bci=" + bci + ", takenCounter=" + takenCounter + ", counter=" + counter + "}"; + } + } + + /** + * Artificial byte code index for the method entry profile. + */ + private static final int JVMCI_METHOD_ENTRY_BCI = -1; + + /** + * Special Java bytecode marker value for method entry that is not actually a bytecode. Used to + * enumerate all operations that can be encountered in Crema that may need a profile. + */ + private static final int NON_BYTECODE_METHOD_ENTRY = 0xDEAD0001; + + /** + * Special Java bytecode marker value for a loop backedge that is not actually a bytecode. Used + * to enumerate all operations that can be encountered in Crema that may need a profile. + */ + private static final int NON_BYTECODE_LOOP_BACKEDGE = 0xDEAD0002; + + /** + * All Java bytecodes that somehow represent a branching structure and thus need branch + * profiles. + */ + public static final int[] BRANCH_PROFILED_BYTECODES = {Bytecodes.IFEQ, + Bytecodes.IFNE, + Bytecodes.IFLT, + Bytecodes.IFLE, + Bytecodes.IFGT, + Bytecodes.IFGE, + Bytecodes.IF_ICMPEQ, + Bytecodes.IF_ICMPNE, + Bytecodes.IF_ICMPLT, + Bytecodes.IF_ICMPLE, + Bytecodes.IF_ICMPGT, + Bytecodes.IF_ICMPGE, + Bytecodes.IF_ACMPEQ, + Bytecodes.IF_ACMPNE, + Bytecodes.IFNULL, + Bytecodes.IFNONNULL, + Bytecodes.TABLESWITCH, + Bytecodes.LOOKUPSWITCH + }; + + /** + * All Java bytecodes that somehow represent a type based operation and thus need type profiles. + */ + public static final int[] TYPE_PROFILED_BYTECODES = { + Bytecodes.INVOKEVIRTUAL, + Bytecodes.INVOKEINTERFACE, + Bytecodes.INVOKEDYNAMIC, + Bytecodes.INVOKESTATIC, + Bytecodes.INVOKESPECIAL, + Bytecodes.CHECKCAST, + Bytecodes.INSTANCEOF, + Bytecodes.AALOAD, + Bytecodes.AASTORE + }; + + /** + * All Java bytecodes that represent special operations that should be counted. + */ + public static final int[] COUNTING_PROFILED_BYTECODES = {NON_BYTECODE_METHOD_ENTRY, NON_BYTECODE_LOOP_BACKEDGE, Bytecodes.GOTO, Bytecodes.GOTO_W}; + + public static final int[] ALL_PROFILED_BYTECODES; + + static { + int size = BRANCH_PROFILED_BYTECODES.length + TYPE_PROFILED_BYTECODES.length + COUNTING_PROFILED_BYTECODES.length; + ALL_PROFILED_BYTECODES = new int[size]; + System.arraycopy(BRANCH_PROFILED_BYTECODES, 0, ALL_PROFILED_BYTECODES, 0, BRANCH_PROFILED_BYTECODES.length); + System.arraycopy(TYPE_PROFILED_BYTECODES, 0, ALL_PROFILED_BYTECODES, BRANCH_PROFILED_BYTECODES.length, TYPE_PROFILED_BYTECODES.length); + System.arraycopy(COUNTING_PROFILED_BYTECODES, 0, ALL_PROFILED_BYTECODES, BRANCH_PROFILED_BYTECODES.length + TYPE_PROFILED_BYTECODES.length, COUNTING_PROFILED_BYTECODES.length); + } + +} diff --git a/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/Interpreter.java b/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/Interpreter.java index f0f43def4dab..c801de2f1b2f 100644 --- a/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/Interpreter.java +++ b/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/Interpreter.java @@ -622,10 +622,7 @@ public static final class Root { @NeverInline("needed far stack walking") private static Object executeBodyFromBCI(InterpreterFrame frame, InterpreterResolvedJavaMethod method, int startBCI, int startTop, boolean forceStayInInterpreter) { - - if (RistrettoProfileSupport.isEnabled()) { - RistrettoProfileSupport.profileMethodCall(method); - } + RistrettoProfileSupport.profileMethodCall(method); int curBCI = startBCI; int top = startTop; @@ -889,9 +886,12 @@ private static Object executeBodyFromBCI(InterpreterFrame frame, InterpreterReso case IFGT: // fall through case IFLE: if (takeBranchPrimitive1(popInt(frame, top - 1), curOpcode)) { + RistrettoProfileSupport.profileIfBranch(method, curBCI, true); top += ConstantBytecodes.stackEffectOf(IFLE); curBCI = beforeJumpChecks(frame, curBCI, BytecodeStream.readBranchDest2(code, curBCI), top); continue loop; + } else { + RistrettoProfileSupport.profileIfBranch(method, curBCI, false); } break; @@ -902,27 +902,36 @@ private static Object executeBodyFromBCI(InterpreterFrame frame, InterpreterReso case IF_ICMPGT: // fall through case IF_ICMPLE: if (takeBranchPrimitive2(popInt(frame, top - 1), popInt(frame, top - 2), curOpcode)) { + RistrettoProfileSupport.profileIfBranch(method, curBCI, true); top += ConstantBytecodes.stackEffectOf(IF_ICMPLE); curBCI = beforeJumpChecks(frame, curBCI, BytecodeStream.readBranchDest2(code, curBCI), top); continue loop; + } else { + RistrettoProfileSupport.profileIfBranch(method, curBCI, false); } break; case IF_ACMPEQ: // fall through case IF_ACMPNE: if (takeBranchRef2(popObject(frame, top - 1), popObject(frame, top - 2), curOpcode)) { + RistrettoProfileSupport.profileIfBranch(method, curBCI, true); top += ConstantBytecodes.stackEffectOf(IF_ACMPNE); curBCI = beforeJumpChecks(frame, curBCI, BytecodeStream.readBranchDest2(code, curBCI), top); continue loop; + } else { + RistrettoProfileSupport.profileIfBranch(method, curBCI, false); } break; case IFNULL: // fall through case IFNONNULL: if (takeBranchRef1(popObject(frame, top - 1), curOpcode)) { + RistrettoProfileSupport.profileIfBranch(method, curBCI, true); top += ConstantBytecodes.stackEffectOf(IFNONNULL); curBCI = beforeJumpChecks(frame, curBCI, BytecodeStream.readBranchDest2(code, curBCI), top); continue loop; + } else { + RistrettoProfileSupport.profileIfBranch(method, curBCI, false); } break; @@ -1280,6 +1289,7 @@ private static void arrayStore(InterpreterFrame frame, int top, int storeOpcode) private static int beforeJumpChecks(InterpreterFrame frame, int curBCI, int targetBCI, int top) { if (targetBCI <= curBCI) { // GR-55055: Safepoint poll needed? + // TODO GR-71799 - add ristretto backedge profiles } return targetBCI; } diff --git a/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/RistrettoRuntimeOptions.java b/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/RistrettoRuntimeOptions.java index 5069b6f20245..65b35d9e139f 100644 --- a/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/RistrettoRuntimeOptions.java +++ b/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/RistrettoRuntimeOptions.java @@ -46,4 +46,7 @@ public class RistrettoRuntimeOptions { @Option(help = "Trace compilation events.")// public static final RuntimeOptionKey JITTraceCompilation = new RuntimeOptionKey<>(false); + + @Option(help = "Number of invocations before profiling considers a method profile to be mature.")// + public static final RuntimeOptionKey JITProfileMatureInvocationThreshold = new RuntimeOptionKey<>(1000); } diff --git a/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/RistrettoUtils.java b/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/RistrettoUtils.java index 722d89473b90..07ab99b19407 100644 --- a/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/RistrettoUtils.java +++ b/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/RistrettoUtils.java @@ -25,6 +25,7 @@ package com.oracle.svm.interpreter.ristretto; import java.io.PrintStream; +import java.util.Optional; import org.graalvm.collections.EconomicMap; import org.graalvm.nativeimage.ImageSingletons; @@ -47,6 +48,7 @@ import com.oracle.svm.interpreter.metadata.InterpreterResolvedJavaMethod; import com.oracle.svm.interpreter.ristretto.compile.RistrettoGraphBuilderPhase; import com.oracle.svm.interpreter.ristretto.meta.RistrettoMethod; +import com.oracle.svm.interpreter.ristretto.profile.RistrettoProfileProvider; import jdk.graal.compiler.code.CompilationResult; import jdk.graal.compiler.core.CompilationWatchDog; @@ -54,6 +56,7 @@ import jdk.graal.compiler.debug.DebugContext; import jdk.graal.compiler.debug.GraalError; import jdk.graal.compiler.lir.phases.LIRSuites; +import jdk.graal.compiler.nodes.GraphState; import jdk.graal.compiler.nodes.StructuredGraph; import jdk.graal.compiler.nodes.graphbuilderconf.GraphBuilderConfiguration; import jdk.graal.compiler.nodes.spi.ProfileProvider; @@ -61,17 +64,18 @@ import jdk.graal.compiler.options.OptionKey; import jdk.graal.compiler.options.OptionValues; import jdk.graal.compiler.phases.OptimisticOptimizations; +import jdk.graal.compiler.phases.Phase; +import jdk.graal.compiler.phases.common.HighTierLoweringPhase; +import jdk.graal.compiler.phases.common.LowTierLoweringPhase; +import jdk.graal.compiler.phases.common.MidTierLoweringPhase; import jdk.graal.compiler.phases.tiers.HighTierContext; import jdk.graal.compiler.phases.tiers.Suites; import jdk.graal.compiler.phases.util.Providers; import jdk.graal.compiler.printer.GraalDebugHandlersFactory; import jdk.vm.ci.code.InstalledCode; -import jdk.vm.ci.meta.DefaultProfilingInfo; -import jdk.vm.ci.meta.ProfilingInfo; import jdk.vm.ci.meta.ResolvedJavaMethod; import jdk.vm.ci.meta.ResolvedJavaType; import jdk.vm.ci.meta.SpeculationLog; -import jdk.vm.ci.meta.TriState; public class RistrettoUtils { @@ -157,20 +161,6 @@ public static InstalledCode compileAndInstall(SubstrateMethod method, SubstrateI return (InstalledCode) installedCode; } - private static final ProfileProvider NO_PROFILE_PROVIDER = new ProfileProvider() { - - @Override - public ProfilingInfo getProfilingInfo(ResolvedJavaMethod method) { - return getProfilingInfo(method, true, true); - } - - @Override - public ProfilingInfo getProfilingInfo(ResolvedJavaMethod method, boolean includeNormal, boolean includeOSR) { - return DefaultProfilingInfo.get(TriState.FALSE); - } - - }; - public static CompilationResult doCompile(DebugContext initialDebug, RuntimeConfiguration runtimeConfig, LIRSuites lirSuites, SubstrateMethod method) { SubstrateGraalUtils.updateGraalArchitectureWithHostCPUFeatures(runtimeConfig.lookupBackend(method)); @@ -202,13 +192,11 @@ protected CompilationResult performCompilation(DebugContext debug) { try (CompilationWatchDog _ = CompilationWatchDog.watch(compilationId, debug.getOptions(), false, SubstrateGraalUtils.COMPILATION_WATCH_DOG_EVENT_HANDLER, null)) { StructuredGraph graph; Suites suites; - if (method instanceof RistrettoMethod) { + if (method instanceof RistrettoMethod rMethod) { final OptionValues options = debug.getOptions(); // final int entryBCI = 0; final SpeculationLog speculationLog = new SubstrateSpeculationLog(); - // TODO GR-71495 - add branch profiles and combine profile provider with - // crema profiles - final ProfileProvider profileProvider = NO_PROFILE_PROVIDER; + final ProfileProvider profileProvider = new RistrettoProfileProvider(rMethod); final StructuredGraph.AllowAssumptions allowAssumptions = StructuredGraph.AllowAssumptions.NO; // TODO GR-71494 - OSR support will require setting the entry BCI for // parsing @@ -217,6 +205,11 @@ protected CompilationResult performCompilation(DebugContext debug) { assert graph != null; suites = RuntimeCompilationSupport.getMatchingSuitesForGraph(graph); parseFromBytecode(graph, runtimeConfig); + if (TestingBackdoor.shouldRememberGraph()) { + // override the suites with graph capturing phases + suites = suites.copy(); + TestingBackdoor.installLastGraphThieves(suites, graph); + } } else { graph = RuntimeCompilationSupport.decodeGraph(debug, null, compilationId, method, null); suites = RuntimeCompilationSupport.getMatchingSuitesForGraph(graph); @@ -255,4 +248,60 @@ private static void parseFromBytecode(StructuredGraph graph, RuntimeConfiguratio assert graph.getNodeCount() > 1 : "Must have nodes after parsing"; } + public static final class TestingBackdoor { + private TestingBackdoor() { + // this type should never be allocated + } + + /** + * Export access to the last compiled graph of a ristretto compile to be able to white box + * test various properties. + */ + public static final ThreadLocal LastCompiledGraph = new ThreadLocal<>(); + public static final ThreadLocal LastCompiledGraphAfterHighTierLowering = new ThreadLocal<>(); + public static final ThreadLocal LastCompiledGraphAfterMidTierLowering = new ThreadLocal<>(); + public static final ThreadLocal LastCompiledGraphAfterLowTierLowering = new ThreadLocal<>(); + + public static boolean shouldRememberGraph() { + String prop = System.getProperty("com.oracle.svm.interpreter.ristretto.RistrettoUtils.PreserveLastCompiledGraph", "false"); + return Boolean.parseBoolean(prop); + } + + static void installLastGraphThieves(Suites suites, StructuredGraph rootGraph) { + TestingBackdoor.LastCompiledGraph.set(rootGraph); + suites.getHighTier().insertAfterPhase(HighTierLoweringPhase.class, new Phase() { + @Override + public Optional notApplicableTo(GraphState graphState) { + return ALWAYS_APPLICABLE; + } + + @Override + protected void run(StructuredGraph graph) { + TestingBackdoor.LastCompiledGraphAfterHighTierLowering.set((StructuredGraph) graph.copy(graph.getDebug())); + } + }); + suites.getMidTier().insertAfterPhase(MidTierLoweringPhase.class, new Phase() { + @Override + public Optional notApplicableTo(GraphState graphState) { + return ALWAYS_APPLICABLE; + } + + @Override + protected void run(StructuredGraph graph) { + TestingBackdoor.LastCompiledGraphAfterMidTierLowering.set((StructuredGraph) graph.copy(graph.getDebug())); + } + }); + suites.getLowTier().insertAfterPhase(LowTierLoweringPhase.class, new Phase() { + @Override + public Optional notApplicableTo(GraphState graphState) { + return ALWAYS_APPLICABLE; + } + + @Override + protected void run(StructuredGraph graph) { + TestingBackdoor.LastCompiledGraphAfterLowTierLowering.set((StructuredGraph) graph.copy(graph.getDebug())); + } + }); + } + } } diff --git a/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/meta/RistrettoMethod.java b/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/meta/RistrettoMethod.java index 78f4b7852b7e..09e6845c55dc 100644 --- a/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/meta/RistrettoMethod.java +++ b/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/meta/RistrettoMethod.java @@ -29,6 +29,7 @@ import com.oracle.svm.graal.meta.SubstrateMethod; import com.oracle.svm.graal.meta.SubstrateType; import com.oracle.svm.interpreter.metadata.InterpreterResolvedJavaMethod; +import com.oracle.svm.interpreter.metadata.profile.InterpreterProfileContainer; import com.oracle.svm.interpreter.ristretto.RistrettoConstants; import com.oracle.svm.interpreter.ristretto.RistrettoUtils; @@ -58,10 +59,11 @@ public final class RistrettoMethod extends SubstrateMethod { // JIT COMPILER SUPPORT START /** - * Field exposed for profiling support for this method. May be written and read in a - * multithreaded fashion. + * Field exposed for profiling support for this method. Initialized once upon first profiling + * under heavy synchronization. Never written again. If a ristretto method is GCed profile is + * lost. */ - public volatile Object profile; + private InterpreterProfileContainer profile; /** * State-machine for compilation handling of this crema method. Every methods starts in a * NEVER_COMPILED state and than can cycle through different states. @@ -95,6 +97,23 @@ public static RistrettoMethod create(InterpreterResolvedJavaMethod interpreterMe return (RistrettoMethod) interpreterMethod.getRistrettoMethod(RISTRETTO_METHOD_FUNCTION); } + public InterpreterProfileContainer getProfile() { + if (profile == null) { + /* + * Allocate the profile per method once. Note that out of test scenarios this is never + * done and thus the heavy locking code below is normally not run. + */ + synchronized (this) { + profile = new InterpreterProfileContainer(this); + } + } + return profile; + } + + public synchronized void resetProfile() { + profile = null; + } + @Override public boolean canBeInlined() { final boolean wasCompiledAOT = RistrettoUtils.wasAOTCompiled(this.getInterpreterMethod()); diff --git a/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/profile/RistrettoCompilationManager.java b/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/profile/RistrettoCompilationManager.java index e89e7d57b97b..eb0d22f68d60 100644 --- a/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/profile/RistrettoCompilationManager.java +++ b/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/profile/RistrettoCompilationManager.java @@ -175,7 +175,7 @@ public static synchronized void reset() { task.getRMethod().installedCode.invalidate(); } task.getRMethod().installedCode = null; - task.getRMethod().profile = null; + task.getRMethod().resetProfile(); } m.performedCompilations.clear(); m.submittedRequests.set(0); diff --git a/substratevm/src/com.oracle.svm.interpreter.metadata/src/com/oracle/svm/interpreter/metadata/profile/InterpreterProfile.java b/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/profile/RistrettoProfileProvider.java similarity index 58% rename from substratevm/src/com.oracle.svm.interpreter.metadata/src/com/oracle/svm/interpreter/metadata/profile/InterpreterProfile.java rename to substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/profile/RistrettoProfileProvider.java index d61778ce21ed..9e969b0b9fab 100644 --- a/substratevm/src/com.oracle.svm.interpreter.metadata/src/com/oracle/svm/interpreter/metadata/profile/InterpreterProfile.java +++ b/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/profile/RistrettoProfileProvider.java @@ -22,15 +22,31 @@ * or visit www.oracle.com if you need additional information or have any * questions. */ -package com.oracle.svm.interpreter.metadata.profile; +package com.oracle.svm.interpreter.ristretto.profile; -/** - * Abstract base representation of profile data for crema. - */ -public abstract class InterpreterProfile { +import com.oracle.svm.interpreter.ristretto.meta.RistrettoMethod; + +import jdk.graal.compiler.nodes.spi.ProfileProvider; +import jdk.vm.ci.meta.ProfilingInfo; +import jdk.vm.ci.meta.ResolvedJavaMethod; + +public final class RistrettoProfileProvider implements ProfileProvider { + private final RistrettoProfilingInfo info; - public static class CountingProfile extends InterpreterProfile { - public long counter; + public RistrettoProfileProvider(RistrettoMethod rMethod) { + this.info = new RistrettoProfilingInfo(rMethod.getProfile()); } + @Override + public ProfilingInfo getProfilingInfo(ResolvedJavaMethod method) { + return info; + } + + @Override + public ProfilingInfo getProfilingInfo(ResolvedJavaMethod method, boolean includeNormal, boolean includeOSR) { + /* + * TODO GR-71494 - no OSR support for now + */ + return getProfilingInfo(method); + } } diff --git a/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/profile/RistrettoProfileSupport.java b/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/profile/RistrettoProfileSupport.java index 729f52f99c69..1a7992d21187 100644 --- a/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/profile/RistrettoProfileSupport.java +++ b/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/profile/RistrettoProfileSupport.java @@ -33,7 +33,7 @@ import com.oracle.svm.core.option.RuntimeOptionKey; import com.oracle.svm.interpreter.metadata.CremaResolvedJavaMethodImpl; import com.oracle.svm.interpreter.metadata.InterpreterResolvedJavaMethod; -import com.oracle.svm.interpreter.metadata.profile.InterpreterProfile; +import com.oracle.svm.interpreter.metadata.profile.InterpreterProfileContainer; import com.oracle.svm.interpreter.ristretto.RistrettoConstants; import com.oracle.svm.interpreter.ristretto.RistrettoFeature; import com.oracle.svm.interpreter.ristretto.RistrettoRuntimeOptions; @@ -58,6 +58,67 @@ public static void trace(RuntimeOptionKey optionKey, String msg, Object } } + public static void profileIfBranch(InterpreterResolvedJavaMethod iMethod, int bci, boolean taken) { + if (!RistrettoProfileSupport.isEnabled()) { + return; + } + + assert iMethod instanceof CremaResolvedJavaMethodImpl; + final RistrettoMethod rMethod = RistrettoMethod.create(iMethod); + + int oldState = COMPILATION_STATE_UPDATER.get(rMethod); + if (!RistrettoCompileStateMachine.shouldEnterProfiling(oldState)) { + // no need to keep profiling this code, we are done + trace(RistrettoRuntimeOptions.JITTraceCompilationQueuing, "[Ristretto Compile Queue]Should not enter profiling for method %s because of state %s%n", iMethod, + RistrettoCompileStateMachine.toString(oldState)); + return; + } + + // this point is only reached for state=INIT_VAL|INITIALIZING|NEVER_COMPILED + while (true) { + oldState = COMPILATION_STATE_UPDATER.get(rMethod); + + switch (oldState) { + /* + * Profile has not been initialized yet for this method, given we are profiling a + * branch we must have profiled the method entry already, thus this is a hard error. + */ + case RistrettoConstants.COMPILE_STATE_INIT_VAL: { + throw GraalError.shouldNotReachHere(String.format("Reached a method without an initialized profile when trying to profile a branch. " + + "This must not happen. Every profile must be created and initially written when profiling the entry of a method in the interpreter. " + + "Method=%s", rMethod)); + } + case RistrettoConstants.COMPILE_STATE_SUBMITTED: + case RistrettoConstants.COMPILE_STATE_COMPILED: { + profileSkipCase(iMethod, rMethod); + return; + } + case RistrettoConstants.COMPILE_STATE_NEVER_COMPILED: { + + InterpreterProfileContainer methodProfile = rMethod.getProfile(); + trace(RistrettoRuntimeOptions.JITTraceProfilingIncrements, String.format("[Ristretto Compile Queue]Entering state %s for %s, counter=%s%n", + RistrettoCompileStateMachine.toString(COMPILATION_STATE_UPDATER.get(rMethod)), iMethod, methodProfile.getProfileEntryCount())); + /* + * We write without any synchronization to the branch profile value at the cost + * of lost updates. + */ + methodProfile.profileBranch(bci, taken); + + // we only increment (and submit if applicable) once, thus return now + return; + } + case RistrettoConstants.COMPILE_STATE_INITIALIZING: { + // another thread is initializing the compilation data, do a few more spins + // until that is done and then go on + break; + } + default: + throw GraalError.shouldNotReachHere("Unknown state " + oldState); + } + } + + } + /** * Profiles a method call and manages the compilation submission process for the Ristretto * interpreter. This method implements a thread-safe state machine that handles the lifecycle of @@ -91,6 +152,10 @@ public static void trace(RuntimeOptionKey optionKey, String msg, Object * @throws AssertionError if iMethod is not a InterpreterResolvedJavaMethod instance */ public static void profileMethodCall(InterpreterResolvedJavaMethod iMethod) { + if (!RistrettoProfileSupport.isEnabled()) { + return; + } + if (!RistrettoRuntimeOptions.JITEnableCompilation.getValue()) { return; } @@ -126,7 +191,7 @@ public static void profileMethodCall(InterpreterResolvedJavaMethod iMethod) { } case RistrettoConstants.COMPILE_STATE_SUBMITTED: case RistrettoConstants.COMPILE_STATE_COMPILED: { - methodEntrySkipCase(iMethod, rMethod); + profileSkipCase(iMethod, rMethod); return; } case RistrettoConstants.COMPILE_STATE_NEVER_COMPILED: { @@ -146,12 +211,14 @@ public static void profileMethodCall(InterpreterResolvedJavaMethod iMethod) { } private static void methodEntryNeverCompiledCase(InterpreterResolvedJavaMethod iMethod, RistrettoMethod rMethod, int oldState) { - InterpreterProfile.CountingProfile methodProfile = (InterpreterProfile.CountingProfile) rMethod.profile; + InterpreterProfileContainer methodProfile = rMethod.getProfile(); trace(RistrettoRuntimeOptions.JITTraceProfilingIncrements, String.format("[Ristretto Compile Queue]Entering state %s for %s, counter=%s%n", - RistrettoCompileStateMachine.toString(COMPILATION_STATE_UPDATER.get(rMethod)), iMethod, methodProfile.counter)); - // we write without any synchronization to the methodProfile.counter value at - // the cost of lost updates - if (++methodProfile.counter > RistrettoRuntimeOptions.JITCompilerInvocationThreshold.getValue()) { + RistrettoCompileStateMachine.toString(COMPILATION_STATE_UPDATER.get(rMethod)), iMethod, methodProfile.getProfileEntryCount())); + /* + * We write without any synchronization to the methodProfile.counter value at the cost of + * lost updates. + */ + if (methodProfile.profileMethodEntry() > RistrettoRuntimeOptions.JITCompilerInvocationThreshold.getValue()) { trace(RistrettoRuntimeOptions.JITTraceCompilationQueuing, "[Ristretto Compile Queue]Entering state %s for %s, profile overflown, trying to submit compile%n", RistrettoCompileStateMachine.toString(oldState), iMethod); while (!COMPILATION_STATE_UPDATER.compareAndSet(rMethod, RistrettoConstants.COMPILE_STATE_NEVER_COMPILED, RistrettoConstants.COMPILE_STATE_SUBMITTED)) { @@ -165,7 +232,7 @@ private static void methodEntryNeverCompiledCase(InterpreterResolvedJavaMethod i } } - private static void methodEntrySkipCase(InterpreterResolvedJavaMethod iMethod, RistrettoMethod rMethod) { + private static void profileSkipCase(InterpreterResolvedJavaMethod iMethod, RistrettoMethod rMethod) { // in the meantime compilation happened already, we are done trace(RistrettoRuntimeOptions.JITTraceCompilationQueuing, "[Ristretto Compile Queue]Entering state %s for %s, skipping any profiling or compilation%n", RistrettoCompileStateMachine.toString(COMPILATION_STATE_UPDATER.get(rMethod)), iMethod); @@ -178,9 +245,6 @@ private static void methodEntryInitCase(InterpreterResolvedJavaMethod iMethod, R if (COMPILATION_STATE_UPDATER.compareAndSet(rMethod, RistrettoConstants.COMPILE_STATE_INIT_VAL, RistrettoConstants.COMPILE_STATE_INITIALIZING)) { trace(RistrettoRuntimeOptions.JITTraceCompilationQueuing, "[Ristretto Compile Queue]Entering state %s for %s%n", RistrettoCompileStateMachine.toString(COMPILATION_STATE_UPDATER.get(rMethod)), iMethod); - - // we are the one writing - rMethod.profile = new InterpreterProfile.CountingProfile(); while (!COMPILATION_STATE_UPDATER.compareAndSet(rMethod, RistrettoConstants.COMPILE_STATE_INITIALIZING, RistrettoConstants.COMPILE_STATE_NEVER_COMPILED)) { // spin until we are done writing PauseNode.pause(); diff --git a/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/profile/RistrettoProfilingInfo.java b/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/profile/RistrettoProfilingInfo.java new file mode 100644 index 000000000000..bf93036aaec6 --- /dev/null +++ b/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/profile/RistrettoProfilingInfo.java @@ -0,0 +1,119 @@ +/* + * Copyright (c) 2025, 2025, 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. Oracle 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. + * + * 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 com.oracle.svm.interpreter.ristretto.profile; + +import com.oracle.svm.interpreter.metadata.profile.InterpreterProfileContainer; +import com.oracle.svm.interpreter.ristretto.RistrettoRuntimeOptions; + +import jdk.graal.compiler.debug.Assertions; +import jdk.vm.ci.meta.DeoptimizationReason; +import jdk.vm.ci.meta.JavaMethodProfile; +import jdk.vm.ci.meta.JavaTypeProfile; +import jdk.vm.ci.meta.ProfilingInfo; +import jdk.vm.ci.meta.TriState; + +public class RistrettoProfilingInfo implements ProfilingInfo { + private final InterpreterProfileContainer interpreterProfileContainer; + + public RistrettoProfilingInfo(InterpreterProfileContainer interpreterProfileContainer) { + this.interpreterProfileContainer = interpreterProfileContainer; + } + + @Override + public int getCodeSize() { + return 0; + } + + @Override + public JavaTypeProfile getTypeProfile(int bci) { + return null; + } + + @Override + public JavaMethodProfile getMethodProfile(int bci) { + return null; + } + + @Override + public double getBranchTakenProbability(int bci) { + final double takenProfile = interpreterProfileContainer.getBranchTakenProbability(bci); + assert !Double.isNaN(takenProfile) && !Double.isNaN(takenProfile) : Assertions.errorMessage("Must have sane value", takenProfile, interpreterProfileContainer.getMethod(), + InterpreterProfileContainer.TestingBackdoor.profilesAtBCI(interpreterProfileContainer, bci)); + return takenProfile; + } + + @Override + public double[] getSwitchProbabilities(int bci) { + return null; + } + + @Override + public TriState getExceptionSeen(int bci) { + return TriState.UNKNOWN; + } + + @Override + public TriState getNullSeen(int bci) { + return TriState.UNKNOWN; + } + + @Override + public int getExecutionCount(int bci) { + return -1; + } + + @Override + public int getDeoptimizationCount(DeoptimizationReason reason) { + return 0; + } + + @Override + public boolean isMature() { + /* + * Either maturity was explicitly requested or we follow regular ergonomics. + */ + return interpreterProfileContainer.isMature() || interpreterProfileContainer.getProfileEntryCount() > RistrettoRuntimeOptions.JITProfileMatureInvocationThreshold.getValue(); + } + + @Override + public String toString() { + return "DefaultProfilingInfo<" + this.toString(null, "; ") + ">"; + } + + @Override + public void setMature() { + // Do nothing + } + + @Override + public boolean setCompilerIRSize(Class irType, int nodeCount) { + return false; + } + + @Override + public int getCompilerIRSize(Class irType) { + return -1; + } +} From 13d472d5bc7566ae96b7b7ff074cc3e8711d05ac Mon Sep 17 00:00:00 2001 From: David Leopoldseder Date: Thu, 4 Dec 2025 09:53:08 +0100 Subject: [PATCH 03/18] address review comments --- .../svm/interpreter/metadata/Bytecodes.java | 87 ++++++--- ...ontainer.java => MethodProfilingData.java} | 168 ++++-------------- .../ristretto/meta/RistrettoMethod.java | 12 +- .../profile/RistrettoProfileSupport.java | 6 +- .../profile/RistrettoProfilingInfo.java | 18 +- 5 files changed, 119 insertions(+), 172 deletions(-) rename substratevm/src/com.oracle.svm.interpreter.metadata/src/com/oracle/svm/interpreter/metadata/profile/{InterpreterProfileContainer.java => MethodProfilingData.java} (51%) diff --git a/substratevm/src/com.oracle.svm.interpreter.metadata/src/com/oracle/svm/interpreter/metadata/Bytecodes.java b/substratevm/src/com.oracle.svm.interpreter.metadata/src/com/oracle/svm/interpreter/metadata/Bytecodes.java index dcab28120e7a..20b2c27ad32e 100644 --- a/substratevm/src/com.oracle.svm.interpreter.metadata/src/com/oracle/svm/interpreter/metadata/Bytecodes.java +++ b/substratevm/src/com.oracle.svm.interpreter.metadata/src/com/oracle/svm/interpreter/metadata/Bytecodes.java @@ -31,11 +31,13 @@ import static com.oracle.svm.interpreter.metadata.Bytecodes.Flags.FALL_THROUGH; import static com.oracle.svm.interpreter.metadata.Bytecodes.Flags.FIELD_READ; import static com.oracle.svm.interpreter.metadata.Bytecodes.Flags.FIELD_WRITE; +import static com.oracle.svm.interpreter.metadata.Bytecodes.Flags.IF_BRANCH_PROFILED; import static com.oracle.svm.interpreter.metadata.Bytecodes.Flags.INVOKE; import static com.oracle.svm.interpreter.metadata.Bytecodes.Flags.LOAD; import static com.oracle.svm.interpreter.metadata.Bytecodes.Flags.STOP; import static com.oracle.svm.interpreter.metadata.Bytecodes.Flags.STORE; import static com.oracle.svm.interpreter.metadata.Bytecodes.Flags.TRAP; +import static com.oracle.svm.interpreter.metadata.Bytecodes.Flags.TYPE_PROFILED; import java.lang.reflect.Field; import java.lang.reflect.Modifier; @@ -325,6 +327,14 @@ static class Flags { * Denotes the 4 INVOKE* instructions. */ static final int INVOKE = 0x00001000; + /** + * Denotes all binary branches that are subject to branch profiling. + */ + static final int IF_BRANCH_PROFILED = 0x00002000; + /** + * Denotes all instructions that are subject to type profiling. + */ + static final int TYPE_PROFILED = 0x00004000; } // Performs a sanity check that none of the flags overlap. @@ -423,7 +433,7 @@ static class Flags { def(LALOAD , "laload" , "b" , 0, TRAP); def(FALOAD , "faload" , "b" , -1, TRAP); def(DALOAD , "daload" , "b" , 0, TRAP); - def(AALOAD , "aaload" , "b" , -1, TRAP); + def(AALOAD , "aaload" , "b" , -1, TRAP | TYPE_PROFILED); def(BALOAD , "baload" , "b" , -1, TRAP); def(CALOAD , "caload" , "b" , -1, TRAP); def(SALOAD , "saload" , "b" , -1, TRAP); @@ -456,7 +466,7 @@ static class Flags { def(LASTORE , "lastore" , "b" , -4, TRAP); def(FASTORE , "fastore" , "b" , -3, TRAP); def(DASTORE , "dastore" , "b" , -4, TRAP); - def(AASTORE , "aastore" , "b" , -3, TRAP); + def(AASTORE , "aastore" , "b" , -3, TRAP | TYPE_PROFILED); def(BASTORE , "bastore" , "b" , -3, TRAP); def(CASTORE , "castore" , "b" , -3, TRAP); def(SASTORE , "sastore" , "b" , -3, TRAP); @@ -526,22 +536,22 @@ static class Flags { def(FCMPG , "fcmpg" , "b" , -1); def(DCMPL , "dcmpl" , "b" , -3); def(DCMPG , "dcmpg" , "b" , -3); - def(IFEQ , "ifeq" , "boo" , -1, FALL_THROUGH | BRANCH); - def(IFNE , "ifne" , "boo" , -1, FALL_THROUGH | BRANCH); - def(IFLT , "iflt" , "boo" , -1, FALL_THROUGH | BRANCH); - def(IFGE , "ifge" , "boo" , -1, FALL_THROUGH | BRANCH); - def(IFGT , "ifgt" , "boo" , -1, FALL_THROUGH | BRANCH); - def(IFLE , "ifle" , "boo" , -1, FALL_THROUGH | BRANCH); - def(IF_ICMPEQ , "if_icmpeq" , "boo" , -2, COMMUTATIVE | FALL_THROUGH | BRANCH); - def(IF_ICMPNE , "if_icmpne" , "boo" , -2, COMMUTATIVE | FALL_THROUGH | BRANCH); - def(IF_ICMPLT , "if_icmplt" , "boo" , -2, FALL_THROUGH | BRANCH); - def(IF_ICMPGE , "if_icmpge" , "boo" , -2, FALL_THROUGH | BRANCH); - def(IF_ICMPGT , "if_icmpgt" , "boo" , -2, FALL_THROUGH | BRANCH); - def(IF_ICMPLE , "if_icmple" , "boo" , -2, FALL_THROUGH | BRANCH); - def(IF_ACMPEQ , "if_acmpeq" , "boo" , -2, COMMUTATIVE | FALL_THROUGH | BRANCH); - def(IF_ACMPNE , "if_acmpne" , "boo" , -2, COMMUTATIVE | FALL_THROUGH | BRANCH); - def(GOTO , "goto" , "boo" , 0, STOP | BRANCH); - def(JSR , "jsr" , "boo" , 0, STOP | BRANCH); + def(IFEQ , "ifeq" , "boo" , -1, FALL_THROUGH | BRANCH | IF_BRANCH_PROFILED); + def(IFNE , "ifne" , "boo" , -1, FALL_THROUGH | BRANCH | IF_BRANCH_PROFILED); + def(IFLT , "iflt" , "boo" , -1, FALL_THROUGH | BRANCH | IF_BRANCH_PROFILED); + def(IFGE , "ifge" , "boo" , -1, FALL_THROUGH | BRANCH | IF_BRANCH_PROFILED); + def(IFGT , "ifgt" , "boo" , -1, FALL_THROUGH | BRANCH | IF_BRANCH_PROFILED); + def(IFLE , "ifle" , "boo" , -1, FALL_THROUGH | BRANCH | IF_BRANCH_PROFILED); + def(IF_ICMPEQ , "if_icmpeq" , "boo" , -2, COMMUTATIVE | FALL_THROUGH | BRANCH | IF_BRANCH_PROFILED); + def(IF_ICMPNE , "if_icmpne" , "boo" , -2, COMMUTATIVE | FALL_THROUGH | BRANCH | IF_BRANCH_PROFILED); + def(IF_ICMPLT , "if_icmplt" , "boo" , -2, FALL_THROUGH | BRANCH | IF_BRANCH_PROFILED); + def(IF_ICMPGE , "if_icmpge" , "boo" , -2, FALL_THROUGH | BRANCH | IF_BRANCH_PROFILED); + def(IF_ICMPGT , "if_icmpgt" , "boo" , -2, FALL_THROUGH | BRANCH | IF_BRANCH_PROFILED); + def(IF_ICMPLE , "if_icmple" , "boo" , -2, FALL_THROUGH | BRANCH | IF_BRANCH_PROFILED); + def(IF_ACMPEQ , "if_acmpeq" , "boo" , -2, COMMUTATIVE | FALL_THROUGH | BRANCH | IF_BRANCH_PROFILED); + def(IF_ACMPNE , "if_acmpne" , "boo" , -2, COMMUTATIVE | FALL_THROUGH | BRANCH | IF_BRANCH_PROFILED); + def(GOTO , "goto" , "boo" , 0, STOP | BRANCH | IF_BRANCH_PROFILED); + def(JSR , "jsr" , "boo" , 0, STOP | BRANCH | IF_BRANCH_PROFILED); def(RET , "ret" , "bi" , 0, STOP); def(TABLESWITCH , "tableswitch" , "" , -1, STOP); def(LOOKUPSWITCH , "lookupswitch" , "" , -1, STOP); @@ -555,24 +565,24 @@ static class Flags { def(PUTSTATIC , "putstatic" , "bjj" , -1, TRAP | FIELD_WRITE); def(GETFIELD , "getfield" , "bjj" , 0, TRAP | FIELD_READ); def(PUTFIELD , "putfield" , "bjj" , -2, TRAP | FIELD_WRITE); - def(INVOKEVIRTUAL , "invokevirtual" , "bjj" , -1, TRAP | INVOKE); - def(INVOKESPECIAL , "invokespecial" , "bjj" , -1, TRAP | INVOKE); - def(INVOKESTATIC , "invokestatic" , "bjj" , 0, TRAP | INVOKE); - def(INVOKEINTERFACE , "invokeinterface" , "bjja_", -1, TRAP | INVOKE); - def(INVOKEDYNAMIC , "invokedynamic" , "bjjjj", 0, TRAP | INVOKE); + def(INVOKEVIRTUAL , "invokevirtual" , "bjj" , -1, TRAP | INVOKE | TYPE_PROFILED); + def(INVOKESPECIAL , "invokespecial" , "bjj" , -1, TRAP | INVOKE | TYPE_PROFILED); + def(INVOKESTATIC , "invokestatic" , "bjj" , 0, TRAP | INVOKE | TYPE_PROFILED); + def(INVOKEINTERFACE , "invokeinterface" , "bjja_", -1, TRAP | INVOKE | TYPE_PROFILED); + def(INVOKEDYNAMIC , "invokedynamic" , "bjjjj", 0, TRAP | INVOKE | TYPE_PROFILED); def(NEW , "new" , "bii" , 1, TRAP); def(NEWARRAY , "newarray" , "bc" , 0, TRAP); def(ANEWARRAY , "anewarray" , "bii" , 0, TRAP); def(ARRAYLENGTH , "arraylength" , "b" , 0, TRAP); def(ATHROW , "athrow" , "b" , -1, TRAP | STOP); - def(CHECKCAST , "checkcast" , "bii" , 0, TRAP); - def(INSTANCEOF , "instanceof" , "bii" , 0, TRAP); + def(CHECKCAST , "checkcast" , "bii" , 0, TRAP | TYPE_PROFILED); + def(INSTANCEOF , "instanceof" , "bii" , 0, TRAP | TYPE_PROFILED); def(MONITORENTER , "monitorenter" , "b" , -1, TRAP); def(MONITOREXIT , "monitorexit" , "b" , -1, TRAP); def(WIDE , "wide" , "" , 0); def(MULTIANEWARRAY , "multianewarray" , "biic" , 1, TRAP); - def(IFNULL , "ifnull" , "boo" , -1, FALL_THROUGH | BRANCH); - def(IFNONNULL , "ifnonnull" , "boo" , -1, FALL_THROUGH | BRANCH); + def(IFNULL , "ifnull" , "boo" , -1, FALL_THROUGH | BRANCH | IF_BRANCH_PROFILED); + def(IFNONNULL , "ifnonnull" , "boo" , -1, FALL_THROUGH | BRANCH | IF_BRANCH_PROFILED); def(GOTO_W , "goto_w" , "boooo", 0, STOP | BRANCH); def(JSR_W , "jsr_w" , "boooo", 0, STOP | BRANCH); def(BREAKPOINT , "breakpoint" , "b" , 0, TRAP); @@ -648,6 +658,29 @@ public static boolean isBranch(int opcode) { return (flagsArray[opcode & 0xff] & BRANCH) != 0; } + /** + * Determines if a given opcode is an instruction that is subject to binary branch profiling in + * the interpreter. Binary branches are instructions that have 2 successors - the taken and not + * taken successor. + * + * @param opcode an opcode to test + * @return {@code true} iff {@code opcode} is a binary branch instruction that is profiled + */ + public static boolean isProfiledBranch(int opcode) { + return (flagsArray[opcode & 0xff] & IF_BRANCH_PROFILED) != 0; + } + + /** + * Determines if a given opcode is an instruction that is subject to type profiling. This covers + * all instructions that deal with objects of a dynamic type. + * + * @param opcode an opcode to test + * @return {@code true} iff {@code opcode} is a type profiled branch instruction + */ + public static boolean isTypeProfiled(int opcode) { + return (flagsArray[opcode & 0xff] & TYPE_PROFILED) != 0; + } + /** * Determines if a given opcode denotes a conditional branch. * diff --git a/substratevm/src/com.oracle.svm.interpreter.metadata/src/com/oracle/svm/interpreter/metadata/profile/InterpreterProfileContainer.java b/substratevm/src/com.oracle.svm.interpreter.metadata/src/com/oracle/svm/interpreter/metadata/profile/MethodProfilingData.java similarity index 51% rename from substratevm/src/com.oracle.svm.interpreter.metadata/src/com/oracle/svm/interpreter/metadata/profile/InterpreterProfileContainer.java rename to substratevm/src/com.oracle.svm.interpreter.metadata/src/com/oracle/svm/interpreter/metadata/profile/MethodProfilingData.java index 8258c20a5cfd..f3df0a13beb9 100644 --- a/substratevm/src/com.oracle.svm.interpreter.metadata/src/com/oracle/svm/interpreter/metadata/profile/InterpreterProfileContainer.java +++ b/substratevm/src/com.oracle.svm.interpreter.metadata/src/com/oracle/svm/interpreter/metadata/profile/MethodProfilingData.java @@ -27,72 +27,81 @@ import static jdk.graal.compiler.bytecode.Bytecodes.END; import java.util.ArrayList; -import java.util.Arrays; import java.util.List; -import com.oracle.svm.core.log.Log; import com.oracle.svm.interpreter.metadata.Bytecodes; import jdk.graal.compiler.bytecode.BytecodeStream; +import jdk.vm.ci.meta.ProfilingInfo; import jdk.vm.ci.meta.ResolvedJavaMethod; /** - * Abstract base representation of profile data for crema. + * Abstract base representation of profile data for crema per method. */ -public final class InterpreterProfileContainer { +public final class MethodProfilingData { - /** - * Developer debug only flag to log profile creation and access. - */ - private static final boolean LOG_PROFILE_MACHINERY = false; + private final InterpreterProfile[] profiles; - private InterpreterProfile[] profiles; - /* + /** * Cached entry always starts at 0, if it's a cache miss we will initialize it to the correct * value. */ - private int lastIndex = 0; + private int lastIndex; + private final ResolvedJavaMethod method; + + /** + * Testing override for {@link ProfilingInfo#isMature()}. + *

+ * Normally, maturity is derived ergonomically from the collected profiles (for example, the + * method-entry invocation count) by consumers of this container. When this flag is set to true, + * consumers may treat the profiling information as mature regardless of the collected counts. + * Defaults to false. + */ private boolean isMature; - public InterpreterProfileContainer(ResolvedJavaMethod method) { + public MethodProfilingData(ResolvedJavaMethod method) { this.method = method; - buildProfiles(); + this.profiles = buildProfiles(method); } - private void buildProfiles() { + private static InterpreterProfile[] buildProfiles(ResolvedJavaMethod method) { BytecodeStream stream = new BytecodeStream(method.getCode()); stream.setBCI(0); - ArrayList allProfiles = new ArrayList<>(); + List allProfiles = new ArrayList<>(); // we always add a method entry counting profile allProfiles.add(new CountingProfile(JVMCI_METHOD_ENTRY_BCI)); while (stream.currentBC() != END) { int bci = stream.currentBCI(); - int bc = stream.currentBC(); - // in theory, we can have multiple profiles for a single BCI, type, exception etc - if (inArray(BRANCH_PROFILED_BYTECODES, bc)) { - // allocate a branch profile for the bci + int opcode = stream.currentBC(); + // we can have multiple profiles for a single BCI: type, exception etc + if (Bytecodes.isProfiledBranch(opcode)) { allProfiles.add(new BranchProfile(bci)); } - if (inArray(TYPE_PROFILED_BYTECODES, bc)) { - // allocate a type profile + if (Bytecodes.isTypeProfiled(opcode)) { // TODO GR-71567 } - // TODO - backedge / goto profiles + // TODO GR-71799 - backedge / goto profiles stream.next(); } - if (LOG_PROFILE_MACHINERY) { - Log.log().string("Assigning profiles ").string(Arrays.toString(allProfiles.toArray())).string(" to method ").string(method.toString()).newline(); - } - profiles = allProfiles.toArray(new InterpreterProfile[0]); + return allProfiles.toArray(new InterpreterProfile[0]); } public ResolvedJavaMethod getMethod() { return method; } + /** + * Similar semantic as {@link ProfilingInfo#isMature()} except this method does not perform an + * ergonomic decision. A profile is only mature if it was explicitly set with + * {@link #setMature(boolean)}. Normally this is done by test code for example. Users of this + * {@link MethodProfilingData} can combine this with real ergonomics. + * + * @return true if an explicit maturity override has been set on this profiling data; false + * otherwise + */ public boolean isMature() { return isMature; } @@ -101,15 +110,6 @@ public void setMature(boolean mature) { isMature = mature; } - private static boolean inArray(int[] arr, int key) { - for (int i : arr) { - if (i == key) { - return true; - } - } - return false; - } - public long profileMethodEntry() { return ((CountingProfile) getAtBCI(JVMCI_METHOD_ENTRY_BCI, CountingProfile.class)).counter++; } @@ -131,29 +131,18 @@ public double getBranchTakenProbability(int bci) { } /** - * Get the given profile at the current {@code bci} that's class equals the {@code clazz} - * given. There can be multiple profiles per bci, we want the one that matches the given clazz. - * Beware: it is not checked nor guaranteed that there is only a single profile which's class - * equals the given one. This is guaranteed by construction. + * Gets the profile for {@code bci} whose class is {@code clazz}. + * + * @return null if there's no prof */ private InterpreterProfile getAtBCI(int bci, Class clazz) { - if (LOG_PROFILE_MACHINERY) { - Log.log().string("getAtBCI method=").string(method.toString()).string(" bci=").unsigned(bci).string(" clazz=").string(clazz.getName()).string(" profilesArray=") - .string(Arrays.toString(profiles)).string(" lastIndex=").unsigned(lastIndex).newline(); - } InterpreterProfile lastUsedProfile = profiles[lastIndex]; if (lastUsedProfile == null || lastUsedProfile.getBci() != bci || lastUsedProfile.getClass() != clazz) { - if (LOG_PROFILE_MACHINERY) { - Log.log().string("\tlastUsedProfile was ").string(lastUsedProfile.toString()).string(" but that is not matching the bci or class").newline(); - } // search for the correct profile for (int i = 0; i < profiles.length; i++) { InterpreterProfile profile = profiles[i]; // make sure lastUsedProfile == null if we did not find a match if (profile.getBci() == bci && profile.getClass() == clazz) { - if (LOG_PROFILE_MACHINERY) { - Log.log().string("\tFound profile with bci and class ").string(profile.toString()).string(" setting index to ").unsigned(i).newline(); - } lastUsedProfile = profile; lastIndex = i; break; @@ -161,17 +150,13 @@ private InterpreterProfile getAtBCI(int bci, Class lastUsedProfile = null; } } - } else { - if (LOG_PROFILE_MACHINERY) { - Log.log().string("\tReturning cached lastUsedProfile ").string(lastUsedProfile.toString()).newline(); - } } // found, probably the last entry used it as well return lastUsedProfile; } public static class TestingBackdoor { - public static List profilesAtBCI(InterpreterProfileContainer methodProfile, int bci) { + public static List profilesAtBCI(MethodProfilingData methodProfile, int bci) { ArrayList profiles = new ArrayList<>(); for (int i = 0; i < methodProfile.profiles.length; i++) { InterpreterProfile profile = methodProfile.profiles[i]; @@ -184,21 +169,12 @@ public static List profilesAtBCI(InterpreterProfileContainer } public abstract static class InterpreterProfile { - protected InterpreterProfile next; protected final int bci; protected InterpreterProfile(int bci) { this.bci = bci; } - public void setNext(InterpreterProfile next) { - this.next = next; - } - - public InterpreterProfile getNext() { - return next; - } - public int getBci() { return bci; } @@ -266,70 +242,4 @@ public String toString() { */ private static final int JVMCI_METHOD_ENTRY_BCI = -1; - /** - * Special Java bytecode marker value for method entry that is not actually a bytecode. Used to - * enumerate all operations that can be encountered in Crema that may need a profile. - */ - private static final int NON_BYTECODE_METHOD_ENTRY = 0xDEAD0001; - - /** - * Special Java bytecode marker value for a loop backedge that is not actually a bytecode. Used - * to enumerate all operations that can be encountered in Crema that may need a profile. - */ - private static final int NON_BYTECODE_LOOP_BACKEDGE = 0xDEAD0002; - - /** - * All Java bytecodes that somehow represent a branching structure and thus need branch - * profiles. - */ - public static final int[] BRANCH_PROFILED_BYTECODES = {Bytecodes.IFEQ, - Bytecodes.IFNE, - Bytecodes.IFLT, - Bytecodes.IFLE, - Bytecodes.IFGT, - Bytecodes.IFGE, - Bytecodes.IF_ICMPEQ, - Bytecodes.IF_ICMPNE, - Bytecodes.IF_ICMPLT, - Bytecodes.IF_ICMPLE, - Bytecodes.IF_ICMPGT, - Bytecodes.IF_ICMPGE, - Bytecodes.IF_ACMPEQ, - Bytecodes.IF_ACMPNE, - Bytecodes.IFNULL, - Bytecodes.IFNONNULL, - Bytecodes.TABLESWITCH, - Bytecodes.LOOKUPSWITCH - }; - - /** - * All Java bytecodes that somehow represent a type based operation and thus need type profiles. - */ - public static final int[] TYPE_PROFILED_BYTECODES = { - Bytecodes.INVOKEVIRTUAL, - Bytecodes.INVOKEINTERFACE, - Bytecodes.INVOKEDYNAMIC, - Bytecodes.INVOKESTATIC, - Bytecodes.INVOKESPECIAL, - Bytecodes.CHECKCAST, - Bytecodes.INSTANCEOF, - Bytecodes.AALOAD, - Bytecodes.AASTORE - }; - - /** - * All Java bytecodes that represent special operations that should be counted. - */ - public static final int[] COUNTING_PROFILED_BYTECODES = {NON_BYTECODE_METHOD_ENTRY, NON_BYTECODE_LOOP_BACKEDGE, Bytecodes.GOTO, Bytecodes.GOTO_W}; - - public static final int[] ALL_PROFILED_BYTECODES; - - static { - int size = BRANCH_PROFILED_BYTECODES.length + TYPE_PROFILED_BYTECODES.length + COUNTING_PROFILED_BYTECODES.length; - ALL_PROFILED_BYTECODES = new int[size]; - System.arraycopy(BRANCH_PROFILED_BYTECODES, 0, ALL_PROFILED_BYTECODES, 0, BRANCH_PROFILED_BYTECODES.length); - System.arraycopy(TYPE_PROFILED_BYTECODES, 0, ALL_PROFILED_BYTECODES, BRANCH_PROFILED_BYTECODES.length, TYPE_PROFILED_BYTECODES.length); - System.arraycopy(COUNTING_PROFILED_BYTECODES, 0, ALL_PROFILED_BYTECODES, BRANCH_PROFILED_BYTECODES.length + TYPE_PROFILED_BYTECODES.length, COUNTING_PROFILED_BYTECODES.length); - } - } diff --git a/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/meta/RistrettoMethod.java b/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/meta/RistrettoMethod.java index 09e6845c55dc..22c542a6bb0c 100644 --- a/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/meta/RistrettoMethod.java +++ b/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/meta/RistrettoMethod.java @@ -29,10 +29,11 @@ import com.oracle.svm.graal.meta.SubstrateMethod; import com.oracle.svm.graal.meta.SubstrateType; import com.oracle.svm.interpreter.metadata.InterpreterResolvedJavaMethod; -import com.oracle.svm.interpreter.metadata.profile.InterpreterProfileContainer; +import com.oracle.svm.interpreter.metadata.profile.MethodProfilingData; import com.oracle.svm.interpreter.ristretto.RistrettoConstants; import com.oracle.svm.interpreter.ristretto.RistrettoUtils; +import jdk.graal.compiler.nodes.extended.MembarNode; import jdk.vm.ci.code.InstalledCode; import jdk.vm.ci.meta.ConstantPool; import jdk.vm.ci.meta.ExceptionHandler; @@ -63,7 +64,7 @@ public final class RistrettoMethod extends SubstrateMethod { * under heavy synchronization. Never written again. If a ristretto method is GCed profile is * lost. */ - private InterpreterProfileContainer profile; + private MethodProfilingData profile; /** * State-machine for compilation handling of this crema method. Every methods starts in a * NEVER_COMPILED state and than can cycle through different states. @@ -97,14 +98,17 @@ public static RistrettoMethod create(InterpreterResolvedJavaMethod interpreterMe return (RistrettoMethod) interpreterMethod.getRistrettoMethod(RISTRETTO_METHOD_FUNCTION); } - public InterpreterProfileContainer getProfile() { + public MethodProfilingData getProfile() { if (profile == null) { /* * Allocate the profile per method once. Note that out of test scenarios this is never * done and thus the heavy locking code below is normally not run. */ synchronized (this) { - profile = new InterpreterProfileContainer(this); + if (profile == null) { + MembarNode.memoryBarrier(MembarNode.FenceKind.STORE_STORE); + profile = new MethodProfilingData(this); + } } } return profile; diff --git a/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/profile/RistrettoProfileSupport.java b/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/profile/RistrettoProfileSupport.java index 1a7992d21187..86f57fa067ae 100644 --- a/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/profile/RistrettoProfileSupport.java +++ b/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/profile/RistrettoProfileSupport.java @@ -33,7 +33,7 @@ import com.oracle.svm.core.option.RuntimeOptionKey; import com.oracle.svm.interpreter.metadata.CremaResolvedJavaMethodImpl; import com.oracle.svm.interpreter.metadata.InterpreterResolvedJavaMethod; -import com.oracle.svm.interpreter.metadata.profile.InterpreterProfileContainer; +import com.oracle.svm.interpreter.metadata.profile.MethodProfilingData; import com.oracle.svm.interpreter.ristretto.RistrettoConstants; import com.oracle.svm.interpreter.ristretto.RistrettoFeature; import com.oracle.svm.interpreter.ristretto.RistrettoRuntimeOptions; @@ -95,7 +95,7 @@ public static void profileIfBranch(InterpreterResolvedJavaMethod iMethod, int bc } case RistrettoConstants.COMPILE_STATE_NEVER_COMPILED: { - InterpreterProfileContainer methodProfile = rMethod.getProfile(); + MethodProfilingData methodProfile = rMethod.getProfile(); trace(RistrettoRuntimeOptions.JITTraceProfilingIncrements, String.format("[Ristretto Compile Queue]Entering state %s for %s, counter=%s%n", RistrettoCompileStateMachine.toString(COMPILATION_STATE_UPDATER.get(rMethod)), iMethod, methodProfile.getProfileEntryCount())); /* @@ -211,7 +211,7 @@ public static void profileMethodCall(InterpreterResolvedJavaMethod iMethod) { } private static void methodEntryNeverCompiledCase(InterpreterResolvedJavaMethod iMethod, RistrettoMethod rMethod, int oldState) { - InterpreterProfileContainer methodProfile = rMethod.getProfile(); + MethodProfilingData methodProfile = rMethod.getProfile(); trace(RistrettoRuntimeOptions.JITTraceProfilingIncrements, String.format("[Ristretto Compile Queue]Entering state %s for %s, counter=%s%n", RistrettoCompileStateMachine.toString(COMPILATION_STATE_UPDATER.get(rMethod)), iMethod, methodProfile.getProfileEntryCount())); /* diff --git a/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/profile/RistrettoProfilingInfo.java b/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/profile/RistrettoProfilingInfo.java index bf93036aaec6..3dcdb3d4eea7 100644 --- a/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/profile/RistrettoProfilingInfo.java +++ b/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/profile/RistrettoProfilingInfo.java @@ -24,7 +24,7 @@ */ package com.oracle.svm.interpreter.ristretto.profile; -import com.oracle.svm.interpreter.metadata.profile.InterpreterProfileContainer; +import com.oracle.svm.interpreter.metadata.profile.MethodProfilingData; import com.oracle.svm.interpreter.ristretto.RistrettoRuntimeOptions; import jdk.graal.compiler.debug.Assertions; @@ -35,10 +35,10 @@ import jdk.vm.ci.meta.TriState; public class RistrettoProfilingInfo implements ProfilingInfo { - private final InterpreterProfileContainer interpreterProfileContainer; + private final MethodProfilingData methodProfilingData; - public RistrettoProfilingInfo(InterpreterProfileContainer interpreterProfileContainer) { - this.interpreterProfileContainer = interpreterProfileContainer; + public RistrettoProfilingInfo(MethodProfilingData methodProfilingData) { + this.methodProfilingData = methodProfilingData; } @Override @@ -58,9 +58,9 @@ public JavaMethodProfile getMethodProfile(int bci) { @Override public double getBranchTakenProbability(int bci) { - final double takenProfile = interpreterProfileContainer.getBranchTakenProbability(bci); - assert !Double.isNaN(takenProfile) && !Double.isNaN(takenProfile) : Assertions.errorMessage("Must have sane value", takenProfile, interpreterProfileContainer.getMethod(), - InterpreterProfileContainer.TestingBackdoor.profilesAtBCI(interpreterProfileContainer, bci)); + final double takenProfile = methodProfilingData.getBranchTakenProbability(bci); + assert !Double.isNaN(takenProfile) && !Double.isNaN(takenProfile) : Assertions.errorMessage("Must have sane value", takenProfile, methodProfilingData.getMethod(), + MethodProfilingData.TestingBackdoor.profilesAtBCI(methodProfilingData, bci)); return takenProfile; } @@ -94,12 +94,12 @@ public boolean isMature() { /* * Either maturity was explicitly requested or we follow regular ergonomics. */ - return interpreterProfileContainer.isMature() || interpreterProfileContainer.getProfileEntryCount() > RistrettoRuntimeOptions.JITProfileMatureInvocationThreshold.getValue(); + return methodProfilingData.isMature() || methodProfilingData.getProfileEntryCount() > RistrettoRuntimeOptions.JITProfileMatureInvocationThreshold.getValue(); } @Override public String toString() { - return "DefaultProfilingInfo<" + this.toString(null, "; ") + ">"; + return "RistrettoProfilingInfo<" + this.toString(null, "; ") + ">"; } @Override From dd3d9ebb9d8fb79f15e13dff265f7d6c89c4b912 Mon Sep 17 00:00:00 2001 From: David Leopoldseder Date: Thu, 4 Dec 2025 11:16:41 +0100 Subject: [PATCH 04/18] address review comments --- ...dProfilingData.java => MethodProfile.java} | 29 +++++++++++-------- .../ristretto/meta/RistrettoMethod.java | 13 +++++---- .../profile/RistrettoProfileSupport.java | 6 ++-- .../profile/RistrettoProfilingInfo.java | 18 ++++++------ 4 files changed, 36 insertions(+), 30 deletions(-) rename substratevm/src/com.oracle.svm.interpreter.metadata/src/com/oracle/svm/interpreter/metadata/profile/{MethodProfilingData.java => MethodProfile.java} (87%) diff --git a/substratevm/src/com.oracle.svm.interpreter.metadata/src/com/oracle/svm/interpreter/metadata/profile/MethodProfilingData.java b/substratevm/src/com.oracle.svm.interpreter.metadata/src/com/oracle/svm/interpreter/metadata/profile/MethodProfile.java similarity index 87% rename from substratevm/src/com.oracle.svm.interpreter.metadata/src/com/oracle/svm/interpreter/metadata/profile/MethodProfilingData.java rename to substratevm/src/com.oracle.svm.interpreter.metadata/src/com/oracle/svm/interpreter/metadata/profile/MethodProfile.java index f3df0a13beb9..11a066c464ef 100644 --- a/substratevm/src/com.oracle.svm.interpreter.metadata/src/com/oracle/svm/interpreter/metadata/profile/MethodProfilingData.java +++ b/substratevm/src/com.oracle.svm.interpreter.metadata/src/com/oracle/svm/interpreter/metadata/profile/MethodProfile.java @@ -36,9 +36,19 @@ import jdk.vm.ci.meta.ResolvedJavaMethod; /** - * Abstract base representation of profile data for crema per method. + * Stores interpreter profiling data collected during the execution of a single + * {@link ResolvedJavaMethod}. + *

+ * The data is written concurrently by multiple Crema interpreter threads during method execution. + * It is subsequently read by compilation consumers, typically wrapped in a {@link ProfilingInfo} + * object. + *

+ * **Thread Safety and Mutability:** Because multiple interpreter threads update the profiles + * concurrently, the data within this object is **highly volatile**. Any profile-related information + * returned by methods of this class can change significantly and rapidly over time. Consumers must + * be aware of this mutability when reading and acting upon the profiling data. */ -public final class MethodProfilingData { +public final class MethodProfile { private final InterpreterProfile[] profiles; @@ -51,16 +61,11 @@ public final class MethodProfilingData { private final ResolvedJavaMethod method; /** - * Testing override for {@link ProfilingInfo#isMature()}. - *

- * Normally, maturity is derived ergonomically from the collected profiles (for example, the - * method-entry invocation count) by consumers of this container. When this flag is set to true, - * consumers may treat the profiling information as mature regardless of the collected counts. - * Defaults to false. + * See {@link #isMature()}. */ private boolean isMature; - public MethodProfilingData(ResolvedJavaMethod method) { + public MethodProfile(ResolvedJavaMethod method) { this.method = method; this.profiles = buildProfiles(method); } @@ -94,10 +99,10 @@ public ResolvedJavaMethod getMethod() { } /** - * Similar semantic as {@link ProfilingInfo#isMature()} except this method does not perform an + * Similar semantics as {@link ProfilingInfo#isMature()} except this method does not perform an * ergonomic decision. A profile is only mature if it was explicitly set with * {@link #setMature(boolean)}. Normally this is done by test code for example. Users of this - * {@link MethodProfilingData} can combine this with real ergonomics. + * {@link MethodProfile} can combine this with real ergonomics. * * @return true if an explicit maturity override has been set on this profiling data; false * otherwise @@ -156,7 +161,7 @@ private InterpreterProfile getAtBCI(int bci, Class } public static class TestingBackdoor { - public static List profilesAtBCI(MethodProfilingData methodProfile, int bci) { + public static List profilesAtBCI(MethodProfile methodProfile, int bci) { ArrayList profiles = new ArrayList<>(); for (int i = 0; i < methodProfile.profiles.length; i++) { InterpreterProfile profile = methodProfile.profiles[i]; diff --git a/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/meta/RistrettoMethod.java b/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/meta/RistrettoMethod.java index 22c542a6bb0c..c36b3d3c9705 100644 --- a/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/meta/RistrettoMethod.java +++ b/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/meta/RistrettoMethod.java @@ -29,7 +29,7 @@ import com.oracle.svm.graal.meta.SubstrateMethod; import com.oracle.svm.graal.meta.SubstrateType; import com.oracle.svm.interpreter.metadata.InterpreterResolvedJavaMethod; -import com.oracle.svm.interpreter.metadata.profile.MethodProfilingData; +import com.oracle.svm.interpreter.metadata.profile.MethodProfile; import com.oracle.svm.interpreter.ristretto.RistrettoConstants; import com.oracle.svm.interpreter.ristretto.RistrettoUtils; @@ -64,7 +64,7 @@ public final class RistrettoMethod extends SubstrateMethod { * under heavy synchronization. Never written again. If a ristretto method is GCed profile is * lost. */ - private MethodProfilingData profile; + private MethodProfile profile; /** * State-machine for compilation handling of this crema method. Every methods starts in a * NEVER_COMPILED state and than can cycle through different states. @@ -98,16 +98,17 @@ public static RistrettoMethod create(InterpreterResolvedJavaMethod interpreterMe return (RistrettoMethod) interpreterMethod.getRistrettoMethod(RISTRETTO_METHOD_FUNCTION); } - public MethodProfilingData getProfile() { + public MethodProfile getProfile() { if (profile == null) { /* - * Allocate the profile per method once. Note that out of test scenarios this is never - * done and thus the heavy locking code below is normally not run. + * Allocate the profile once per method. Apart from test scenarios the profile is never + * set to null again. Thus, the heavy locking code below is normally not run in a fast + * path. */ synchronized (this) { if (profile == null) { MembarNode.memoryBarrier(MembarNode.FenceKind.STORE_STORE); - profile = new MethodProfilingData(this); + profile = new MethodProfile(this); } } } diff --git a/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/profile/RistrettoProfileSupport.java b/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/profile/RistrettoProfileSupport.java index 86f57fa067ae..74b456bce7a4 100644 --- a/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/profile/RistrettoProfileSupport.java +++ b/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/profile/RistrettoProfileSupport.java @@ -33,7 +33,7 @@ import com.oracle.svm.core.option.RuntimeOptionKey; import com.oracle.svm.interpreter.metadata.CremaResolvedJavaMethodImpl; import com.oracle.svm.interpreter.metadata.InterpreterResolvedJavaMethod; -import com.oracle.svm.interpreter.metadata.profile.MethodProfilingData; +import com.oracle.svm.interpreter.metadata.profile.MethodProfile; import com.oracle.svm.interpreter.ristretto.RistrettoConstants; import com.oracle.svm.interpreter.ristretto.RistrettoFeature; import com.oracle.svm.interpreter.ristretto.RistrettoRuntimeOptions; @@ -95,7 +95,7 @@ public static void profileIfBranch(InterpreterResolvedJavaMethod iMethod, int bc } case RistrettoConstants.COMPILE_STATE_NEVER_COMPILED: { - MethodProfilingData methodProfile = rMethod.getProfile(); + MethodProfile methodProfile = rMethod.getProfile(); trace(RistrettoRuntimeOptions.JITTraceProfilingIncrements, String.format("[Ristretto Compile Queue]Entering state %s for %s, counter=%s%n", RistrettoCompileStateMachine.toString(COMPILATION_STATE_UPDATER.get(rMethod)), iMethod, methodProfile.getProfileEntryCount())); /* @@ -211,7 +211,7 @@ public static void profileMethodCall(InterpreterResolvedJavaMethod iMethod) { } private static void methodEntryNeverCompiledCase(InterpreterResolvedJavaMethod iMethod, RistrettoMethod rMethod, int oldState) { - MethodProfilingData methodProfile = rMethod.getProfile(); + MethodProfile methodProfile = rMethod.getProfile(); trace(RistrettoRuntimeOptions.JITTraceProfilingIncrements, String.format("[Ristretto Compile Queue]Entering state %s for %s, counter=%s%n", RistrettoCompileStateMachine.toString(COMPILATION_STATE_UPDATER.get(rMethod)), iMethod, methodProfile.getProfileEntryCount())); /* diff --git a/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/profile/RistrettoProfilingInfo.java b/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/profile/RistrettoProfilingInfo.java index 3dcdb3d4eea7..55f8cf86a8d3 100644 --- a/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/profile/RistrettoProfilingInfo.java +++ b/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/profile/RistrettoProfilingInfo.java @@ -24,7 +24,7 @@ */ package com.oracle.svm.interpreter.ristretto.profile; -import com.oracle.svm.interpreter.metadata.profile.MethodProfilingData; +import com.oracle.svm.interpreter.metadata.profile.MethodProfile; import com.oracle.svm.interpreter.ristretto.RistrettoRuntimeOptions; import jdk.graal.compiler.debug.Assertions; @@ -35,10 +35,10 @@ import jdk.vm.ci.meta.TriState; public class RistrettoProfilingInfo implements ProfilingInfo { - private final MethodProfilingData methodProfilingData; + private final MethodProfile methodProfile; - public RistrettoProfilingInfo(MethodProfilingData methodProfilingData) { - this.methodProfilingData = methodProfilingData; + public RistrettoProfilingInfo(MethodProfile methodProfile) { + this.methodProfile = methodProfile; } @Override @@ -58,9 +58,9 @@ public JavaMethodProfile getMethodProfile(int bci) { @Override public double getBranchTakenProbability(int bci) { - final double takenProfile = methodProfilingData.getBranchTakenProbability(bci); - assert !Double.isNaN(takenProfile) && !Double.isNaN(takenProfile) : Assertions.errorMessage("Must have sane value", takenProfile, methodProfilingData.getMethod(), - MethodProfilingData.TestingBackdoor.profilesAtBCI(methodProfilingData, bci)); + final double takenProfile = methodProfile.getBranchTakenProbability(bci); + assert !Double.isNaN(takenProfile) && !Double.isNaN(takenProfile) : Assertions.errorMessage("Must have sane value", takenProfile, methodProfile.getMethod(), + MethodProfile.TestingBackdoor.profilesAtBCI(methodProfile, bci)); return takenProfile; } @@ -94,7 +94,7 @@ public boolean isMature() { /* * Either maturity was explicitly requested or we follow regular ergonomics. */ - return methodProfilingData.isMature() || methodProfilingData.getProfileEntryCount() > RistrettoRuntimeOptions.JITProfileMatureInvocationThreshold.getValue(); + return methodProfile.isMature() || methodProfile.getProfileEntryCount() > RistrettoRuntimeOptions.JITProfileMatureInvocationThreshold.getValue(); } @Override @@ -104,7 +104,7 @@ public String toString() { @Override public void setMature() { - // Do nothing + methodProfile.setMature(true); } @Override From 4bc87ca1692786ff1f6999bc775230d47456f6b9 Mon Sep 17 00:00:00 2001 From: David Leopoldseder Date: Thu, 4 Dec 2025 12:21:57 +0100 Subject: [PATCH 05/18] ristretto: ensure object is allocated before barrier --- .../svm/interpreter/ristretto/meta/RistrettoMethod.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/meta/RistrettoMethod.java b/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/meta/RistrettoMethod.java index c36b3d3c9705..185d79196e91 100644 --- a/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/meta/RistrettoMethod.java +++ b/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/meta/RistrettoMethod.java @@ -107,8 +107,11 @@ public MethodProfile getProfile() { */ synchronized (this) { if (profile == null) { + MethodProfile newProfile = new MethodProfile(this); + // ensure everything is allocated and initialized before we signal the barrier + // for the publishing write MembarNode.memoryBarrier(MembarNode.FenceKind.STORE_STORE); - profile = new MethodProfile(this); + profile = newProfile; } } } From a1874c0646587f83f435fd251d4926a60facfe34 Mon Sep 17 00:00:00 2001 From: David Leopoldseder Date: Thu, 4 Dec 2025 12:23:09 +0100 Subject: [PATCH 06/18] ristretto: early return on last index, then check next index contributed by gilles.m.duboscq@oracle.com --- .../metadata/profile/MethodProfile.java | 29 +++++++++---------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/substratevm/src/com.oracle.svm.interpreter.metadata/src/com/oracle/svm/interpreter/metadata/profile/MethodProfile.java b/substratevm/src/com.oracle.svm.interpreter.metadata/src/com/oracle/svm/interpreter/metadata/profile/MethodProfile.java index 11a066c464ef..8bc209220afa 100644 --- a/substratevm/src/com.oracle.svm.interpreter.metadata/src/com/oracle/svm/interpreter/metadata/profile/MethodProfile.java +++ b/substratevm/src/com.oracle.svm.interpreter.metadata/src/com/oracle/svm/interpreter/metadata/profile/MethodProfile.java @@ -141,23 +141,22 @@ public double getBranchTakenProbability(int bci) { * @return null if there's no prof */ private InterpreterProfile getAtBCI(int bci, Class clazz) { - InterpreterProfile lastUsedProfile = profiles[lastIndex]; - if (lastUsedProfile == null || lastUsedProfile.getBci() != bci || lastUsedProfile.getClass() != clazz) { - // search for the correct profile - for (int i = 0; i < profiles.length; i++) { - InterpreterProfile profile = profiles[i]; - // make sure lastUsedProfile == null if we did not find a match - if (profile.getBci() == bci && profile.getClass() == clazz) { - lastUsedProfile = profile; - lastIndex = i; - break; - } else { - lastUsedProfile = null; - } + int lastIndexLocal = lastIndex; + for (int i = lastIndexLocal; i < profiles.length; i++) { + InterpreterProfile profile = profiles[i]; + if (profile.getBci() == bci && profile.getClass() == clazz) { + lastIndex = i; + return profile; + } + } + for (int i = 0; i < lastIndexLocal; i++) { + InterpreterProfile profile = profiles[i]; + if (profile.getBci() == bci && profile.getClass() == clazz) { + lastIndex = i; + return profile; } } - // found, probably the last entry used it as well - return lastUsedProfile; + return null; } public static class TestingBackdoor { From d2d773c7443556198e9ebf343bb6ff687dd20df5 Mon Sep 17 00:00:00 2001 From: David Leopoldseder Date: Thu, 4 Dec 2025 12:24:35 +0100 Subject: [PATCH 07/18] ristretto: style fixes --- .../interpreter/metadata/profile/MethodProfile.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/substratevm/src/com.oracle.svm.interpreter.metadata/src/com/oracle/svm/interpreter/metadata/profile/MethodProfile.java b/substratevm/src/com.oracle.svm.interpreter.metadata/src/com/oracle/svm/interpreter/metadata/profile/MethodProfile.java index 8bc209220afa..03ceba947e03 100644 --- a/substratevm/src/com.oracle.svm.interpreter.metadata/src/com/oracle/svm/interpreter/metadata/profile/MethodProfile.java +++ b/substratevm/src/com.oracle.svm.interpreter.metadata/src/com/oracle/svm/interpreter/metadata/profile/MethodProfile.java @@ -50,6 +50,11 @@ */ public final class MethodProfile { + /** + * Artificial byte code index for the method entry profile. + */ + private static final int JVMCI_METHOD_ENTRY_BCI = -1; + private final InterpreterProfile[] profiles; /** @@ -241,9 +246,4 @@ public String toString() { } } - /** - * Artificial byte code index for the method entry profile. - */ - private static final int JVMCI_METHOD_ENTRY_BCI = -1; - } From ef3e1bf8630f69c34ed0527962d3f4109a19a978 Mon Sep 17 00:00:00 2001 From: David Leopoldseder Date: Thu, 4 Dec 2025 12:32:03 +0100 Subject: [PATCH 08/18] ristretto: do not spin during branch profiling if state is currently initialized --- .../metadata/profile/MethodProfile.java | 8 +- .../profile/RistrettoProfileSupport.java | 97 +++++++++---------- 2 files changed, 49 insertions(+), 56 deletions(-) diff --git a/substratevm/src/com.oracle.svm.interpreter.metadata/src/com/oracle/svm/interpreter/metadata/profile/MethodProfile.java b/substratevm/src/com.oracle.svm.interpreter.metadata/src/com/oracle/svm/interpreter/metadata/profile/MethodProfile.java index 03ceba947e03..98bf10b4d171 100644 --- a/substratevm/src/com.oracle.svm.interpreter.metadata/src/com/oracle/svm/interpreter/metadata/profile/MethodProfile.java +++ b/substratevm/src/com.oracle.svm.interpreter.metadata/src/com/oracle/svm/interpreter/metadata/profile/MethodProfile.java @@ -43,10 +43,10 @@ * It is subsequently read by compilation consumers, typically wrapped in a {@link ProfilingInfo} * object. *

- * **Thread Safety and Mutability:** Because multiple interpreter threads update the profiles - * concurrently, the data within this object is **highly volatile**. Any profile-related information - * returned by methods of this class can change significantly and rapidly over time. Consumers must - * be aware of this mutability when reading and acting upon the profiling data. + * Thread Safety and Mutability: Because multiple interpreter threads update the profiles + * concurrently, the data within this object is highly volatile. Any profile-related + * information returned by methods of this class can change significantly and rapidly over time. + * Consumers must be aware of this mutability when reading and acting upon the profiling data. */ public final class MethodProfile { diff --git a/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/profile/RistrettoProfileSupport.java b/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/profile/RistrettoProfileSupport.java index 74b456bce7a4..b08f12d96459 100644 --- a/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/profile/RistrettoProfileSupport.java +++ b/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/profile/RistrettoProfileSupport.java @@ -66,57 +66,48 @@ public static void profileIfBranch(InterpreterResolvedJavaMethod iMethod, int bc assert iMethod instanceof CremaResolvedJavaMethodImpl; final RistrettoMethod rMethod = RistrettoMethod.create(iMethod); - int oldState = COMPILATION_STATE_UPDATER.get(rMethod); - if (!RistrettoCompileStateMachine.shouldEnterProfiling(oldState)) { - // no need to keep profiling this code, we are done - trace(RistrettoRuntimeOptions.JITTraceCompilationQueuing, "[Ristretto Compile Queue]Should not enter profiling for method %s because of state %s%n", iMethod, - RistrettoCompileStateMachine.toString(oldState)); - return; - } + final int oldState = COMPILATION_STATE_UPDATER.get(rMethod); - // this point is only reached for state=INIT_VAL|INITIALIZING|NEVER_COMPILED - while (true) { - oldState = COMPILATION_STATE_UPDATER.get(rMethod); - - switch (oldState) { + switch (oldState) { + /* + * Profile has not been initialized yet for this method, given we are profiling a branch + * we must have profiled the method entry already, thus this is a hard error. + */ + case RistrettoConstants.COMPILE_STATE_INIT_VAL: { + throw GraalError.shouldNotReachHere(String.format("Reached a method without an initialized profile when trying to profile a branch. " + + "This must not happen. Every profile must be created and initially written when profiling the entry of a method in the interpreter. " + + "Method=%s", rMethod)); + } + case RistrettoConstants.COMPILE_STATE_SUBMITTED: + case RistrettoConstants.COMPILE_STATE_COMPILED: { + profileSkipCase(iMethod, rMethod); + return; + } + case RistrettoConstants.COMPILE_STATE_NEVER_COMPILED: { + profileBranch(iMethod, bci, taken, rMethod); + return; + } + case RistrettoConstants.COMPILE_STATE_INITIALIZING: { /* - * Profile has not been initialized yet for this method, given we are profiling a - * branch we must have profiled the method entry already, thus this is a hard error. + * Another thread is initializing the compilation data, given we are only profiling + * a branch we can avoid losing a few branch profile updates. */ - case RistrettoConstants.COMPILE_STATE_INIT_VAL: { - throw GraalError.shouldNotReachHere(String.format("Reached a method without an initialized profile when trying to profile a branch. " + - "This must not happen. Every profile must be created and initially written when profiling the entry of a method in the interpreter. " + - "Method=%s", rMethod)); - } - case RistrettoConstants.COMPILE_STATE_SUBMITTED: - case RistrettoConstants.COMPILE_STATE_COMPILED: { - profileSkipCase(iMethod, rMethod); - return; - } - case RistrettoConstants.COMPILE_STATE_NEVER_COMPILED: { - - MethodProfile methodProfile = rMethod.getProfile(); - trace(RistrettoRuntimeOptions.JITTraceProfilingIncrements, String.format("[Ristretto Compile Queue]Entering state %s for %s, counter=%s%n", - RistrettoCompileStateMachine.toString(COMPILATION_STATE_UPDATER.get(rMethod)), iMethod, methodProfile.getProfileEntryCount())); - /* - * We write without any synchronization to the branch profile value at the cost - * of lost updates. - */ - methodProfile.profileBranch(bci, taken); - - // we only increment (and submit if applicable) once, thus return now - return; - } - case RistrettoConstants.COMPILE_STATE_INITIALIZING: { - // another thread is initializing the compilation data, do a few more spins - // until that is done and then go on - break; - } - default: - throw GraalError.shouldNotReachHere("Unknown state " + oldState); + return; } + default: + throw GraalError.shouldNotReachHere("Unknown state " + oldState); } + } + private static void profileBranch(InterpreterResolvedJavaMethod iMethod, int bci, boolean taken, RistrettoMethod rMethod) { + MethodProfile methodProfile = rMethod.getProfile(); + trace(RistrettoRuntimeOptions.JITTraceProfilingIncrements, String.format("[Ristretto Compile Queue]Entering state %s for %s, counter=%s%n", + RistrettoCompileStateMachine.toString(COMPILATION_STATE_UPDATER.get(rMethod)), iMethod, methodProfile.getProfileEntryCount())); + /* + * We write without any synchronization to the branch profile value at the cost of lost + * updates. + */ + methodProfile.profileBranch(bci, taken); } /** @@ -155,7 +146,6 @@ public static void profileMethodCall(InterpreterResolvedJavaMethod iMethod) { if (!RistrettoProfileSupport.isEnabled()) { return; } - if (!RistrettoRuntimeOptions.JITEnableCompilation.getValue()) { return; } @@ -180,11 +170,12 @@ public static void profileMethodCall(InterpreterResolvedJavaMethod iMethod) { * to ensure better readability. However, for performance we should ensure inlining * happens on most of the frequently executed cases here. */ - switch (oldState) { - // profile has not been initialized yet for this method, do so by switching to - // INITIALIZING and then wait, if another thread went to initializing in the - // meantime we are done + /* + * profile has not been initialized yet for this method, do so by switching to + * INITIALIZING and then wait, if another thread went to initializing in the + * meantime we are done + */ case RistrettoConstants.COMPILE_STATE_INIT_VAL: { methodEntryInitCase(iMethod, rMethod); break; @@ -200,8 +191,10 @@ public static void profileMethodCall(InterpreterResolvedJavaMethod iMethod) { return; } case RistrettoConstants.COMPILE_STATE_INITIALIZING: { - // another thread is initializing the compilation data, do a few more spins - // until that is done and then go on + /* + * another thread is initializing the compilation data, do a few more spins + * until that is done and then go on + */ break; } default: From 299df18daabdfeaa352c01cf56f5fa6401b07e85 Mon Sep 17 00:00:00 2001 From: David Leopoldseder Date: Thu, 4 Dec 2025 12:45:31 +0100 Subject: [PATCH 09/18] ristretto: profileMethodCall refactor loop to avoid second read upon entering the loop --- .../ristretto/profile/RistrettoProfileSupport.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/profile/RistrettoProfileSupport.java b/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/profile/RistrettoProfileSupport.java index b08f12d96459..a3c4da27b07e 100644 --- a/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/profile/RistrettoProfileSupport.java +++ b/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/profile/RistrettoProfileSupport.java @@ -162,9 +162,7 @@ public static void profileMethodCall(InterpreterResolvedJavaMethod iMethod) { } // this point is only reached for state=INIT_VAL|INITIALIZING|NEVER_COMPILED - while (true) { - oldState = COMPILATION_STATE_UPDATER.get(rMethod); - + do { /* * TODO GR-71597 A note on interpreter performance. Code is abstracted in methods here * to ensure better readability. However, for performance we should ensure inlining @@ -200,7 +198,8 @@ public static void profileMethodCall(InterpreterResolvedJavaMethod iMethod) { default: throw GraalError.shouldNotReachHere("Unknown state " + oldState); } - } + oldState = COMPILATION_STATE_UPDATER.get(rMethod); + } while (true); } private static void methodEntryNeverCompiledCase(InterpreterResolvedJavaMethod iMethod, RistrettoMethod rMethod, int oldState) { From 1d25b85e7739ef41dbaa37620b99f99ed02873d1 Mon Sep 17 00:00:00 2001 From: David Leopoldseder Date: Fri, 5 Dec 2025 12:34:46 +0100 Subject: [PATCH 10/18] ristretto: move profile get into interpreter to only query (and potentially submit compile) on method entry --- .../oracle/svm/interpreter/Interpreter.java | 35 +++++++--- .../profile/RistrettoProfileSupport.java | 66 +++---------------- 2 files changed, 34 insertions(+), 67 deletions(-) diff --git a/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/Interpreter.java b/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/Interpreter.java index c801de2f1b2f..36a8d30a3307 100644 --- a/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/Interpreter.java +++ b/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/Interpreter.java @@ -291,6 +291,7 @@ import com.oracle.svm.interpreter.metadata.ReferenceConstant; import com.oracle.svm.interpreter.metadata.TableSwitch; import com.oracle.svm.interpreter.metadata.UnsupportedResolutionException; +import com.oracle.svm.interpreter.metadata.profile.MethodProfile; import com.oracle.svm.interpreter.ristretto.profile.RistrettoProfileSupport; import jdk.graal.compiler.api.directives.GraalDirectives; @@ -622,7 +623,7 @@ public static final class Root { @NeverInline("needed far stack walking") private static Object executeBodyFromBCI(InterpreterFrame frame, InterpreterResolvedJavaMethod method, int startBCI, int startTop, boolean forceStayInInterpreter) { - RistrettoProfileSupport.profileMethodCall(method); + final MethodProfile methodProfile = RistrettoProfileSupport.profileMethodCall(method); int curBCI = startBCI; int top = startTop; @@ -886,12 +887,16 @@ private static Object executeBodyFromBCI(InterpreterFrame frame, InterpreterReso case IFGT: // fall through case IFLE: if (takeBranchPrimitive1(popInt(frame, top - 1), curOpcode)) { - RistrettoProfileSupport.profileIfBranch(method, curBCI, true); + if (methodProfile != null) { + methodProfile.profileBranch(curBCI, true); + } top += ConstantBytecodes.stackEffectOf(IFLE); curBCI = beforeJumpChecks(frame, curBCI, BytecodeStream.readBranchDest2(code, curBCI), top); continue loop; } else { - RistrettoProfileSupport.profileIfBranch(method, curBCI, false); + if (methodProfile != null) { + methodProfile.profileBranch(curBCI, false); + } } break; @@ -902,36 +907,48 @@ private static Object executeBodyFromBCI(InterpreterFrame frame, InterpreterReso case IF_ICMPGT: // fall through case IF_ICMPLE: if (takeBranchPrimitive2(popInt(frame, top - 1), popInt(frame, top - 2), curOpcode)) { - RistrettoProfileSupport.profileIfBranch(method, curBCI, true); + if (methodProfile != null) { + methodProfile.profileBranch(curBCI, true); + } top += ConstantBytecodes.stackEffectOf(IF_ICMPLE); curBCI = beforeJumpChecks(frame, curBCI, BytecodeStream.readBranchDest2(code, curBCI), top); continue loop; } else { - RistrettoProfileSupport.profileIfBranch(method, curBCI, false); + if (methodProfile != null) { + methodProfile.profileBranch(curBCI, false); + } } break; case IF_ACMPEQ: // fall through case IF_ACMPNE: if (takeBranchRef2(popObject(frame, top - 1), popObject(frame, top - 2), curOpcode)) { - RistrettoProfileSupport.profileIfBranch(method, curBCI, true); + if (methodProfile != null) { + methodProfile.profileBranch(curBCI, true); + } top += ConstantBytecodes.stackEffectOf(IF_ACMPNE); curBCI = beforeJumpChecks(frame, curBCI, BytecodeStream.readBranchDest2(code, curBCI), top); continue loop; } else { - RistrettoProfileSupport.profileIfBranch(method, curBCI, false); + if (methodProfile != null) { + methodProfile.profileBranch(curBCI, false); + } } break; case IFNULL: // fall through case IFNONNULL: if (takeBranchRef1(popObject(frame, top - 1), curOpcode)) { - RistrettoProfileSupport.profileIfBranch(method, curBCI, true); + if (methodProfile != null) { + methodProfile.profileBranch(curBCI, true); + } top += ConstantBytecodes.stackEffectOf(IFNONNULL); curBCI = beforeJumpChecks(frame, curBCI, BytecodeStream.readBranchDest2(code, curBCI), top); continue loop; } else { - RistrettoProfileSupport.profileIfBranch(method, curBCI, false); + if (methodProfile != null) { + methodProfile.profileBranch(curBCI, false); + } } break; diff --git a/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/profile/RistrettoProfileSupport.java b/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/profile/RistrettoProfileSupport.java index a3c4da27b07e..88079ff665ac 100644 --- a/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/profile/RistrettoProfileSupport.java +++ b/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/profile/RistrettoProfileSupport.java @@ -58,58 +58,6 @@ public static void trace(RuntimeOptionKey optionKey, String msg, Object } } - public static void profileIfBranch(InterpreterResolvedJavaMethod iMethod, int bci, boolean taken) { - if (!RistrettoProfileSupport.isEnabled()) { - return; - } - - assert iMethod instanceof CremaResolvedJavaMethodImpl; - final RistrettoMethod rMethod = RistrettoMethod.create(iMethod); - - final int oldState = COMPILATION_STATE_UPDATER.get(rMethod); - - switch (oldState) { - /* - * Profile has not been initialized yet for this method, given we are profiling a branch - * we must have profiled the method entry already, thus this is a hard error. - */ - case RistrettoConstants.COMPILE_STATE_INIT_VAL: { - throw GraalError.shouldNotReachHere(String.format("Reached a method without an initialized profile when trying to profile a branch. " + - "This must not happen. Every profile must be created and initially written when profiling the entry of a method in the interpreter. " + - "Method=%s", rMethod)); - } - case RistrettoConstants.COMPILE_STATE_SUBMITTED: - case RistrettoConstants.COMPILE_STATE_COMPILED: { - profileSkipCase(iMethod, rMethod); - return; - } - case RistrettoConstants.COMPILE_STATE_NEVER_COMPILED: { - profileBranch(iMethod, bci, taken, rMethod); - return; - } - case RistrettoConstants.COMPILE_STATE_INITIALIZING: { - /* - * Another thread is initializing the compilation data, given we are only profiling - * a branch we can avoid losing a few branch profile updates. - */ - return; - } - default: - throw GraalError.shouldNotReachHere("Unknown state " + oldState); - } - } - - private static void profileBranch(InterpreterResolvedJavaMethod iMethod, int bci, boolean taken, RistrettoMethod rMethod) { - MethodProfile methodProfile = rMethod.getProfile(); - trace(RistrettoRuntimeOptions.JITTraceProfilingIncrements, String.format("[Ristretto Compile Queue]Entering state %s for %s, counter=%s%n", - RistrettoCompileStateMachine.toString(COMPILATION_STATE_UPDATER.get(rMethod)), iMethod, methodProfile.getProfileEntryCount())); - /* - * We write without any synchronization to the branch profile value at the cost of lost - * updates. - */ - methodProfile.profileBranch(bci, taken); - } - /** * Profiles a method call and manages the compilation submission process for the Ristretto * interpreter. This method implements a thread-safe state machine that handles the lifecycle of @@ -142,12 +90,12 @@ private static void profileBranch(InterpreterResolvedJavaMethod iMethod, int bci * {@link CremaResolvedJavaMethodImpl} * @throws AssertionError if iMethod is not a InterpreterResolvedJavaMethod instance */ - public static void profileMethodCall(InterpreterResolvedJavaMethod iMethod) { + public static MethodProfile profileMethodCall(InterpreterResolvedJavaMethod iMethod) { if (!RistrettoProfileSupport.isEnabled()) { - return; + return null; } if (!RistrettoRuntimeOptions.JITEnableCompilation.getValue()) { - return; + return null; } assert iMethod instanceof CremaResolvedJavaMethodImpl; @@ -158,7 +106,7 @@ public static void profileMethodCall(InterpreterResolvedJavaMethod iMethod) { // no need to keep profiling this code, we are done trace(RistrettoRuntimeOptions.JITTraceCompilationQueuing, "[Ristretto Compile Queue]Should not enter profiling for method %s because of state %s%n", iMethod, RistrettoCompileStateMachine.toString(oldState)); - return; + return null; } // this point is only reached for state=INIT_VAL|INITIALIZING|NEVER_COMPILED @@ -181,12 +129,14 @@ public static void profileMethodCall(InterpreterResolvedJavaMethod iMethod) { case RistrettoConstants.COMPILE_STATE_SUBMITTED: case RistrettoConstants.COMPILE_STATE_COMPILED: { profileSkipCase(iMethod, rMethod); - return; + return null; } case RistrettoConstants.COMPILE_STATE_NEVER_COMPILED: { methodEntryNeverCompiledCase(iMethod, rMethod, oldState); // we only increment (and submit if applicable) once, thus return now - return; + MethodProfile profile = rMethod.getProfile(); + assert profile != null; + return profile; } case RistrettoConstants.COMPILE_STATE_INITIALIZING: { /* From f27f6ae6cd35ff2ee8af0ab025333806822717f0 Mon Sep 17 00:00:00 2001 From: David Leopoldseder Date: Fri, 5 Dec 2025 13:03:24 +0100 Subject: [PATCH 11/18] ristretto:getBranchTakenProbability add more assertions and clamping --- .../profile/RistrettoProfilingInfo.java | 25 ++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/profile/RistrettoProfilingInfo.java b/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/profile/RistrettoProfilingInfo.java index 55f8cf86a8d3..ad8ff5a3eed2 100644 --- a/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/profile/RistrettoProfilingInfo.java +++ b/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/profile/RistrettoProfilingInfo.java @@ -36,6 +36,7 @@ public class RistrettoProfilingInfo implements ProfilingInfo { private final MethodProfile methodProfile; + private static final double PROB_DELTA = 1e-6; public RistrettoProfilingInfo(MethodProfile methodProfile) { this.methodProfile = methodProfile; @@ -58,10 +59,28 @@ public JavaMethodProfile getMethodProfile(int bci) { @Override public double getBranchTakenProbability(int bci) { - final double takenProfile = methodProfile.getBranchTakenProbability(bci); - assert !Double.isNaN(takenProfile) && !Double.isNaN(takenProfile) : Assertions.errorMessage("Must have sane value", takenProfile, methodProfile.getMethod(), + double recordedProbability = methodProfile.getBranchTakenProbability(bci); + + assert !Double.isNaN(recordedProbability) && !Double.isInfinite(recordedProbability) : Assertions.errorMessage("Invalid recorded branch probability", recordedProbability, + methodProfile.getMethod(), MethodProfile.TestingBackdoor.profilesAtBCI(methodProfile, bci)); + + // at runtime if assertions are disabled fall back to unknown probability + if (Double.isNaN(recordedProbability) || Double.isInfinite(recordedProbability)) { + recordedProbability = 0.5D; + } + + // ensure profile is in an expected range (modulo a small delta) + assert recordedProbability >= 0D - PROB_DELTA && recordedProbability <= 1D + PROB_DELTA : Assertions.errorMessage("Must be within [0,1] bounds modulo a small delta but is ", + recordedProbability); + + // Clamp to [0, 1] + double clamped = Math.max(0.0, Math.min(1.0, recordedProbability)); + + // Verify the invariant + assert clamped >= 0.0 && clamped <= 1.0 : Assertions.errorMessage("Branch probability out of [0,1]", clamped, methodProfile.getMethod(), MethodProfile.TestingBackdoor.profilesAtBCI(methodProfile, bci)); - return takenProfile; + + return clamped; } @Override From 8db1a594714f6e43b59b089661d976b89128179c Mon Sep 17 00:00:00 2001 From: David Leopoldseder Date: Fri, 5 Dec 2025 13:05:31 +0100 Subject: [PATCH 12/18] ristretto: check non recorded before assertion logic --- .../interpreter/ristretto/profile/RistrettoProfilingInfo.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/profile/RistrettoProfilingInfo.java b/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/profile/RistrettoProfilingInfo.java index ad8ff5a3eed2..b16d5d6aaaff 100644 --- a/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/profile/RistrettoProfilingInfo.java +++ b/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/profile/RistrettoProfilingInfo.java @@ -60,6 +60,9 @@ public JavaMethodProfile getMethodProfile(int bci) { @Override public double getBranchTakenProbability(int bci) { double recordedProbability = methodProfile.getBranchTakenProbability(bci); + if (recordedProbability == -1D) { + return -1D; + } assert !Double.isNaN(recordedProbability) && !Double.isInfinite(recordedProbability) : Assertions.errorMessage("Invalid recorded branch probability", recordedProbability, methodProfile.getMethod(), MethodProfile.TestingBackdoor.profilesAtBCI(methodProfile, bci)); From 65e72492d0478605f6125e199464f1715df79eec Mon Sep 17 00:00:00 2001 From: David Leopoldseder Date: Tue, 9 Dec 2025 07:15:25 +0100 Subject: [PATCH 13/18] ristretto: checkstyle fixes --- .../interpreter/metadata/profile/MethodProfile.java | 10 +++++----- .../src/com/oracle/svm/interpreter/Interpreter.java | 2 +- .../ristretto/profile/RistrettoProfileSupport.java | 4 +++- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/substratevm/src/com.oracle.svm.interpreter.metadata/src/com/oracle/svm/interpreter/metadata/profile/MethodProfile.java b/substratevm/src/com.oracle.svm.interpreter.metadata/src/com/oracle/svm/interpreter/metadata/profile/MethodProfile.java index 98bf10b4d171..dc9e5c0d291b 100644 --- a/substratevm/src/com.oracle.svm.interpreter.metadata/src/com/oracle/svm/interpreter/metadata/profile/MethodProfile.java +++ b/substratevm/src/com.oracle.svm.interpreter.metadata/src/com/oracle/svm/interpreter/metadata/profile/MethodProfile.java @@ -58,8 +58,8 @@ public final class MethodProfile { private final InterpreterProfile[] profiles; /** - * Cached entry always starts at 0, if it's a cache miss we will initialize it to the correct - * value. + * Caches the index of the last returned profile for the next access. Initialized to 0, will be + * set in {@link #getAtBCI(int, Class)}. */ private int lastIndex; @@ -108,7 +108,7 @@ public ResolvedJavaMethod getMethod() { * ergonomic decision. A profile is only mature if it was explicitly set with * {@link #setMature(boolean)}. Normally this is done by test code for example. Users of this * {@link MethodProfile} can combine this with real ergonomics. - * + * * @return true if an explicit maturity override has been set on this profiling data; false * otherwise */ @@ -142,8 +142,8 @@ public double getBranchTakenProbability(int bci) { /** * Gets the profile for {@code bci} whose class is {@code clazz}. - * - * @return null if there's no prof + * + * @return null if there's no profile */ private InterpreterProfile getAtBCI(int bci, Class clazz) { int lastIndexLocal = lastIndex; diff --git a/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/Interpreter.java b/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/Interpreter.java index 36a8d30a3307..7f268d9a6da6 100644 --- a/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/Interpreter.java +++ b/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/Interpreter.java @@ -623,7 +623,7 @@ public static final class Root { @NeverInline("needed far stack walking") private static Object executeBodyFromBCI(InterpreterFrame frame, InterpreterResolvedJavaMethod method, int startBCI, int startTop, boolean forceStayInInterpreter) { - final MethodProfile methodProfile = RistrettoProfileSupport.profileMethodCall(method); + final MethodProfile methodProfile = RistrettoProfileSupport.profileMethodEntry(method); int curBCI = startBCI; int top = startTop; diff --git a/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/profile/RistrettoProfileSupport.java b/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/profile/RistrettoProfileSupport.java index 88079ff665ac..9e716e03cc8a 100644 --- a/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/profile/RistrettoProfileSupport.java +++ b/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/profile/RistrettoProfileSupport.java @@ -90,7 +90,7 @@ public static void trace(RuntimeOptionKey optionKey, String msg, Object * {@link CremaResolvedJavaMethodImpl} * @throws AssertionError if iMethod is not a InterpreterResolvedJavaMethod instance */ - public static MethodProfile profileMethodCall(InterpreterResolvedJavaMethod iMethod) { + public static MethodProfile profileMethodEntry(InterpreterResolvedJavaMethod iMethod) { if (!RistrettoProfileSupport.isEnabled()) { return null; } @@ -140,6 +140,8 @@ public static MethodProfile profileMethodCall(InterpreterResolvedJavaMethod iMet } case RistrettoConstants.COMPILE_STATE_INITIALIZING: { /* + * TODO GR-71948 - investigate an early return here + * * another thread is initializing the compilation data, do a few more spins * until that is done and then go on */ From b6697936c83425b45b21b7455a41d45dd64e9ed4 Mon Sep 17 00:00:00 2001 From: David Leopoldseder Date: Tue, 9 Dec 2025 09:45:22 +0100 Subject: [PATCH 14/18] ristretto: make it clear goto and jsr are not considered binary if branches --- .../com/oracle/svm/interpreter/metadata/Bytecodes.java | 8 +++++--- .../svm/interpreter/metadata/profile/MethodProfile.java | 2 +- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/substratevm/src/com.oracle.svm.interpreter.metadata/src/com/oracle/svm/interpreter/metadata/Bytecodes.java b/substratevm/src/com.oracle.svm.interpreter.metadata/src/com/oracle/svm/interpreter/metadata/Bytecodes.java index 20b2c27ad32e..62d0d0610b72 100644 --- a/substratevm/src/com.oracle.svm.interpreter.metadata/src/com/oracle/svm/interpreter/metadata/Bytecodes.java +++ b/substratevm/src/com.oracle.svm.interpreter.metadata/src/com/oracle/svm/interpreter/metadata/Bytecodes.java @@ -550,8 +550,8 @@ static class Flags { def(IF_ICMPLE , "if_icmple" , "boo" , -2, FALL_THROUGH | BRANCH | IF_BRANCH_PROFILED); def(IF_ACMPEQ , "if_acmpeq" , "boo" , -2, COMMUTATIVE | FALL_THROUGH | BRANCH | IF_BRANCH_PROFILED); def(IF_ACMPNE , "if_acmpne" , "boo" , -2, COMMUTATIVE | FALL_THROUGH | BRANCH | IF_BRANCH_PROFILED); - def(GOTO , "goto" , "boo" , 0, STOP | BRANCH | IF_BRANCH_PROFILED); - def(JSR , "jsr" , "boo" , 0, STOP | BRANCH | IF_BRANCH_PROFILED); + def(GOTO , "goto" , "boo" , 0, STOP | BRANCH); + def(JSR , "jsr" , "boo" , 0, STOP | BRANCH); def(RET , "ret" , "bi" , 0, STOP); def(TABLESWITCH , "tableswitch" , "" , -1, STOP); def(LOOKUPSWITCH , "lookupswitch" , "" , -1, STOP); @@ -663,10 +663,12 @@ public static boolean isBranch(int opcode) { * the interpreter. Binary branches are instructions that have 2 successors - the taken and not * taken successor. * + * Note that {@link #GOTO} and {@link #JSR} are not considered a profiled if branch + * * @param opcode an opcode to test * @return {@code true} iff {@code opcode} is a binary branch instruction that is profiled */ - public static boolean isProfiledBranch(int opcode) { + public static boolean isProfiledIfBranch(int opcode) { return (flagsArray[opcode & 0xff] & IF_BRANCH_PROFILED) != 0; } diff --git a/substratevm/src/com.oracle.svm.interpreter.metadata/src/com/oracle/svm/interpreter/metadata/profile/MethodProfile.java b/substratevm/src/com.oracle.svm.interpreter.metadata/src/com/oracle/svm/interpreter/metadata/profile/MethodProfile.java index dc9e5c0d291b..949e3c15e02a 100644 --- a/substratevm/src/com.oracle.svm.interpreter.metadata/src/com/oracle/svm/interpreter/metadata/profile/MethodProfile.java +++ b/substratevm/src/com.oracle.svm.interpreter.metadata/src/com/oracle/svm/interpreter/metadata/profile/MethodProfile.java @@ -87,7 +87,7 @@ private static InterpreterProfile[] buildProfiles(ResolvedJavaMethod method) { int bci = stream.currentBCI(); int opcode = stream.currentBC(); // we can have multiple profiles for a single BCI: type, exception etc - if (Bytecodes.isProfiledBranch(opcode)) { + if (Bytecodes.isProfiledIfBranch(opcode)) { allProfiles.add(new BranchProfile(bci)); } if (Bytecodes.isTypeProfiled(opcode)) { From f10d4c4c874ed159ccf3bb8027530eb40810bda1 Mon Sep 17 00:00:00 2001 From: David Leopoldseder Date: Tue, 9 Dec 2025 09:46:52 +0100 Subject: [PATCH 15/18] ristretto: pause on other thread init profile --- .../interpreter/ristretto/profile/RistrettoProfileSupport.java | 1 + 1 file changed, 1 insertion(+) diff --git a/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/profile/RistrettoProfileSupport.java b/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/profile/RistrettoProfileSupport.java index 9e716e03cc8a..2c7b998e4f7b 100644 --- a/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/profile/RistrettoProfileSupport.java +++ b/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/profile/RistrettoProfileSupport.java @@ -145,6 +145,7 @@ public static MethodProfile profileMethodEntry(InterpreterResolvedJavaMethod iMe * another thread is initializing the compilation data, do a few more spins * until that is done and then go on */ + PauseNode.pause(); break; } default: From e175b0ea26f1fc30f70c57e5ed4498c69d373fc0 Mon Sep 17 00:00:00 2001 From: David Leopoldseder Date: Tue, 9 Dec 2025 09:56:44 +0100 Subject: [PATCH 16/18] ristretto: address review comments --- .../metadata/profile/MethodProfile.java | 3 -- .../oracle/svm/interpreter/Interpreter.java | 52 +++++++------------ .../ristretto/meta/RistrettoMethod.java | 29 ++++++----- .../profile/RistrettoProfileSupport.java | 15 +++--- .../profile/RistrettoProfilingInfo.java | 14 ++--- 5 files changed, 50 insertions(+), 63 deletions(-) diff --git a/substratevm/src/com.oracle.svm.interpreter.metadata/src/com/oracle/svm/interpreter/metadata/profile/MethodProfile.java b/substratevm/src/com.oracle.svm.interpreter.metadata/src/com/oracle/svm/interpreter/metadata/profile/MethodProfile.java index 949e3c15e02a..cd4dff33e8c7 100644 --- a/substratevm/src/com.oracle.svm.interpreter.metadata/src/com/oracle/svm/interpreter/metadata/profile/MethodProfile.java +++ b/substratevm/src/com.oracle.svm.interpreter.metadata/src/com/oracle/svm/interpreter/metadata/profile/MethodProfile.java @@ -65,9 +65,6 @@ public final class MethodProfile { private final ResolvedJavaMethod method; - /** - * See {@link #isMature()}. - */ private boolean isMature; public MethodProfile(ResolvedJavaMethod method) { diff --git a/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/Interpreter.java b/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/Interpreter.java index 7f268d9a6da6..29ed36c21712 100644 --- a/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/Interpreter.java +++ b/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/Interpreter.java @@ -886,17 +886,14 @@ private static Object executeBodyFromBCI(InterpreterFrame frame, InterpreterReso case IFGE: // fall through case IFGT: // fall through case IFLE: - if (takeBranchPrimitive1(popInt(frame, top - 1), curOpcode)) { - if (methodProfile != null) { - methodProfile.profileBranch(curBCI, true); - } + final boolean branchTaken1 = takeBranchPrimitive1(popInt(frame, top - 1), curOpcode); + if (methodProfile != null) { + methodProfile.profileBranch(curBCI, branchTaken1); + } + if (branchTaken1) { top += ConstantBytecodes.stackEffectOf(IFLE); curBCI = beforeJumpChecks(frame, curBCI, BytecodeStream.readBranchDest2(code, curBCI), top); continue loop; - } else { - if (methodProfile != null) { - methodProfile.profileBranch(curBCI, false); - } } break; @@ -906,49 +903,40 @@ private static Object executeBodyFromBCI(InterpreterFrame frame, InterpreterReso case IF_ICMPGE: // fall through case IF_ICMPGT: // fall through case IF_ICMPLE: - if (takeBranchPrimitive2(popInt(frame, top - 1), popInt(frame, top - 2), curOpcode)) { - if (methodProfile != null) { - methodProfile.profileBranch(curBCI, true); - } + final boolean branchTaken2 = takeBranchPrimitive2(popInt(frame, top - 1), popInt(frame, top - 2), curOpcode); + if (methodProfile != null) { + methodProfile.profileBranch(curBCI, branchTaken2); + } + if (branchTaken2) { top += ConstantBytecodes.stackEffectOf(IF_ICMPLE); curBCI = beforeJumpChecks(frame, curBCI, BytecodeStream.readBranchDest2(code, curBCI), top); continue loop; - } else { - if (methodProfile != null) { - methodProfile.profileBranch(curBCI, false); - } } break; case IF_ACMPEQ: // fall through case IF_ACMPNE: - if (takeBranchRef2(popObject(frame, top - 1), popObject(frame, top - 2), curOpcode)) { - if (methodProfile != null) { - methodProfile.profileBranch(curBCI, true); - } + final boolean branchTakenRef2 = takeBranchRef2(popObject(frame, top - 1), popObject(frame, top - 2), curOpcode); + if (methodProfile != null) { + methodProfile.profileBranch(curBCI, branchTakenRef2); + } + if (branchTakenRef2) { top += ConstantBytecodes.stackEffectOf(IF_ACMPNE); curBCI = beforeJumpChecks(frame, curBCI, BytecodeStream.readBranchDest2(code, curBCI), top); continue loop; - } else { - if (methodProfile != null) { - methodProfile.profileBranch(curBCI, false); - } } break; case IFNULL: // fall through case IFNONNULL: - if (takeBranchRef1(popObject(frame, top - 1), curOpcode)) { - if (methodProfile != null) { - methodProfile.profileBranch(curBCI, true); - } + final boolean branchTakenRef1 = takeBranchRef1(popObject(frame, top - 1), curOpcode); + if (methodProfile != null) { + methodProfile.profileBranch(curBCI, branchTakenRef1); + } + if (branchTakenRef1) { top += ConstantBytecodes.stackEffectOf(IFNONNULL); curBCI = beforeJumpChecks(frame, curBCI, BytecodeStream.readBranchDest2(code, curBCI), top); continue loop; - } else { - if (methodProfile != null) { - methodProfile.profileBranch(curBCI, false); - } } break; diff --git a/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/meta/RistrettoMethod.java b/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/meta/RistrettoMethod.java index 185d79196e91..ea419045483c 100644 --- a/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/meta/RistrettoMethod.java +++ b/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/meta/RistrettoMethod.java @@ -100,24 +100,25 @@ public static RistrettoMethod create(InterpreterResolvedJavaMethod interpreterMe public MethodProfile getProfile() { if (profile == null) { - /* - * Allocate the profile once per method. Apart from test scenarios the profile is never - * set to null again. Thus, the heavy locking code below is normally not run in a fast - * path. - */ - synchronized (this) { - if (profile == null) { - MethodProfile newProfile = new MethodProfile(this); - // ensure everything is allocated and initialized before we signal the barrier - // for the publishing write - MembarNode.memoryBarrier(MembarNode.FenceKind.STORE_STORE); - profile = newProfile; - } - } + initializedProfile(); } return profile; } + /** + * Allocate the profile once per method. Apart from test scenarios the profile is never set to + * null again. Thus, the heavy locking code below is normally not run in a fast path. + */ + private synchronized void initializedProfile() { + if (profile == null) { + MethodProfile newProfile = new MethodProfile(this); + // ensure everything is allocated and initialized before we signal the barrier + // for the publishing write + MembarNode.memoryBarrier(MembarNode.FenceKind.STORE_STORE); + profile = newProfile; + } + } + public synchronized void resetProfile() { profile = null; } diff --git a/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/profile/RistrettoProfileSupport.java b/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/profile/RistrettoProfileSupport.java index 2c7b998e4f7b..fb789284ee5e 100644 --- a/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/profile/RistrettoProfileSupport.java +++ b/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/profile/RistrettoProfileSupport.java @@ -117,12 +117,12 @@ public static MethodProfile profileMethodEntry(InterpreterResolvedJavaMethod iMe * happens on most of the frequently executed cases here. */ switch (oldState) { - /* - * profile has not been initialized yet for this method, do so by switching to - * INITIALIZING and then wait, if another thread went to initializing in the - * meantime we are done - */ case RistrettoConstants.COMPILE_STATE_INIT_VAL: { + /* + * The profile has not been initialized yet for this method. Do so by switching + * to INITIALIZING. If another thread transitioned to initializing in the + * meantime we are done. + */ methodEntryInitCase(iMethod, rMethod); break; } @@ -190,9 +190,8 @@ private static void methodEntryInitCase(InterpreterResolvedJavaMethod iMethod, R if (COMPILATION_STATE_UPDATER.compareAndSet(rMethod, RistrettoConstants.COMPILE_STATE_INIT_VAL, RistrettoConstants.COMPILE_STATE_INITIALIZING)) { trace(RistrettoRuntimeOptions.JITTraceCompilationQueuing, "[Ristretto Compile Queue]Entering state %s for %s%n", RistrettoCompileStateMachine.toString(COMPILATION_STATE_UPDATER.get(rMethod)), iMethod); - while (!COMPILATION_STATE_UPDATER.compareAndSet(rMethod, RistrettoConstants.COMPILE_STATE_INITIALIZING, RistrettoConstants.COMPILE_STATE_NEVER_COMPILED)) { - // spin until we are done writing - PauseNode.pause(); + if (!COMPILATION_STATE_UPDATER.compareAndSet(rMethod, RistrettoConstants.COMPILE_STATE_INITIALIZING, RistrettoConstants.COMPILE_STATE_NEVER_COMPILED)) { + throw GraalError.shouldNotReachHere("We set transition to COMPILE_STATE_INITIALIZING, we must be allowed to set it to COMPILE_STATE_NEVER_COMPILED"); } // continue to the NEVER_COMPILED state trace(RistrettoRuntimeOptions.JITTraceCompilationQueuing, "[Ristretto Compile Queue]Finished setting state %s for %s%n", diff --git a/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/profile/RistrettoProfilingInfo.java b/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/profile/RistrettoProfilingInfo.java index b16d5d6aaaff..0d0536e5900a 100644 --- a/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/profile/RistrettoProfilingInfo.java +++ b/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/profile/RistrettoProfilingInfo.java @@ -35,9 +35,11 @@ import jdk.vm.ci.meta.TriState; public class RistrettoProfilingInfo implements ProfilingInfo { - private final MethodProfile methodProfile; + private static final double PROB_DELTA = 1e-6; + private final MethodProfile methodProfile; + public RistrettoProfilingInfo(MethodProfile methodProfile) { this.methodProfile = methodProfile; } @@ -119,11 +121,6 @@ public boolean isMature() { return methodProfile.isMature() || methodProfile.getProfileEntryCount() > RistrettoRuntimeOptions.JITProfileMatureInvocationThreshold.getValue(); } - @Override - public String toString() { - return "RistrettoProfilingInfo<" + this.toString(null, "; ") + ">"; - } - @Override public void setMature() { methodProfile.setMature(true); @@ -138,4 +135,9 @@ public boolean setCompilerIRSize(Class irType, int nodeCount) { public int getCompilerIRSize(Class irType) { return -1; } + + @Override + public String toString() { + return "RistrettoProfilingInfo<" + this.toString(null, "; ") + ">"; + } } From f5ef36952f0baafa38e11cf019f1d961829fc3e4 Mon Sep 17 00:00:00 2001 From: David Leopoldseder Date: Tue, 9 Dec 2025 10:25:27 +0100 Subject: [PATCH 17/18] ristretto: fix typo --- .../svm/interpreter/ristretto/meta/RistrettoMethod.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/meta/RistrettoMethod.java b/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/meta/RistrettoMethod.java index ea419045483c..75894c1e33ca 100644 --- a/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/meta/RistrettoMethod.java +++ b/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/meta/RistrettoMethod.java @@ -100,7 +100,7 @@ public static RistrettoMethod create(InterpreterResolvedJavaMethod interpreterMe public MethodProfile getProfile() { if (profile == null) { - initializedProfile(); + initializeProfile(); } return profile; } @@ -109,7 +109,7 @@ public MethodProfile getProfile() { * Allocate the profile once per method. Apart from test scenarios the profile is never set to * null again. Thus, the heavy locking code below is normally not run in a fast path. */ - private synchronized void initializedProfile() { + private synchronized void initializeProfile() { if (profile == null) { MethodProfile newProfile = new MethodProfile(this); // ensure everything is allocated and initialized before we signal the barrier From cbff6cde003cb3a630be92f47ad4efd866721ce5 Mon Sep 17 00:00:00 2001 From: David Leopoldseder Date: Tue, 9 Dec 2025 10:28:36 +0100 Subject: [PATCH 18/18] ristretto: do not use GraalError in profile code that is called from the interpreter --- .../ristretto/profile/RistrettoProfileSupport.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/profile/RistrettoProfileSupport.java b/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/profile/RistrettoProfileSupport.java index fb789284ee5e..419092f6eb1c 100644 --- a/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/profile/RistrettoProfileSupport.java +++ b/substratevm/src/com.oracle.svm.interpreter/src/com/oracle/svm/interpreter/ristretto/profile/RistrettoProfileSupport.java @@ -31,6 +31,7 @@ import com.oracle.svm.core.log.Log; import com.oracle.svm.core.option.RuntimeOptionKey; +import com.oracle.svm.core.util.VMError; import com.oracle.svm.interpreter.metadata.CremaResolvedJavaMethodImpl; import com.oracle.svm.interpreter.metadata.InterpreterResolvedJavaMethod; import com.oracle.svm.interpreter.metadata.profile.MethodProfile; @@ -40,7 +41,6 @@ import com.oracle.svm.interpreter.ristretto.meta.RistrettoMethod; import jdk.graal.compiler.api.replacements.Fold; -import jdk.graal.compiler.debug.GraalError; import jdk.graal.compiler.nodes.PauseNode; public class RistrettoProfileSupport { @@ -149,7 +149,7 @@ public static MethodProfile profileMethodEntry(InterpreterResolvedJavaMethod iMe break; } default: - throw GraalError.shouldNotReachHere("Unknown state " + oldState); + throw VMError.shouldNotReachHere("Unknown state " + oldState); } oldState = COMPILATION_STATE_UPDATER.get(rMethod); } while (true); @@ -191,7 +191,7 @@ private static void methodEntryInitCase(InterpreterResolvedJavaMethod iMethod, R trace(RistrettoRuntimeOptions.JITTraceCompilationQueuing, "[Ristretto Compile Queue]Entering state %s for %s%n", RistrettoCompileStateMachine.toString(COMPILATION_STATE_UPDATER.get(rMethod)), iMethod); if (!COMPILATION_STATE_UPDATER.compareAndSet(rMethod, RistrettoConstants.COMPILE_STATE_INITIALIZING, RistrettoConstants.COMPILE_STATE_NEVER_COMPILED)) { - throw GraalError.shouldNotReachHere("We set transition to COMPILE_STATE_INITIALIZING, we must be allowed to set it to COMPILE_STATE_NEVER_COMPILED"); + throw VMError.shouldNotReachHere("We set transition to COMPILE_STATE_INITIALIZING, we must be allowed to set it to COMPILE_STATE_NEVER_COMPILED"); } // continue to the NEVER_COMPILED state trace(RistrettoRuntimeOptions.JITTraceCompilationQueuing, "[Ristretto Compile Queue]Finished setting state %s for %s%n",