Skip to content

Commit e7b357c

Browse files
authored
feat(benchmarks): gate the layout pass on exact measurement counts (#469)
* feat(benchmarks): gate the layout pass on exact measurement counts Nothing in CI could tell a slower engine from a slower machine. The timing smoke run compares against absolute millisecond ceilings set about three times above observed values, so a regression that stays under the ceiling passes, and no job compares a branch against its own base. MeasurementCountBenchmark already answered that question deterministically and nothing ran it: it wraps the text measurement system in a counting decorator and reports, as exact integers, how many width measurements a document costs. Those counts are identical on every machine and move only when the engine changes. MeasurementCountGateTest now asserts them per scenario. It lands in the module whose whole test suite CI already runs as the deterministic-gates step, so it gates every performance-relevant pull request without a workflow change. A second test requires each probe scenario to carry a recorded expectation, so a new scenario cannot be measured and silently ungated. The gate is complementary to the layout snapshots rather than a duplicate: two implementations that place fragments identically but measure a different number of times satisfy every snapshot, and only this separates them. Verified by measuring one token twice in the wrapping loop - long-text goes from 4472 to 8552 requests and the gate fails with both records printed. The probe's scenario list moves into a shared accessor so the probe and the gate cannot drift apart. Allocation stays ungated: it is stable to about 0.1% across runs on one JVM but is a property of the JDK as much as of the engine, so an expectation recorded on one machine would be a guess about another. * test(benchmarks): report every changed scenario in one run Asserting inside the scenario loop stopped at the first mismatch, so a deliberate engine change surfaced one scenario's new numbers per run and the author had to re-run three more times to collect the rest. Soft assertions report all four together.
1 parent eee6f2f commit e7b357c

4 files changed

Lines changed: 177 additions & 19 deletions

File tree

.github/workflows/ci.yml

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -362,8 +362,10 @@ jobs:
362362

363363
- name: Run deterministic benchmark gates
364364
# Fast, machine-independent unit/gate tests (image-cache reuse,
365-
# render-operator coalescing, scenario/threshold coverage, diff tooling).
366-
# Catches structural regressions the timing smoke run cannot.
365+
# render-operator coalescing, measurement counts, scenario/threshold
366+
# coverage, diff tooling). Catches structural regressions the timing
367+
# smoke run cannot: these are exact integers, so unlike a millisecond
368+
# ceiling they separate a slower engine from a slower machine.
367369
run: ./mvnw -B -ntp -f benchmarks/pom.xml test
368370

369371
- name: Run coarse performance smoke benchmark

benchmarks/src/main/java/com/demcha/compose/MeasurementCountBenchmark.java

Lines changed: 41 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,9 @@
2020
import java.time.LocalDateTime;
2121
import java.time.format.DateTimeFormatter;
2222
import java.util.ArrayList;
23+
import java.util.LinkedHashMap;
2324
import java.util.List;
25+
import java.util.Map;
2426
import java.util.function.Consumer;
2527

2628
/**
@@ -93,13 +95,7 @@ private void run() throws Exception {
9395
System.out.println("Thread allocation measurement: " + (allocationSupported() ? "enabled" : "UNAVAILABLE (Alloc KB = n/a)"));
9496
System.out.println();
9597

96-
Consumer<PageFlowBuilder> longText = flow ->
97-
flow.addParagraph(p -> p.text(LONG_PARAGRAPH).textStyle(BODY_STYLE));
98-
Consumer<PageFlowBuilder> longToken = flow ->
99-
flow.addParagraph(p -> p.text(LONG_TOKEN_PARAGRAPH).textStyle(BODY_STYLE));
100-
Consumer<PageFlowBuilder> accentedText = flow ->
101-
flow.addParagraph(p -> p.text(ACCENTED_LATIN_PARAGRAPH).textStyle(BODY_STYLE));
102-
Consumer<PageFlowBuilder> largeTable = MeasurementCountBenchmark::authorLargeTable;
98+
Map<String, Consumer<PageFlowBuilder>> scenarios = scenarios();
10399

104100
// Warm up the JVM (class loading + JIT) BEFORE the allocation window so the
105101
// "Alloc KB" column reflects steady-state per-document layout allocation, not
@@ -108,17 +104,15 @@ private void run() throws Exception {
108104
// layout cost (verified: cold first compile 36.6 MB vs warm 0.65 MB for the
109105
// same long-text document). The measurement-COUNT columns are exact either way.
110106
for (int warmup = 0; warmup < 5; warmup++) {
111-
measureScenario("warmup", longText);
112-
measureScenario("warmup", longToken);
113-
measureScenario("warmup", accentedText);
114-
measureScenario("warmup", largeTable);
107+
for (Consumer<PageFlowBuilder> author : scenarios.values()) {
108+
measureScenario("warmup", author);
109+
}
115110
}
116111

117112
List<Result> results = new ArrayList<>();
118-
results.add(measureScenario("long-text", longText));
119-
results.add(measureScenario("long-token", longToken));
120-
results.add(measureScenario("accented-latin", accentedText));
121-
results.add(measureScenario("large-table", largeTable));
113+
for (Map.Entry<String, Consumer<PageFlowBuilder>> scenario : scenarios.entrySet()) {
114+
results.add(measureScenario(scenario.getKey(), scenario.getValue()));
115+
}
122116

123117
System.out.printf("%-14s | %11s | %9s | %9s | %11s | %8s | %11s | %10s | %6s%n",
124118
"Scenario", "WidthReqs", "Distinct", "Repeat %", "Sum chars", "Max arg", "LineMetrics", "Alloc KB", "Pages");
@@ -140,7 +134,37 @@ private void run() throws Exception {
140134
writeReport(results);
141135
}
142136

143-
private Result measureScenario(String scenario, Consumer<PageFlowBuilder> author) throws Exception {
137+
/**
138+
* The probe's scenarios, in report order. Single source for the probe and for
139+
* {@code MeasurementCountGateTest}, so a scenario added here cannot slip past
140+
* the gate unnoticed.
141+
*
142+
* @return scenario name to document author
143+
*/
144+
static Map<String, Consumer<PageFlowBuilder>> scenarios() {
145+
Map<String, Consumer<PageFlowBuilder>> scenarios = new LinkedHashMap<>();
146+
scenarios.put("long-text", flow ->
147+
flow.addParagraph(p -> p.text(LONG_PARAGRAPH).textStyle(BODY_STYLE)));
148+
scenarios.put("long-token", flow ->
149+
flow.addParagraph(p -> p.text(LONG_TOKEN_PARAGRAPH).textStyle(BODY_STYLE)));
150+
scenarios.put("accented-latin", flow ->
151+
flow.addParagraph(p -> p.text(ACCENTED_LATIN_PARAGRAPH).textStyle(BODY_STYLE)));
152+
scenarios.put("large-table", MeasurementCountBenchmark::authorLargeTable);
153+
return scenarios;
154+
}
155+
156+
/**
157+
* Compiles one scenario and returns its exact measurement counts.
158+
*
159+
* <p>Package-private and static so the gate test can drive the real layout
160+
* pass rather than re-implementing the instrumentation.</p>
161+
*
162+
* @param scenario report label
163+
* @param author document author for the scenario
164+
* @return the scenario's counts, page/fragment totals and compile allocation
165+
* @throws Exception when composition or measurement fails
166+
*/
167+
static Result measureScenario(String scenario, Consumer<PageFlowBuilder> author) throws Exception {
144168
try (DocumentSession session = GraphCompose.document()
145169
.pageSize(DocumentPageSize.A4)
146170
.margin(24, 24, 24, 24)
@@ -244,7 +268,7 @@ private void writeReport(List<Result> results) throws Exception {
244268
System.out.println("Saved CSV counter report to " + csvPath);
245269
}
246270

247-
private record Result(String scenario,
271+
record Result(String scenario,
248272
CountingTextMeasurementSystem.Counts counts,
249273
int pages,
250274
int fragments,
Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,115 @@
1+
package com.demcha.compose;
2+
3+
import org.assertj.core.api.SoftAssertions;
4+
import org.junit.jupiter.api.Test;
5+
6+
import java.util.LinkedHashMap;
7+
import java.util.Map;
8+
9+
import static org.assertj.core.api.Assertions.assertThat;
10+
11+
/**
12+
* Deterministic regression gate for how much measurement work the layout pass
13+
* does, driving {@link MeasurementCountBenchmark}'s real compile through a
14+
* {@link CountingTextMeasurementSystem}.
15+
*
16+
* <p>These counts are exact integers, not timings. A wall-clock benchmark can
17+
* only say "slower on this machine today"; it cannot separate a slower laptop
18+
* from a slower engine, and an absolute millisecond ceiling generous enough to
19+
* survive CI variance is generous enough to miss a real regression. Asking the
20+
* layout pass how many width measurements it requested has neither problem: the
21+
* answer is identical on every machine and moves only when the engine's
22+
* behaviour actually changes.</p>
23+
*
24+
* <p>Complementary to the layout snapshots rather than a duplicate of them. Two
25+
* implementations that place fragments identically but measure a different
26+
* number of times both satisfy every snapshot; only this gate separates
27+
* them.</p>
28+
*
29+
* <p><b>When this fails</b>, the engine changed how it measures text. Decide
30+
* whether that was intended: fewer requests for the same document is an
31+
* improvement worth recording, more is a regression worth explaining. Either
32+
* way the fix is to run {@code MeasurementCountBenchmark} and update the
33+
* expectation below in the same commit as the behaviour change, so the number
34+
* moves deliberately.</p>
35+
*
36+
* <p>Allocation is deliberately not gated. It is stable to about 0.1% across
37+
* runs on one JVM, but it is a property of the JDK as much as of the engine, so
38+
* an expectation recorded on one machine would be a guess about another. The
39+
* probe still reports it.</p>
40+
*/
41+
class MeasurementCountGateTest {
42+
43+
/**
44+
* Exact measurement work per scenario, recorded from
45+
* {@link MeasurementCountBenchmark} on the 2.1.1 development line.
46+
*/
47+
private static final Map<String, Work> EXPECTED = expected();
48+
49+
private static Map<String, Work> expected() {
50+
Map<String, Work> work = new LinkedHashMap<>();
51+
work.put("long-text", new Work(4472, 32, 32457, 124, 1, 2));
52+
work.put("long-token", new Work(99, 55, 7739, 600, 1, 1));
53+
work.put("accented-latin", new Work(2499, 37, 14393, 123, 1, 1));
54+
work.put("large-table", new Work(1206, 211, 5916, 13, 0, 6));
55+
return work;
56+
}
57+
58+
/**
59+
* The measurement work one scenario costs the layout pass.
60+
*
61+
* @param widthRequests total text-width measurements requested
62+
* @param distinctWidthRequests distinct (style, text) pairs among them
63+
* @param summedRequestChars characters across all requests
64+
* @param maxRequestChars longest single measured string
65+
* @param lineMetricsCalls line-metric lookups
66+
* @param pages pages the document compiled to
67+
*/
68+
private record Work(long widthRequests,
69+
long distinctWidthRequests,
70+
long summedRequestChars,
71+
long maxRequestChars,
72+
long lineMetricsCalls,
73+
int pages) {
74+
75+
static Work of(MeasurementCountBenchmark.Result result) {
76+
CountingTextMeasurementSystem.Counts counts = result.counts();
77+
return new Work(counts.widthRequests(),
78+
counts.distinctWidthRequests(),
79+
counts.summedRequestChars(),
80+
counts.maxRequestChars(),
81+
counts.lineMetricsCalls(),
82+
result.pages());
83+
}
84+
}
85+
86+
@Test
87+
void everyScenarioMeasuresExactlyAsMuchTextAsRecorded() throws Exception {
88+
// Soft, so a deliberate engine change reports all four scenarios' new
89+
// numbers in one run instead of surfacing them one failure at a time.
90+
SoftAssertions soft = new SoftAssertions();
91+
for (var scenario : MeasurementCountBenchmark.scenarios().entrySet()) {
92+
Work actual = Work.of(
93+
MeasurementCountBenchmark.measureScenario(scenario.getKey(), scenario.getValue()));
94+
95+
soft.assertThat(actual)
96+
.describedAs("measurement work for '%s' changed - re-run "
97+
+ "MeasurementCountBenchmark and update the expectation "
98+
+ "in the same commit if the change was intended",
99+
scenario.getKey())
100+
.isEqualTo(EXPECTED.get(scenario.getKey()));
101+
}
102+
soft.assertAll();
103+
}
104+
105+
/**
106+
* A scenario added to the probe without an expectation would be measured and
107+
* silently ignored, which is the failure mode that lets coverage rot.
108+
*/
109+
@Test
110+
void everyProbeScenarioIsGated() {
111+
assertThat(MeasurementCountBenchmark.scenarios().keySet())
112+
.describedAs("probe scenarios missing a recorded expectation")
113+
.containsExactlyInAnyOrderElementsOf(EXPECTED.keySet());
114+
}
115+
}

docs/operations/benchmarks.md

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,23 @@ Changing the engine (layout, pagination, render ordering, PDF session, text
200200
measurement, fonts) and want to see how it moves performance? Pick the view that
201201
fits, cheapest first:
202202

203+
- **"Did the engine change how much work it does?" — the deterministic gate.**
204+
`MeasurementCountGateTest` compiles four fixtures and asserts the exact number
205+
of text measurements each one costs. These are integers, identical on every
206+
machine, so they answer the question a timing run cannot: whether the *engine*
207+
changed or just the hardware it ran on. It runs on every performance-relevant
208+
PR as part of the `Run deterministic benchmark gates` step, and locally in a
209+
second:
210+
211+
```bash
212+
./mvnw -B -ntp test -Dtest=MeasurementCountGateTest -f benchmarks/pom.xml
213+
```
214+
215+
A failure prints the recorded and actual counts side by side. Fewer requests
216+
for the same document is an improvement worth recording, more is a regression
217+
worth explaining; either way, re-run `MeasurementCountBenchmark` and update the
218+
expectation in the same commit as the behaviour change.
219+
203220
- **"Did I regress?" — gate against the committed baseline.** Run a median and
204221
let the `11-verdict-current-speed` step score each scenario IMPROVED /
205222
NEUTRAL / REGRESSED against `baselines/current-speed-full.json` (hard gate:

0 commit comments

Comments
 (0)