address review comments and style fixes

Author: davleopo

compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/core/test/ReportHotMetricsTest.java

@@ -59,7 +59,8 @@ public static int snippet01(int limit1) {
     public void testAbsNegate() {
         ByteArrayOutputStream bos = new ByteArrayOutputStream();
         try (TTY.Filter ttyFilter = new TTY.Filter(new LogStream(bos))) {
-            OptionValues opt = new OptionValues(getInitialOptions(), BasePhase.PhaseOptions.ReportHotMetrics, true, BasePhase.PhaseOptions.ReportHotMetricsAfterPhases, "BoxNodeIdentity=*.snippet01");
+            OptionValues opt = new OptionValues(getInitialOptions(), BasePhase.PhaseOptions.ReportHotMetrics, "*.snippet01", BasePhase.PhaseOptions.ReportHotMetricsAfterPhases,
+                            "BoxNodeIdentity=*.snippet01");
             test(opt, "snippet01", 100);
         }
         // the missing profile

compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/core/common/CompilationIdentifier.java

@@ -79,6 +79,9 @@ public CompilationRequest getRequest() {
      */
     String toString(Verbosity verbosity);
 
+    /**
+     * Get a JavaMethod representation of the current compilation.
+     */
     default JavaMethod asJavaMethod() {
         return null;
     }

compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/phases/BasePhase.java

@@ -47,6 +47,7 @@
 import jdk.graal.compiler.debug.GraalError;
 import jdk.graal.compiler.debug.GraphFilter;
 import jdk.graal.compiler.debug.MemUseTrackerKey;
+import jdk.graal.compiler.debug.MethodFilter;
 import jdk.graal.compiler.debug.TTY;
 import jdk.graal.compiler.debug.TimerKey;
 import jdk.graal.compiler.graph.Graph;
@@ -207,9 +208,9 @@ public static class PhaseOptions {
         @Option(help = "Exclude certain phases from compilation based on the given phase filter(s)." + PhaseFilterKey.HELP, type = OptionType.Debug)
         public static final PhaseFilterKey CompilationExcludePhases = new PhaseFilterKey(null, null);
         @Option(help = "Report hot metrics after each phase matching the given phase filter(s).", type = OptionType.Debug)
-        public static final OptionKey<String> ReportHotMetricsAfterPhases = new OptionKey<>(null);;
+        public static final OptionKey<String> ReportHotMetricsAfterPhases = new OptionKey<>(null);
         @Option(help = "Report hot metrics before each phase matching the given phase filter(s).", type = OptionType.Debug)
-        public static final OptionKey<String> ReportHotMetricsBeforePhases =  new OptionKey<String>("HighTierLoweringPhase");
+        public static final OptionKey<String> ReportHotMetricsBeforePhases =  new OptionKey<>("HighTierLoweringPhase");
         @Option(help = "Report hot metrics extracted from compiler IR.", type = OptionType.Debug)
         public static final OptionKey<String> ReportHotMetrics = new OptionKey<>(null);
         // @formatter:on
@@ -461,8 +462,8 @@ public final void apply(final StructuredGraph graph, final C context, final bool
             String reportHotMetricsMethodFilter = PhaseOptions.ReportHotMetrics.getValue(options);
             boolean logHotMetricsForGraph = false;
             if (reportHotMetricsMethodFilter != null) {
-                jdk.graal.compiler.debug.MethodFilter hotMetricsMethodFilter = null;
-                hotMetricsMethodFilter = jdk.graal.compiler.debug.MethodFilter.parse(reportHotMetricsMethodFilter);
+                MethodFilter hotMetricsMethodFilter = null;
+                hotMetricsMethodFilter = MethodFilter.parse(reportHotMetricsMethodFilter);
                 logHotMetricsForGraph = graph.method() != null && hotMetricsMethodFilter.matches(graph.method());
                 if (!logHotMetricsForGraph) {
                     CompilationIdentifier id = graph.compilationId();

compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/phases/common/ReportHotCodePhase.java

@@ -29,6 +29,8 @@
 import java.util.List;
 import java.util.Optional;
 
+import org.graalvm.word.LocationIdentity;
+
 import jdk.graal.compiler.debug.TTY;
 import jdk.graal.compiler.graph.Node;
 import jdk.graal.compiler.graph.NodeSourcePosition;
@@ -52,6 +54,7 @@
 import jdk.graal.compiler.options.Option;
 import jdk.graal.compiler.options.OptionKey;
 import jdk.graal.compiler.options.OptionType;
+import jdk.graal.compiler.options.OptionValues;
 import jdk.graal.compiler.phases.BasePhase;
 import jdk.graal.compiler.phases.schedule.SchedulePhase;
 
@@ -77,9 +80,13 @@ public static class Options {
         @Option(help = "Dumps the hottest code parts to the Ideal Graph Visualizer (IGV) for further analysis and visualization.", type = OptionType.Debug)
         public static final OptionKey<Boolean> ReportHotCodePartsToIGV = new OptionKey<>(false);
         @Option(help = "Specifies the debug level for dumping hottest code parts to the Ideal Graph Visualizer (IGV).", type = OptionType.Debug)
-        public static final OptionKey<Integer> ReportHotCodIGVLevel = new OptionKey<>(1);
+        public static final OptionKey<Integer> ReportHotCodeIGVLevel = new OptionKey<>(1);
         @Option(help = "Specifies the minimum relative frequency for reporting hot code regions.", type = OptionType.Debug)
-        public static final OptionKey<Double> MinimalFrequencyToReport = new OptionKey<>(1D);
+        public static final OptionKey<Double> ReportHotCodeMinimalFrequencyToReport = new OptionKey<>(1D);
+        @Option(help = "Enables printing of informational messages in hot code regions.", type = OptionType.Debug)
+        public static final OptionKey<Boolean> ReportHotCodeInfos = new OptionKey<>(false);
+        @Option(help = "Enables printing of warning messages about potential performance issues in hot code regions.", type = OptionType.Debug)
+        public static final OptionKey<Boolean> ReportHotCodeWarnings = new OptionKey<>(true);
         //@formatter:on
     }
 
@@ -178,11 +185,17 @@ private static int getLengthCap(int len, int cap) {
         return Math.min(len, cap);
     }
 
-    private static void info(String format, Object... args) {
+    private static void info(OptionValues opt, String format, Object... args) {
+        if (!Options.ReportHotCodeInfos.getValue(opt)) {
+            return;
+        }
         TTY.printf(INFO_KEY + format, args);
     }
 
-    private static void warn(String format, Object... args) {
+    private static void warn(OptionValues opt, String format, Object... args) {
+        if (!Options.ReportHotCodeWarnings.getValue(opt)) {
+            return;
+        }
         TTY.printf(WARNING_KEY + format, args);
     }
 
@@ -196,16 +209,15 @@ protected void run(StructuredGraph graph, C c) {
         }
 
         CoreProviders context = (CoreProviders) c;
+        final OptionValues options = graph.getOptions();
 
         SchedulePhase.runWithoutContextOptimizations(graph, SchedulePhase.SchedulingStrategy.LATEST_OUT_OF_LOOPS, true);
         final StructuredGraph.ScheduleResult scheduleResult = graph.getLastSchedule();
 
-        final double minimalReportFrequency = Options.MinimalFrequencyToReport.getValue(graph.getOptions());
+        final double minimalReportFrequency = Options.ReportHotCodeMinimalFrequencyToReport.getValue(graph.getOptions());
 
         final LoopsData ld = context.getLoopsDataProvider().getLoopsData(scheduleResult.getCFG());
         final ControlFlowGraph cfg = ld.getCFG();
-        // report the 3 hottest blocks and the 3 hottest loops and 3 hottest loops by local loop
-        // frequency
         final List<HIRBlock> hottestBlocks = new ArrayList<>();
 
         Collections.addAll(hottestBlocks, cfg.reversePostOrder());
@@ -223,9 +235,9 @@ protected void run(StructuredGraph graph, C c) {
             String hottestGlobalBlocksString = String.format("Hottest %s blocks are %s %s %s", getLengthCap(hottestFirstBlocks.size(), REPORT_HOT_FIRST_N), hottestFirstBlocks,
                             blocksToString(hottestFirstBlocks, BlockToStringMode.BEGIN_NODE),
                             blocksToString(hottestFirstBlocks, BlockToStringMode.GLOBAL_FREQUENCY));
-            info("%s\n", hottestGlobalBlocksString);
+            info(options, "%s%n", hottestGlobalBlocksString);
             if (Options.ReportHotCodePartsToIGV.getValue(graph.getOptions())) {
-                graph.getDebug().dump(Options.ReportHotCodIGVLevel.getValue(graph.getOptions()), graph, hottestGlobalBlocksString);
+                graph.getDebug().dump(Options.ReportHotCodeIGVLevel.getValue(graph.getOptions()), graph, hottestGlobalBlocksString);
             }
         }
 
@@ -234,9 +246,9 @@ protected void run(StructuredGraph graph, C c) {
             String hottestLocalLoopString = String.format("Hottest %s local loops are %s %s", getLengthCap(hottestFirstLocalLoops.size(), REPORT_HOT_FIRST_N),
                             loopBlocksToString(hottestFirstLocalLoops, LoopToStringMode.BLOCK),
                             loopBlocksToString(hottestFirstLocalLoops, LoopToStringMode.LOCAL_FREQUENCY));
-            info("%s\n", hottestLocalLoopString);
+            info(options, "%s%n", hottestLocalLoopString);
             if (Options.ReportHotCodePartsToIGV.getValue(graph.getOptions())) {
-                graph.getDebug().dump(Options.ReportHotCodIGVLevel.getValue(graph.getOptions()), graph, hottestLocalLoopString);
+                graph.getDebug().dump(Options.ReportHotCodeIGVLevel.getValue(graph.getOptions()), graph, hottestLocalLoopString);
             }
         }
 
@@ -245,9 +257,9 @@ protected void run(StructuredGraph graph, C c) {
             String hottestGlobalLoopString = String.format("Hottest %s global loops are %s %s", getLengthCap(hottestGlobalLoops.size(), REPORT_HOT_FIRST_N),
                             loopBlocksToString(hottestFirstGlobalLoops, LoopToStringMode.BLOCK),
                             loopBlocksToString(hottestFirstGlobalLoops, LoopToStringMode.GLOBAL_FREQUENCY));
-            info("%s\n", hottestGlobalLoopString);
+            info(options, "%s%n", hottestGlobalLoopString);
             if (Options.ReportHotCodePartsToIGV.getValue(graph.getOptions())) {
-                graph.getDebug().dump(Options.ReportHotCodIGVLevel.getValue(graph.getOptions()), graph, hottestGlobalLoopString);
+                graph.getDebug().dump(Options.ReportHotCodeIGVLevel.getValue(graph.getOptions()), graph, hottestGlobalLoopString);
             }
         }
 
@@ -281,12 +293,17 @@ protected void run(StructuredGraph graph, C c) {
     private static void reportUnknownProfile(Loop l, Node inside, ControlFlowGraph cfg) {
         if (inside instanceof IfNode ifNode) {
             if (ifNode.profileSource().isUnknown()) {
+                final OptionValues optionValues = inside.getOptions();
                 NodeSourcePosition nsp = ifNode.getNodeSourcePosition();
                 if (nsp == null) {
-                    warn("Unknown profile for %s with f=%s in hot loop %s, NO NODE SOURCE POSITION%n\tPotential Action Item: Determine lack of node source position and profile.%n", ifNode,
+                    warn(optionValues,
+                                    "Unknown profile for %s with relativeFrequency=%s in hot loop %s, NO NODE SOURCE POSITION%n\tPotential Action Item: Determine lack of node source position and profile.%n",
+                                    ifNode,
                                     cfg.blockFor(inside).getRelativeFrequency(), l);
                 } else {
-                    warn("Unknown profile for %s with f=%s in hot loop %s, nsp is %n%s%n\tPotential Action Item: Add profile to the top-of-stack source location.%n", ifNode,
+                    warn(optionValues,
+                                    "Unknown profile for %s with relativeFrequency=%s in hot loop %s, node source position is %n%s%n\tPotential Action Item: Add profile to the top-of-stack source location.%n",
+                                    ifNode,
                                     cfg.blockFor(inside).getRelativeFrequency(), l,
                                     ifNode.getNodeSourcePosition().toString("\t"));
                 }
@@ -306,14 +323,15 @@ private static void reportInvariantLoopIf(Loop l, Node inside) {
         if (inside instanceof IfNode ifNode) {
             LogicNode logicNode = ifNode.condition();
             if (!l.whole().contains(logicNode)) {
-                warn("If %s with condition %s is inside loop %s while condition is not%n\tPotential Action Item: Determine why compiler does not unswitch the loop.%n", ifNode, logicNode, l);
+                warn(logicNode.getOptions(), "If %s with condition %s is inside loop %s while condition is not%n\tPotential Action Item: Determine why compiler does not unswitch the loop.%n", ifNode,
+                                logicNode, l);
             }
         }
     }
 
     /**
      * Reports memory kill operations within a specified hot loop that target
-     * {@link org.graalvm.word.LocationIdentity.AnyLocationIdentity}.
+     * {@link LocationIdentity#any()}.
      * <p>
      * A memory kill to {@code AnyLocationIdentity} signifies a write or invalidation that is
      * treated as affecting any possible memory location. Such "any" kills prevent the compiler from
@@ -332,10 +350,11 @@ private static void reportMemoryKillANYInLoop(Loop l, Node inside, ControlFlowGr
                 if (sk.getKilledLocationIdentity().isAny()) {
                     if (inside instanceof FixedNode) {
                         // else we don't have a cfg position
-                        warn("Node %s kills any and has relative f=%s  in loop %s %n\tPotential Action Item: Determine if operation is required and replace with less intrusive memory effect if possible.%n",
+                        warn(inside.getOptions(),
+                                        "Node %s kills any and has relative relativeFrequency=%s  in loop %s %n\tPotential Action Item: Determine if operation is required and replace with less intrusive memory effect if possible.%n",
                                         inside, cfg.blockFor(inside).getRelativeFrequency(), l);
                     } else {
-                        warn("Node %s kills any in loop %s %n", inside, l);
+                        warn(inside.getOptions(), "Node %s kills any in loop %s %n", inside, l);
                     }
                 }
             }
@@ -347,10 +366,10 @@ private static void reportMemoryKillANYInLoop(Loop l, Node inside, ControlFlowGr
      * {@code y} argument is eligible for speculative guard movement.
      * <p>
      * Speculative guard movement, as described in
-     * {@link jdk.graal.compiler.loop.phases.SpeculativeGuardMovementPhase.SpeculativeGuardMovement},
-     * attempts to optimize execution by moving eligible guards outside the loop when safe.
-     * Typically, such guards should be speculatively optimized and floated before the loop to
-     * reduce overhead inside hot code regions.
+     * {@link jdk.graal.compiler.loop.phases.SpeculativeGuardMovementPhase}, attempts to optimize
+     * execution by moving eligible guards outside the loop when safe. Typically, such guards should
+     * be speculatively optimized and floated before the loop to reduce overhead inside hot code
+     * regions.
      * </p>
      * <p>
      * This report helps identify guards that remain inside loops and may represent missed
@@ -379,8 +398,9 @@ private static void reportHotLoopGuardsInside(Node inside, Loop loop) {
             }
 
             if (iv != null && limit != null) {
-                warn("Guard %s condition %s inside loop with iv %s and limit %s%n\tPotential Action Item: Determine why speculative guard movement does not consider them for optimization.%n", inside,
-                                compare, iv, limit);
+                warn(inside.getOptions(),
+                                "Guard %s condition %s inside loop with iv %s and limit %s%n\tPotential Action Item: Determine why speculative guard movement does not consider them for optimization.%n",
+                                inside, compare, iv, limit);
             }
         }
     }

truffle/docs/Optimizing.md

@@ -805,7 +805,7 @@ The most common causes are:
 
 The `--engine.TraceCompilation` option also shows CallTarget invalidations with an `[engine] opt inv.` prefix.
 
-## Debugging Compiler Optimization Potential
+## Uncovering Optimization Barriers: Compiler-Level Insights for Truffle Apps
 
 Once you are comfortable profiling Truffle applications and analyzing results (see [Profiling](Profiling.md)), you may reach a point where you wonder, “Why is this benchmark slower than expected?” 
 After determining that the majority of execution time is spent in compiled code, the next step is a deeper analysis of the generated machine code.
@@ -826,19 +826,16 @@ To enable reporting of these metrics, launch your application with:
 ```
 This prints warnings and hot spots to `stdout` for all compilation units.
 
-To narrow output to specific methods or functions use known method names like:
+To narrow the output to specific methods or functions, use known method names like `*wasm-function:5311*`:
 ```
 --vm.Djdk.graal.ReportHotMetrics='*wasm-function:5311*'
 ```
-To improve the quality of the generated data run with node source position tracking `--vm.Djdk.graal.TrackNodeSourcePosition=true`.
+To improve the quality of the generated data, run with node source position tracking `--vm.Djdk.graal.TrackNodeSourcePosition=true`.
 
 ### Example Output
 ```
 Reporting hot metrics before HighTierLoweringPhase during compilation of wasm-function:5311
-[Hot Code Info] Hottest 3 blocks are [B18, B21, B19] BeginNodeIDs=[4951258|LoopBegin, 823|Merge, 707|Begin] GlobalFrequencies=[241.99984874999717, 241.99984874999717, 120.99992437499859]
-[Hot Code Info] Hottest 1 local loops are Blocks=[Loop (depth=1) 4951258|LoopBegin] LocalLoopFrequencies=[241.99999999999716]
-[Hot Code Info] Hottest 1 global loops are Blocks=[Loop (depth=1) 4951258|LoopBegin] BasicBlockFrequencies=[241.99984874999717]
-[Hot Code Warning] Unknown profile for 704|If with f=241.99984874999717 in hot loop Loop (depth=1) 4951258|LoopBegin, nsp is 
+[Hot Code Warning] Unknown profile for 704|If with f=241.99984874999717 in hot loop Loop (depth=1) 4951258|LoopBegin, node source position is 
         at org.graalvm.wasm.nodes.WasmFunctionNode.executeBodyFromOffset(WasmFunctionNode.java:363) [bci: 1654]
         at org.graalvm.wasm.nodes.WasmFunctionNode.execute(WasmFunctionNode.java:241) [bci: 15]
         [...cut for brevity...]
@@ -850,7 +847,7 @@ Reporting hot metrics before HighTierLoweringPhase during compilation of wasm-fu
   * “Local frequency” is the number of times the block is executed per loop iteration.
   * “Global frequency” is across the entire method. A value of 1 means the basic block executes once per method invocation.
 * `[Hot Code Warning]` denotes locations or IR patterns that may inhibit optimization.
-  * Warnings include, for example, missing profile data in hot paths (e.g., `unknown profile for 704|If …`), which is a common actionable warning.
+  * Warnings include, for example, missing profile data in hot paths (for example, `unknown profile for 704|If …`), which is a common actionable warning.
 
 Some warnings may be actionable, while others are for informational purposes. If you encounter frequent or unexplained compiler warnings in your hottest methods and are unsure how to proceed, please reach out to the Truffle community via our Slack channels for advice.