fix(benchmarks): rebuild every branch-versioned module before an A/B run - #465
Merged
Merged
Conversation
The A/B scripts installed only the engine, but the benchmark module resolves render-pdf and templates — plus the templates tests-classifier jar, which is never published — at the branch's own version. Both sides therefore took those two jars from ~/.m2, and whenever the compared branches share a version, which is the normal case for two branches cut from develop, they took the *same* ones: any change in the PDF backend or the templates module measured a delta of exactly zero. A stale backend jar built against the other branch's core could also throw NoSuchMethodError mid-run instead of reporting a number. Both modules are now installed per branch, guarded by the presence of their pom so a pre-split layout, where the root build produces them, still works. Mirrors the install sequence the CI performance job already uses.
Owner
Author
|
The |
…piling render-pdf's tests The A/B section still told readers each branch is rebuilt with install -pl :graph-compose-core "so the benchmark measures that branch's engine" - the sentence the install fix invalidates, and the only place stating what gets rebuilt. render-pdf now installs with -Dmaven.test.skip=true rather than -DskipTests. Only its main jar is needed, but -DskipTests still compiles its whole test tree, so a branch whose render-pdf tests do not compile would abort the entire A/B at the install step with the output sent to /dev/null. templates keeps -DskipTests, because the benchmark resolves its tests-classifier jar and maven.test.skip would not produce one.
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
1 participant
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.
Why
The A/B scripts installed only the engine module, but
benchmarks/pom.xmlresolvesgraph-compose-render-pdfandgraph-compose-templates— plus the templates tests-classifier jar, which is deliberately never published — at${graphcompose.version}, i.e. the branch's own version.So both sides of an A/B took those two jars from
~/.m2. When the compared branches share a version, which is the normal case for two branches cut fromdevelop, they took the same ones: any change in the PDF backend or the templates module measured a delta of exactly zero. A stale backend jar built against the other branch's core could also throwNoSuchMethodErrormid-run instead of reporting a number.What
scripts/ab-bench.shandscripts/run-benchmarks.ps1now installrender-pdfandtemplatesper branch alongside the engine, each guarded by the presence of its pom so a pre-split layout — where the root build produces them — still works. This mirrors the install sequence the CI performance job already uses (.github/workflows/ci.yml:346-361).graph-compose-fontsis left alone deliberately: it tracks an independent version line, so it does not change with the branch under test.Tests
Both scripts parse clean (
bash -n, PowerShell AST parser).The install logic itself was exercised end to end across the layout boundary while investigating engine performance: 8 build-and-measure cycles over
v1.8.0,v1.9.1,v2.0.0,v2.1.0anddevelop, in a detached worktree.v1.8.0predates the module split and has norender-pdf/ortemplates/directory, so it takes the guarded-skip path; the 2.x refs take the install path. All eight produced benchmark output.scripts/ab-bench.shitself was not run end to end here — its own CI smoke (ab-bench-smoke.yml) covers that and fires on changes to the script.