lookup: answer the branchless-binary-search TODO — it is a wash - #168
Open
psiha wants to merge 3 commits into
Open
lookup: answer the branchless-binary-search TODO — it is a wash#168psiha wants to merge 3 commits into
psiha wants to merge 3 commits into
Conversation
…ough is_simple_comparator answers a semantic question - may == replace the double-negation equivalence test - and linear_search_eligible reads it as a cost one. They are different properties, and a common comparator shape separates them: a key that is a handle, ordered by fetching what it points at. Such a comparator is exactly as "simple" while every comparison is a scattered load instead of a read of the cache line the scan is already walking, so a trait that says nothing about locality is selecting the strategy whose whole economics is locality. That is a defect whichever strategy turns out to win. So state the cost question separately, as is_direct_comparator, and have linear_search_eligible require both. std::less/greater, the ranges forms and the erasure adapters name the key type's own ordering, so they read the keys and nothing else - every comparator currently reaching the linear path through a built-in specialisation stays on it. The default for everything else is std::lower_bound, and that is a judgement rather than a measurement: against the benchmark's indirect comparator at 512-byte nodes the two strategies trade places by ~14% in opposite directions - binary ahead on lookup, the scan ahead on insert - both about nine times the in-run noise floor. The comments say so, so a consumer with its own measurement declares rather than inherits. The benchmark's indirect_less keeps its is_simple_comparator specialisation, which is accurate, and declares itself non-direct. PSI_VM_BENCH_INDIRECT_LINEAR=1 claims the directness it does not have, which is the only route back to the scan and the reason that arm is a knob rather than a value: it exists to be measured. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
map_memory() threads the reserved pool onto the free list through free(), and free() writes each node's header and marks it dirty. At that point the bit says something untrue: the tree has only just been created, no COW clone of it exists, and the storage already holds what those nodes say. commit_to() copies every dirty node, so the first commit of the tree's first clone copied the entire reserved pool. It then looked fine forever after, because commit_to() clears the bit in its target and later clones inherit the cleaned state - which is exactly why a single-commit test cannot see this. Measured with a counter in commit_to() over map_memory(20000) plus 100 inserted values: the first commit copied 170 nodes, 169 of which held no values at all, to move one node of real change. With the bits cleared it copies 2. Cleared in map_memory() only. reserve()/reserve_additional() can run on a clone that genuinely owes its new pool nodes to a target, so the same reasoning does not hold there. nodes_dirty() joins nodes_used()/nodes_reserved() so the property is assertable rather than only observable from a probe, and the test drives two commit cycles so a regression cannot hide behind the self-correction described above. Note what the test does NOT assert: dirty <= used is not an invariant, because taking a node off the free list also rewrites its successor's back-link (new_node -> unlink_right), so a single allocation legitimately dirties two nodes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
b+tree/impl.hpp carried a TODO for a branchless binary search
(Alexandrescu's TLC and friends). This implements it and measures it, so
the TODO can stop being an open question: on the two ISAs available it
buys nothing, and the default stays off.
branchless_{lower,upper}_bound do the same halving as std::lower_bound
with both arms of each step written as a select rather than a jump. The
b+tree's binary arm now goes through binary_{lower,upper}_bound, which is
std:: unless PSI_VM_BRANCHLESS_BINARY_SEARCH is set.
Measured in one process, both forms back to back on the same data, so
there are no A/B arms and no cross-process noise to defend
(lookup.branchless_vs_std_binary). x86-64, 8..1024 values, resident:
every type scatters within about +-5% with no systematic sign - uint16,
uint32, uint64, float and double alike. In-tree on AArch64, 4 blocks per
arm against a same-config duplicate arm, every column lands inside a
0.35-2.5% noise floor.
The float case is worth stating because it was the one predicted to win:
x86 clang emits a branchy compare for floating point and a cmov for
integers, so that is where an explicit branchless form had something to
remove. It does not show up. Note float and double cost 13-41 ns per
probe against 3-9 for the integers - the FP comparison chain dominates,
and taking branches out does not shorten it.
Kept rather than dropped: the knob and the instrument are what make the
answer reproducible on the next ISA, and the correctness tests are worth
having regardless. Those compare against std:: over lengths 0..1000 with
duplicates - so lower_bound must find the first of a run and upper_bound
one past the last - and probe outside the range on both sides, which is
where the off-by-one in the "go right" step would show.
Full suite: 2082 passed, 1 pre-existing skip, 0 failed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Base automatically changed from
fix/map-memory-clean-free-pool
to
bt/10-comparator-cost
September 11, 2026 20:28
psiha
force-pushed
the
bt/10-comparator-cost
branch
from
September 11, 2026 21:11
f044dad to
32784a2
Compare
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.
Stacked on #166. Closes out the
TODO branchless binary searchatb+tree/impl.hpp:295— by answering it, not by adopting it. The default staysoff.
What
branchless_lower_bound/branchless_upper_boundinlookup.hpp: the same halvingas
std::lower_bound, with both arms of each step written as a select rather than ajump. The b+tree's binary arm now goes through
binary_{lower,upper}_bound, which isstd::unlessPSI_VM_BRANCHLESS_BINARY_SEARCHis defined.Measured — in one process, no A/B arms
lookup.branchless_vs_std_binarytimes both forms back to back in the same binary onthe same data, so there is no cross-process noise floor to defend. x86-64 (Arrow
Lake-H), resident, 8..1024 values:
Scatter of roughly ±5% with no systematic sign, on every type. (uint16 behaves the
same; two isolated points — uint64 at 64 and double at 64 — read +15% and +10% with
near-zero neighbours either side, which is noise, not signal.)
In-tree on AArch64 (M1), 4 blocks per arm, order reversed on alternate blocks,
with a deliberate same-config duplicate arm as the noise floor: every column —
sequential, random, indirect, u32, u64 — lands inside a 0.35–2.5% floor.
The float case, since it is the one that was predicted to win
x86 clang emits a branchy compare for floating point (
vucomiss;jbe) and acmovforintegers, so float is where an explicit branchless form had something to remove. It
does not show up. Worth noting why: float and double cost 13–41 ns per probe against
3–9 ns for the integers — the FP comparison chain dominates, and removing branches
does not shorten a latency chain.
Why keep it rather than drop it
The knob and the instrument are what make this answer reproducible on the next ISA
instead of re-derived, and the correctness tests are worth having either way:
lookup.branchless_agrees_with_std_{uint32,uint64,float}compares againststd::overlengths 0..1000 with duplicates (so
lower_boundmust find the first of a run andupper_boundone past the last) and probes outside the range on both sides — whichis where the off-by-one in the "go right" step would surface.
Gate
Full suite, Release: 2082 passed / 1 pre-existing skip / 0 failed.
Not covered
The x86-64 in-tree arm. Its A/B came back with an 11–33% same-config noise floor —
the box was not quiet — so I am not reporting those numbers. The resident comparison
above and the AArch64 in-tree result are what this rests on.
🤖 Generated with Claude Code