Repository navigation
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Empty JMH result files can be accepted as successful forks, while runner and harness-version metadata remain inconsistent.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Stabilizes scoredirector performance runs on GitHub-hosted ARM runners by alternating forks, merging results, and reporting paired measurement resolution.
Changes:
- Adds fork-result merging and paired confidence-resolution reporting.
- Adds per-fork execution, retries, JDK validation, and deferred flame-graph generation.
- Updates benchmark launchers and JVM setup.
File summaries
| File | Description |
|---|---|
summarize-scoredirector.py |
Reports paired resolution and updated verdict annotations. |
src/main/java/ai/timefold/solver/benchmarks/micro/scoredirector/Main.java |
Adds fork/flame-graph modes and heap pre-touching. |
src/main/java/ai/timefold/solver/benchmarks/micro/common/AbstractMain.java |
Updates JFR converter imports. |
run-scoredirector.sh |
Supports configurable JDK and benchmark JAR. |
merge-scoredirector-results.py |
Merges single-fork JMH results and recomputes statistics. |
.github/workflows/performance_score_director.yml |
Runs alternating forks, validates results, and archives outputs. |
Review details
Suppressed comments (1)
.github/workflows/performance_score_director.yml:304
- Pinning this checkout to
github.shadoes not guarantee that the benchmark binaries use the same harness revision: both build jobs later switchtimefold-solver-benchmarksto the baseline/SUT branch when it exists (the branch-switch blocks around lines 140-156 and 235-250). The embeddedMainand benchmark code can therefore differ from this checkout and from each other, so a harness change can still confound the comparison. Keep the build and runtime harness revision consistent for this workflow.
path: ./timefold-solver-benchmarks
# The harness and the binaries have to come from one commit, for the same reason
# build_baseline gives: pinned to a branch, a harness fix on any other branch
# was silently never exercised by this workflow.
- Files reviewed: 6/6 changed files
- Comments generated: 5
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| "conf_lo": _to_float(conf[0]), | ||
| "conf_hi": _to_float(conf[1]), | ||
| # One value for each fork, in the order they ran, which is what pairs the two sides. | ||
| "forks": [_to_float(value) for fork in metric.get("rawData", []) for value in fork], |
| ./run-scoredirector.sh --skip-flamegraphs > "$LOG" 2>&1 || true | ||
| # JMH does not fail the process when a benchmark trial errors out - it just omits it | ||
| # from the results, so a crashed run can still exit 0. | ||
| if [ -s "$RESULT_FILE" ]; then |
| score = statistics.fmean(fork_means) | ||
| error = t_quantile(CONFIDENCE_TAIL, fork_count - 1) \ | ||
| * statistics.stdev(fork_means) / math.sqrt(fork_count) | ||
| ordered = sorted(value for fork in raw_data for value in fork) |
| # JMH forks its children from the host process, so pinning the host pins every fork. | ||
| # BENCHMARK_JAR lets the caller alternate between two binaries without copying a file | ||
| # between measurements. Both default to what a plain local run expects. | ||
| "${JAVA_HOME:+$JAVA_HOME/bin/}java" -cp "${BENCHMARK_JAR:-target/benchmarks.jar}" \ |
| cancel-in-progress: true | ||
| needs: [ decisions, build_baseline, build_sut ] | ||
| runs-on: self-hosted # We need a stable machine to actually run the benchmarks. | ||
| runs-on: ubuntu-24.04-arm # Github-hosted Arm runner; free and GA for this public repo. |
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.