Spend memory to save work in the group walk - #22
Open
Roxxik wants to merge 2 commits into
Open
Conversation
nodes_value becomes one byte per node instead of a bitmap. It is read 126 million times per cbmbasic run as get_nodes_value(c.gate) while scanning transistors, and the whole array is 1725 bytes, so the shift and mask cost more than the space they save. A run to READY. takes on the order of ten thousand cache misses in total, so the packing was buying space that was never scarce. nodes_base caches each node's pullup/pulldown contribution, which addNodeToGroup previously read from both bitmaps on all 80 million group-walk steps. Pullups and pulldowns only change in setNode() and writeNodes(), a handful of nodes per half-cycle. Every update to val is a max, so the three compares collapse into one. Allocations go from 88,842 bytes to 92,076: nodes_value grows from 216 bytes to 1725, and nodes_base adds another 1725. The working set comment is updated to match. cbmbasic to READY. is 9.3% fewer cycles, minimum of five runs with turbo disabled. measure is byte-identical over all 256 opcodes.
A group was stored twice: as an array plus a count, and as a redundant bitmap over all 1725 nodes so that group_contains was O(1). Every one of the 79.9 million addNodeToGroup calls a cbmbasic run makes opens with a bitmap test, and every one that proceeds sets a bit that group_clear has to clear again. Scanning the array instead makes group_add a single store and group_clear an assignment. The search runs over one or two entries that were just written. This is quadratic in the group size, which averages 1.45 and peaks at 54. cbmbasic to READY. is 8.8% fewer cycles, minimum of five runs with turbo disabled. measure is byte-identical over all 256 opcodes.
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.
Two changes to how node state is stored, both trading space this program is not
short of for operations it performs tens of millions of times.
A cbmbasic run to
READY.takes on the order of ten thousand cache misses intotal, against 2.9 billion cycles. The working set is 89 KB and lives in L2. The
bit-packing in this file was buying space that was never scarce, at the cost of a
shift and a mask on the hottest path in the program.
One byte per node value, and a precomputed base
nodes_valuebecomes one byte per node. It is read 126 million times per run asget_nodes_value(c.gate)while scanning transistors, and the whole array is 1725bytes.
Separately, each node's pullup/pulldown contribution is precomputed into
nodes_base. Pullups and pulldowns only change insetNode()andwriteNodes(), a handful of nodes per half-cycle, yetaddNodeToGroupread bothbitmaps on all 80 million group-walk steps. Every update to the running value is
a max, so the three separate compares collapse into one.
Answer group membership by scanning the group
A group was stored twice: as an array plus a count, and as a redundant bitmap
over all 1725 nodes so that
group_containswas O(1). Every one of the 79.9million
addNodeToGroupcalls a run makes opens with a bitmap test, and everyone that proceeds sets a bit that
group_clearlater has to clear again.Set that against what the structure actually holds. Two thirds of the 30 million
groups built are a single node, 98% are three or fewer, and the mean is 1.45.
Four cache lines of bitmap and three operations per node visited were being spent
to answer "is this node one of the one or two I have so far".
Dropping the bitmap makes
group_adda single store andgroup_clearanassignment. The search runs over one or two entries that were just written. The
walk becomes quadratic in the group size, which is the point: it is quadratic in
1.45. Groups of eight or more occur 132 times in a whole run and the largest is
54 nodes, so a full scan per insertion costs on the order of 200K comparisons
against the 210M bitmap operations removed.
Verification
measureis byte-identical over all 256 opcodes, and cbmbasic reachesREADY.at the same half-cycle with the same end state.
Beyond the architectural state, every one of the 1725 nodes is folded into an
FNV-1a digest every 16 cycles and compared against a master build. Both suites
agree exactly:
3bbda10b350a759fover 17,946 samples of Tom Harte'sSingleStepTests, and
2e3d527522bdd11dover 125,000 samples of the first twomillion cycles of Klaus Dormann's functional test. Changing how a value is stored
should not change the value, and this is the check that says it did not. Harte
itself is 67,159 passed, 1,711 unstable on die-dependent opcodes, 0 failed,
unchanged from master.
While developing, a build that kept the bitmap and asserted the two membership
answers agreed on every call ran the same 256 opcodes clean.
Performance
cbmbasic to
READY., turbo disabled. Wall clock is the minimum of fifteeninterleaved runs, counters the minimum of five:
Split between the two commits, on cycles against each one's own parent: the
byte-per-node value and the precomputed base are -9.3%, and dropping the
membership bitmap on top of that is -8.8%.
The price is 3,018 bytes. Allocations go from 88,842 to 91,860:
nodes_valuegrows from 216 bytes to 1725,
nodes_baseadds another 1725, and the groupbitmap gives 216 back. The working set comment in
setupNodesAndTransistorsisupdated from 89 KB to 92 KB.
Note for review
This subsumes #17.
group_clearno longer readsgroupcountat all, so theuninitialised read that fix addresses stops existing rather than getting
initialised. valgrind reports it on a master build and is silent on this one.
The two do not conflict textually and either order works.
I used an LLM to help me out in this work, but I manually reviewed all changes made.