perf(light): three changes to the propagation loop from item 2 of Open - #36
Conversation
…back Both propagators tested the occlusion of a neighbour before its level. The level is one array read; the occlusion is two and a branch, and the level rejects far more often — a position is reached from up to six directions and only the first of them raises it. The two are pure, so the order is free to choose and the cheaper, more selective one goes first. A queued position also carries the face pointing back at whoever queued it, in the bits above its index, and the loop skips that face. Whoever queued it holds a level exactly one higher, so the level test could never pass in that direction; the work was always wasted rather than merely redundant. Seeds carry a face value no face uses, so nothing is skipped for them. Neither changes a single output byte. Storing the travelled face instead of its opposite — the plausible way to get this wrong — turns 20 tests red, so the suite does cover the direction rather than only the levels.
The chunk propagator reached the occlusion of a neighbour through sections.get(y >> 4) followed by a call into that section, once per face per queued position — an interface call, a bounds check and a null test before the byte anyone wanted. The column is now written into one flat byte[] in prepare(), and the search reads it directly with a precomputed face bit. The layout is free: a section indexes its table with the same (y << 8) | (z << 4) | x the column uses, so a section occupies one contiguous 4096-byte run and goes in with a single arraycopy. A uniform section, which holds no table at all, is filled instead — still the cheaper of the two cases, so the shortcut that makes uniform sections free is not given up, only paid for differently. The buffer is sized and reused like levels and queue, so it costs one byte per position of the largest column an instance has seen and nothing per run. copyOcclusionInto is package private. It hands out the internal layout of the table for the two propagators of this package and is not API. Filling the column with the sections in reverse — the plausible way to get an offset wrong — turns 20 tests red.
Test results 198 files 198 suites 3m 56s ⏱️ Results for commit 237d60a. |
Measured through the workflow
The controls first
The wide-looking numbers are noise at small absolute values or in single cells, and saying so is not The changed code
Every one of those intervals is disjoint from its baseline, and every one clears the control by a
The wiki's rebuild figure for the flat column was −26 % on searching a whole column. The chunk The one place it does not win
It is worth flagging rather than averaging away, because 4 sections is exactly where the flat column Still open: which commit did whatBoth commits are in this measurement together, so it does not say whether the flat column earns its |
The split run was contaminated, and its own control says soThe third run —
Two things worth saying about that: The merged result is unaffected. It came from runs 30758921758 and 30758926454, both on an EPYC The split is simply not established yet. The uncontaminated part of the third run — the A re-run is in flight. If it holds up, the open question it answers is a real one and worth the CI This is also the second time today a benchmark comparison was saved by having an untouched method in |
The split, on a clean run — and it found a real regressionRe-run of The section propagator: the flat column contributes nothing, as it should
The flat column lives in The chunk propagator: the flat column earns its buffer
Between −8.2 % and −14.5 % on top of the free changes, on every configuration. The 96 KB buffer is And the case I said to watch is a real regression
At four sections the flat column makes the sky pass slower, and this time both configurations Note it is sky only — block light at four sections still gains −9.6 % from the flat column. So What I would do about it, and why not a revertKeep it, and record the losing case rather than average it away. The trade in absolute terms: at four sections the sky pass costs +22 µs; at twenty-four it saves The targeted fix — leave Say the word if you want the |
1 participant
The three entries of the last bullet under
Project Status → Open → Smaller items:
That last sentence is why this is two commits rather than one: the cheap, certain changes are
separated from the one that trades memory and setup cost for a faster inner loop, so the measurement
can tell them apart.
3b74e04— order the two tests by cost, and stop walking backBoth propagators tested a neighbour's occlusion before its level. The level is one array read; the
occlusion is two and a branch. The level also rejects far more often — a position is reached from up
to six directions and only the first raises it — so the cheaper and more selective test goes first.
Both predicates are pure, so the order is free to choose.
A queued position now also carries, in the bits above its index, the face pointing back at whoever
queued it, and the loop skips that face. This is not a heuristic: whoever queued the position holds a
level exactly one higher, so
levels[neighbour] >= nextcould never fail in that direction. The workwas always wasted, not merely redundant. Seeds carry a face value no face uses.
237d60a— lay the occlusion of a column out flatsections.get(y >> 4).blocksFace(…)ran once per face per queued position: an interface call, abounds check and a null test before the byte anyone wanted.
prepare()now writes the whole columninto one flat
byte[]and the search reads it with a precomputed face bit.The layout falls out for free — a section indexes its table with the same
(y << 8) | (z << 4) | xthe column uses, so each section is one contiguous 4096-byte run and goes in with a single
arraycopy. A uniform section holds no table at all and is filled instead, so the shortcut thatmakes uniform sections cheap is not given up, only paid for differently. That is the part of this
commit worth watching: it converts a per-read saving into a per-pass cost, and whether that trade
pays is exactly what the rebuild figure could not say.
copyOcclusionIntois package private — it hands out the internal layout of the table for the twopropagators of this package and is not API.
Correctness
Neither commit changes an output byte, and both were checked by breaking them rather than by
assertion:
20 tests red
20 tests red
So the suite covers direction and layout, not only levels.
./gradlew buildis green.Measurement
LightEngineStageBenchmarkis the right instrument here because it carries its own controls:falcoPropagateis what these commits change, whilefalcoOpacity,falcoReadStatesandfalcoCollectare untouched stages measured in the same jar and the same job.69381afused exactlythat structure.
ChunkLightPropagatorBenchmarkruns alongside it for the whole-column case.Running through the
Benchmarkworkflow,customprofile, three forks, onmainand on thisbranch. Numbers follow in a comment — including if they say one of these should be reverted. The
flat column is the one with a plausible way to lose.