Skip to content

Commit

Permalink
[GR-48937] Fix reasons for triggered CompilationAlarms due to no prog…
Browse files Browse the repository at this point in the history
…ress.

PullRequest: graal/15689
  • Loading branch information
rmosaner committed Oct 9, 2023
2 parents e23b390 + a4d96e5 commit 15e7e23
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -188,10 +188,10 @@ private static void assertProgress(OptionValues opt, EventCounter counter) {
assertProgressNoTracking(opt, counter);
return;
} else {
final double noProgressStartDetectionPriod = noProgressStartPeriod.get() * 1000;
final double noProgressStartDetectionPeriod = noProgressStartPeriod.get() * 1000;
final long currentTimeStamp = System.currentTimeMillis();
final long timeDiff = currentTimeStamp - lastUniqueStackTraceTimeStamp;
final boolean noProgressForPeriodStartDetection = timeDiff > noProgressStartDetectionPriod;
final boolean noProgressForPeriodStartDetection = timeDiff > noProgressStartDetectionPeriod;
if (!noProgressForPeriodStartDetection) {
/*
* Still not enough no-progress before we start doing something.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import java.util.regex.Pattern;

import org.graalvm.collections.EconomicMap;
import org.graalvm.compiler.core.common.util.CompilationAlarm;
import org.graalvm.compiler.debug.CounterKey;
import org.graalvm.compiler.debug.DebugCloseable;
import org.graalvm.compiler.debug.DebugContext;
Expand Down Expand Up @@ -458,6 +459,13 @@ public final void apply(final StructuredGraph graph, final C context, final bool
debug.verify(graph, "%s", getName());
}
assert graph.verify();
/*
* Reset the progress-based compilation alarm to ensure that progress tracking happens
* for each phase in isolation. This prevents false alarms where the same progress state
* is seen in subsequent phases, e.g., during graph verification a the end of each
* phase.
*/
CompilationAlarm.resetProgressDetection();
} catch (Throwable t) {
throw debug.handle(t);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import static org.graalvm.compiler.microbenchmarks.graal.util.GraalUtil.getGraph;
import static org.graalvm.compiler.microbenchmarks.graal.util.GraalUtil.getMethodFromMethodSpec;

import org.graalvm.compiler.core.common.util.CompilationAlarm;
import org.graalvm.compiler.debug.DebugContext;
import org.graalvm.compiler.nodes.StructuredGraph;
import org.openjdk.jmh.annotations.Level;
Expand Down Expand Up @@ -76,6 +77,11 @@ protected StructuredGraph preprocessOriginal(StructuredGraph structuredGraph) {

@Setup(Level.Invocation)
public void beforeInvocation() {
/*
* [GR-48937] Reset the progress-based compilation alarm for the jmh thread, because it can
* falsely assume that this thread is stuck during graph copying.
*/
CompilationAlarm.resetProgressDetection();
graph = (StructuredGraph) originalGraph.copy(originalGraph.getDebug());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,10 @@

import org.openjdk.jmh.annotations.Level;
import org.openjdk.jmh.annotations.Scope;
import org.openjdk.jmh.annotations.Setup;
import org.openjdk.jmh.annotations.State;
import org.openjdk.jmh.annotations.TearDown;

import org.graalvm.compiler.core.common.util.CompilationAlarm;
import org.graalvm.compiler.graph.Node;
import org.graalvm.compiler.graph.NodeClass;
import org.graalvm.compiler.nodes.StructuredGraph;
Expand Down Expand Up @@ -107,4 +108,14 @@ public void afterInvocation() {
}
}
}

@Setup(Level.Invocation)
public void beforeInvocation() {
/*
* [GR-48937] Reset the progress-based compilation alarm, because repeated invocations of
* methods which include progress detection (i.e. node.inputs()) on the same graph
* throughout many benchmark invocations can falsely detect endless loops.
*/
CompilationAlarm.resetProgressDetection();
}
}

0 comments on commit 15e7e23

Please sign in to comment.