Repository navigation
perf(core)!: two Ir campaigns, released as 0.3.0 (BREAKING: ClientRecord field types) - #11
Merged
Merged
Conversation
`serde_json` now resolves to Remade-With-Rust/rusty_json_turbo, a performance
fork of serde_json whose output is byte-identical to upstream's by contract --
gated per commit against upstream serde_json linked in the same process, on
bytes, `Value`, error text with line and column, float bit patterns and
`StreamDeserializer` offsets. Its LIBRARY is still named `serde_json`, so not
one `use serde_json::...` line in this repository changes.
WHY A PATCH AND NOT A RENAMED DEPENDENCY. Writing
`serde_json = { package = "rusty_json_turbo", ... }` would convert only the
JSON this repository calls itself and leave every third-party copy (axum,
cozo, dioxus, candle-transformers, lambda_http, spacedb-crdt) on upstream --
and the two would then be DIFFERENT TYPES, so a `Value` handed across that
boundary would not compile. A patch converts every copy at once and leaves
exactly one `Value` in the graph. It needs a package named `serde_json`
because `[patch]` matches on PACKAGE name; the fork carries one for exactly
this purpose (`crates/serde_json-shim`).
VALIDATED: 217 tests green across the workspace, and all 124 JSON fixtures in
the repository (6,465 number tokens) checked byte-identical against upstream
through the fork's differential oracle -- 0 mismatches.
The lockfile is intentionally NOT part of this commit: it re-resolves against
the git remote on the first fetch. Verify the swap actually took effect with
cargo tree -i serde_json
which must show the fork. A stale lock pin makes cargo skip the patch with
only a WARNING in the middle of a successful build, so this is checked rather
than assumed; `cargo update serde_json` is the fix.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Measured with callgrind Ir (exact: same binary re-runs to 0), via the three
benches already in the tree for exactly this. client_path 219,439,512 ->
197,467,368, -21,972,144 Ir (-10.01%). hot_path and mru_report unchanged.
All three checksums, every work-parity anchor, 217 tests, clippy -D warnings,
fmt, and the no_std leaf on both bare-metal targets are unchanged.
A1 -17,918,178 (-8.17%) regress called wls_fit(&used) twice on identical
data -- `fit` above it already held the result and nothing between them
touched `used`. The most expensive helper in the hottest function in the
crate, run for nothing on every estimate reaching the adaptive block.
Accounting closes: regress -4.9M, residual_sd -10.5M, satellites -2.4M.
A5 -3,363,768 (-1.67%) a median taken by full sort_by now uses
select_nth_unstable_by -- the same f64, byte-identical. Honest label: the
branch does NOT execute at the default dispersion_k = 0, so this is a dead
stable-sort instantiation leaving the binary, i.e. a codegen/layout win
that can reverse, not work removal.
A6 -690,184 (-0.34%) 2f64.powi(k) -> exact exponent-field construction.
16,000 __powidf2 calls per run, gone. server.rs:390 had already made this
exact fix with a comment saying so; it never reached client.rs,
discipline.rs or refclock.rs.
Also pinned RUSTY_TIME_HASH_SEED in the measurement harness: unpinned, an
UNCHANGED binary drifted 2,794 Ir on hot_path and 1,006 on mru_report, wider
than most wins here. client_path has no client table and was already exact,
which is what named the cause.
Eight probes were refuted and reverted, each with its number in
docs/plans/instruction-veins.md: A2 (+629,281 and +1,331,400), A3 (-12,228,455
but REFUSED -- it moved the `plans` anchor 16029 -> 16025; sxx = sum(wt^2) -
sw*t0^2 loses ~14 of 16 digits on clustered timestamps), A9 (+1,754,122), A10
(+584,293), A11 (+1,428,602, the fourth refutation of a shape the source had
already refuted three times), C19 (+1,659,861), C20 (+741,504, premise
disproved -- the push count stayed at 8). The clever-local-rewrite class went
0-for-6; all three wins were deletions.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…used A3's -12.2M was never a prize: the unsafe one-pass form was faster because sxx had collapsed to garbage and the loop took shorter paths (plans 16029 -> 16025). The numerically safe origin (u = t - samples[0].t, so the cancellation is ULP-level) is +137,561 AND still moves the anchor to 16032. Dead both ways. A8 reclassified: the edge census names 32,000 __ieee754_log_fma calls (1,494,250 Ir) as benches/client_path.rs:66 '-u.ln()'. rusty_time-core has no log at all. Third vein that was the instrument, with C19 and B17. B16 measured -1,016,192 (hot_path, -2.14%) and -164,640 (mru_report), anchors and checksums unchanged -- and is NOT shipped. The finaliser's third avalanche step is what keeps adversary-chosen sequential addresses out of adjacent buckets, which is the documented purpose of seeding this hasher. One line if the owner wants the trade. B13's blocker named: masking elides the check for any len because x & m <= m, but it needs slots.len() to be a power of two at every access, so slots must be pre-filled -- and Slot<K> has no Default. Still the largest unmined vein at 4,970,506 Ir (10.48%). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
spike_threshold derives its threshold from |residual| for every row and then threw the buffer away, so the caller recomputed all of them to filter: a subtract, a multiply, an add and an abs per row, 5.6M Ir on one line. Handing the BUFFER back was refuted twice earlier in this campaign (+629,281 indexed, +1,331,400 zipped) -- carrying a second slice through the walk costs more than the arithmetic it saves. A u64 keep mask is the cheapest possible carrier: one register, no allocation, no indirection, and the filter becomes a shift and a test. Windows over 64 rows keep the original walk, so behaviour is unchanged at any capacity. The mask must be built from |residual|s in SAMPLE order, and select_nth_unstable_by_key permutes what it is given -- which broke the checksum twice before the selection was moved onto a copy. That copy sits past both of the threshold's early returns, so it is paid only on the rare estimate that actually needs the median. Gate: checksum f456d8bf8ff664d5 and every anchor unchanged, 217 tests, clippy -D warnings, fmt, and the no_std leaf on both bare-metal targets. Campaign total so far: client_path 219,439,512 -> 195,025,671, -24,413,841 Ir (-11.13%). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…6 Ir) drain(..k) memmoves the whole remainder, and the regime-change trim can fire up to four times per estimate, so the rows that survive were shifted down again on every pass. A cursor advances instead and the buffer is reclaimed once. This is the fix SampleRegister already applies to samples/weights -- the head cursor at the top of this file, documented there as 'the same total movement spread over sixty-four times fewer operations'. The trim loop never got it; second time in this campaign that an existing fix in this repo had not reached a sibling site (see also A6's powi). I had PRUNED this on arithmetic at a <298K ceiling taken from the memcpy symbol alone. The real figure is -557,376, because the ceiling missed the drain's own length bookkeeping and the used.len() reloads the cursor also removes. A ceiling is only as wide as the terms you put in it. checksum f456d8bf8ff664d5 and every anchor unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A2 (-2,441,711) and A7 (-557,376) were a refutation and a prune in round 1 and are wins now. A2's mechanism was right and its CARRIER was wrong: a u64 keep mask instead of a second buffer. A7's ceiling was wrong: I took <298K from the memcpy symbol and missed the drain's length bookkeeping and the len() reloads. B13's 4,970,506 'ceiling' is retracted. .min(len-1) measured +2,364,380 -- slots.len() must be loaded through self per access to replace a branch the predictor always gets right. slice/index.rs attribution is address arithmetic and the load, not the check, which is codec-analyzer's law that the bounds-check tax is ~0. A4 recorded as out of this instrument's range rather than unmined: SoA cuts bytes streamed and leaves instructions per element unchanged, so Ir reads ~0 for it by construction. wls_fit inlining already optimal: inline(always) +452,629, inline hint -10 (a no-op, so LLVM had already chosen, and a free null arm). Campaign: five wins of twenty veins, client_path -24,971,217 Ir (-11.38%). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
B15 (hashbrown probe, 3.5M) was the last un-attempted buildable vein and is safe to replace: self.index is never iterated and eviction is MRU-driven, so hash order is invisible. Refused anyway, and not for budget -- the bench reports 'evicted 0', so it never removes a key and therefore cannot exercise the one failure mode a hand-rolled open-addressed table has (tombstone accumulation degrading probes under churn). Measuring at zero churn and shipping into a long-lived table with eviction is an unmeasured risk for ~1.5M Ir. Reopen condition recorded: a bench arm that drives the table past capacity. Final: five wins of twenty, client_path -24,971,217 Ir (-11.38%). Thirteen of the fifteen remaining have no win as a fact about the code -- five are the harness or load-bearing work, one is invisible to Ir by construction, one is not gateable by this harness, six measured slower. B16 is a measured win refused on the hasher's threat model. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ted 0'
HOT_PATH_EVICT sizes the client table at a quarter of the client population
instead of twice it, so the recency list churns: evicted 149,227, table_len
1024. The default arm is unchanged (evicted 0, table_len 4096).
Why it exists: no change to the client index was gateable without it. A
harness that never removes a key cannot see tombstone accumulation or probe
degradation, which is the whole risk surface of replacing the index.
What it immediately showed, none of which the steady arm could:
- churn costs 59% more per request (378 vs 237 Ir)
- hashbrown is 13.7M = 18.1% of the churn arm (8.49M raw + 1.90M tag +
3.30M sse2), against 3.5M = 7.4% steady. B15's prize is ~4x what the
steady arm priced it at.
- net/ip_addr.rs appears at 1,666,362 -- which turned out to be hashbrown's
Eq during lookup, not evict_one's key clone: destructuring self to drop
that clone measured -1 Ir, so LLVM had already elided it. Reverted.
Two harness bugs found and fixed on the way, both of the same species:
std::env::var(..).is_ok() is TRUE for an empty value, so exporting
HOT_PATH_EVICT= to mean 'off' ran both arms in churn mode; and the arm
selection reached the bare run but not the callgrind run, so the anchors came
from one arm and the Ir from the other -- two rows printed identical Ir for 0
and 149,227 evictions. Every guard must cover every arm.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…45% churn)
Replaces HashMap<K, u32, ClientHashBuilder> with a fixed-capacity,
open-addressed index using backward-shift deletion.
hot_path steady 47,424,979 -> 46,382,549 -1,042,430 (-2.20%)
hot_path churn 75,642,533 -> 70,764,432 -4,878,101 (-6.45%)
mru_report 5,804,467 -> 5,422,843 -381,624 (-6.57%)
Three simplifications, all properties of what a client table IS:
- fixed capacity, so no resize path at all (the table evicts, never grows)
- backward-shift deletion, so NO TOMBSTONES -- the one failure mode an
open-addressed table has under churn does not exist in this design
- load factor <= 0.25, so probe runs stay short
THE LOAD FACTOR IS NOT A FREE KNOB, and this is the whole reason the churn arm
had to be built first. At 0.5 the table read -789,020 on the steady arm -- a
clean win on the only instrument that existed before today -- and +7,455,786
(+9.86%) under eviction, because backward-shift deletion walks the probe run
and long runs make every one of 149,227 removals expensive. Shipped on the
steady arm alone, that is a 10% regression on the churn path that nothing would
have caught. At 0.25 both arms win.
The price is four buckets per client instead of hashbrown's one entry, and
bytes_per_client() now reports it rather than leaving a caller to find it in
RSS.
This vein was refused twice earlier in the campaign: priced at 3.5M from the
steady arm (it is really 13.7M under churn, ~4x), then declined as ungateable
because the only harness reported `evicted 0`. Both objections were about the
instrument, and both were answered by building one.
Gate: 217 tests, clippy -D warnings, fmt, the no_std leaf on both bare-metal
targets, and every checksum and anchor unchanged on all four instruments
(evicted 149,227 and table_len 1024 on the churn arm).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A4 was recorded as 'out of instrument range -- Ir is blind to locality, needs a clock'. That was wrong. REGISTER_CAPACITY is 64 and Row is 4 x f64, so the whole row buffer is 2,048 bytes -- L1-resident at the maximum capacity the register can hold. There is no locality to recover, so SoA's benefit is not invisible to Ir, it is absent, while its cost (three or four pointer increments per element instead of one) is real on every instrument. The 41.0M is iterator instruction count, and the way to reduce that is fewer passes, which A1, A2 and A7 did. codec-cache-tiles names this trap exactly; building a clock harness for it would have measured a change with nothing to find. Final: six wins of twenty. client_path -24,971,217 Ir (-11.38%); hot_path -1,042,430 steady and -4,878,101 churn; mru_report -381,624. Fourteen veins have no win and each now carries a mechanism: five are the harness or load-bearing work, one has no locality to recover, seven measured slower, one is a refusal on the hasher's threat model. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…Ir, +10 B/client)
The corpus priced the first version of this index honestly and it was a bad
trade: 150 -> 256 bytes/client, +71% memory for 2-6% CPU. A time server would
rather track more clients than save a few instructions, so that is the wrong
direction.
The cost was a DUPLICATED KEY. HashMap had to own its own copy; this index does
not need one, because the key already lives in slots[handle].key. A bucket is
now two u32s -- the low 32 bits of the hash and the handle -- so four buckets
at a 0.25 load factor cost about what hashbrown's single entry did (key + handle
+ control byte).
The hash fragment is what lets the key stay out: a non-match is rejected on the
fragment, so the dependent load into `slots` happens on a true match or a
1-in-4-billion collision. The low word is also sufficient for backward-shift
deletion, which needs only `hash & mask`.
main v1 (key in bucket) this
bytes/client 150 256 160 (+10, +6.7%)
hot_path steady -- -1,042,430 -1,760,095 (-3.71%)
hot_path churn -- -4,878,101 -7,856,405 (-10.39%)
mru_report -- -381,624 -569,584 (-9.81%)
Smaller AND faster than the version it replaces: the removed key is 24 bytes a
bucket and a copy on every insert.
Also drops evict_one's key.clone(), which only existed to dodge a borrow
between two different fields of self.
Gate: TIMECORP S1/S6/S8 at 31 seeds byte-identical to main; S12a/b/c counts
identical including 1,021,672 evictions in S12b; all four Ir instruments'
checksums and anchors unchanged; 217 tests; clippy -D warnings on lib, tests
and the three benches of mine; no_std leaf green on both bare-metal targets.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Six deterministic Ir wins in the client filter and the server's client table. client_path -24,968,715 (-11.38%); hot_path -3.71% steady and -10.39% under churn; mru_report -9.81%. No behaviour changed, and the corpus proves it rather than the commit asserting it: TIMECORP S1/S6/S8 at 31 seeds are byte-identical to 0.2.0, and S12a/b/c server-load counts match including 1,021,672 evictions in S12b. cargo semver-checks reports no semver update required, so this is a patch. bytes_per_client() now reports 160 rather than 150 -- the index holds four buckets per client instead of one entry. Signature unchanged. Also adds tools/validate-release.sh, the pre-release gate this release was run through: 17 checks over a clean git worktree of the exact commit (never a working tree, which can hold another session's uncommitted edits). Two of its own checks were wrong on the first run and are fixed: it tripped over itself in the dirty check, and it grepped for 'unsafe' across all crates when rusty_time-clock legitimately lifts that lint for the syscall boundary and the compiler is the authority anyway. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The twenty veins were closed. These ten came from the two censuses the first
five rounds never took -- call counts and caller-callee edges -- plus a reading
of the no_std codec leaf, which had no instrument at all.
Kept:
N3 ClientRecord's three Option<NtpTimestamp> -> NtpTimestamp, zero = unset
hot_path -808,139 Ir (-1.77%), 228.3 -> 224.3 Ir/request
N2a #[inline(always)] on MultiController::estimate -- single caller, single
callee, 37.2 Ir/call of pure frame
client_path -317,822 Ir (-0.16%)
N3 is the B5 pattern: a 16-byte Option over a type with no niche, where the
protocol already spells "unset" as all-zero. The origin-zero rejection was
already explicit and independent of the Option, which is what makes the
sentinel safe rather than merely convenient.
Refuted, with the numbers left in the source so they are not re-derived:
N9 from_bits as a lookup table +2,048,014 LLVM already emits
arithmetic (B5)
N8 parse via one &[u8; 48] first_chunk +0 exactly compiler already did
it; B18 was right
N9' repr(u8) + self as u8 for bits() +0
N2b bind registers[index] once +0 already CSE'd
N1 inline(always) on residual_sd +0 / +0 removed
The clever-local-rewrite class is now 0-for-8 here. Every win across six rounds
has been a deletion, a representation change, or a libm swap.
Instruments:
N7 benches/codec.rs + tools/perf/codec_ir.sh -- the third corpus shape, and
the only one that prices ntp.rs on its own. Its first tap was 63% of its
own measurement; `write` is the inverse of `parse`, so folding the six
output words covers all thirteen fields transitively (19 mixes -> 9).
N4 ir.sh and client_ir.sh now touch their sources before building. Caught
live: three identical A/B results in a row, and a run that read the
pre-A1 Ir with A1 already in the tree. Cargo's fingerprint across the
/mnt drvfs mount does not reliably notice an edit.
N6 the client_path harness tax, never computed before: 3,661,681 Ir (1.67%).
Corrections: A8 is the harness, not the product, and its figure was half the
true 1,718,250 (32,000 calls, 2 per step) -- there is no ln or log call
anywhere in the product source. B18's "already at its floor" was reached on an
instrument that inlines the codec away; N8 has now made it a measurement
instead of an assumption.
Also: N5 deletes a parallel Vec of timestamps the density branch built to index
what window[i].t answers directly. That path had NO test, so a rewrite of its
inner loop passed 217 tests without executing a line of it; a test now pins the
half-gap rule and enters the branch.
Gates: both checksums byte-identical (0xc0452ab605328800, f456d8bf8ff664d5),
codec gate 0x99f61c51fcc31496, work parity exact on all three instruments, 218
tests, clippy -D warnings, fmt, no_std leaf green on thumbv7em-none-eabihf and
riscv32imac-unknown-none-elf.
Committed with an explicit pathspec: a concurrent session had release work
staged in the index, which this deliberately does not touch.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The README carried 237 Ir/request and 13,613 Ir/step; both predate rounds 1-6. Measured now: 224.3 and 12,137.5, with the ledger row that produced them, per the repo's own rule that no README performance claim exists without one. The ledger row records the two wins (N3 -808,139 Ir on the server, N2a -317,822 on the client), the five refutations with their numbers, the third instrument and its 63%-tap correction, the stale-binary trap on the /mnt mount, the client-harness tax nobody had computed, the two corrections to rounds 1-5 (A8 is the harness; B18 was reached on an instrument that inlines the codec away), and the coverage hole where a knob-gated branch had no test at all. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two instruction-count campaigns land together. The version is 0.3.0 rather than
the 0.2.1 I had staged, because round 6 changed three PUBLIC field types:
pub last_receive: Option<NtpTimestamp> -> NtpTimestamp
pub last_transmit: Option<NtpTimestamp> -> NtpTimestamp
pub last_receive_sent: Option<NtpTimestamp> -> NtpTimestamp
Any downstream `if let Some(ts) = rec.last_receive` breaks, and under Cargo's
0.x rules a breaking change needs the minor bump.
**cargo-semver-checks reports "no semver update required" for this.** It has no
lint for a public field's TYPE changing, so it would have let a breaking change
ship as a patch. The bump was decided by reading the diff. tools/validate-
release.sh now carries a public-API-surface check that greps the diff for
changed `pub` lines and warns independently of the tool; on this release it
flags all six lines the tool missed.
Also fixed, caught by CI and missed by my own validator: the new client index
asserted `n <= (u32::MAX as usize) + 1`, which overflows at COMPILE TIME on a
32-bit usize -- wasm32 is a shipping target and `arithmetic_overflow` is
deny-by-default. Phrased against the mask (`n - 1 <= u32::MAX as usize`) it is
trivially true on 32-bit and a real bound on 64-bit.
The validator missed it because it gated the target matrix with `cargo check`,
and `check` skips codegen while the const-eval lints fire during const-prop.
Measured: check exits 0, build exits 101 on the same source. Every target in
the validator is now BUILT, not checked.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Ttimmahlax
changed the title
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
2 participants
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.
What
Two instruction-count campaigns, released together as 0.3.0. Measured with callgrind Ir — exact, so the same binary re-runs to zero.
client_pathhot_path(steady)hot_path(churn)mru_report(Rounds 1–5 + B15 from this branch; round 6's N3/N7 on top.)
cargo-semver-checksdoes not see whyRound 6 changed three public field types:
NtpTimestamp::ZEROmeans unset, which is how RFC 5905 already spells it on the wire.Option<NtpTimestamp>was 16 bytes for 8 bytes of payload (NtpTimestampis a bareu64, no niche) and every read tested a discriminant.Migration:
if let Some(ts) = rec.last_receive→if !rec.last_receive.is_zero().cargo semver-checksreports "no semver update required" for this. It has no lint for a public field's type changing, so it would have let this ship as a patch under Cargo's 0.x rules. The bump was decided by reading the diff.tools/validate-release.shnow greps the diff for changedpublines independently of the tool, and flags all six.No behaviour changed — the corpus proves it
Also fixed: a bug CI caught that my validator missed
The new client index asserted
n <= (u32::MAX as usize) + 1, which overflows at compile time on a 32-bitusize—wasm32is a shipping target andarithmetic_overflowis deny-by-default. Phrased against the mask (n - 1 <= u32::MAX as usize) it's trivially true on 32-bit and a real bound on 64-bit.The validator missed it because it gated the target matrix with
cargo check, andcheckskips codegen while the const-eval lints fire during const-prop. Measured on the same source:checkexits 0,buildexits 101. Every target in the validator is now built, not checked.Release gate
tools/validate-release.sh— 17 checks over a clean worktree of the exact commit. 17 passed, 0 failed, 1 warned (the warn being the breaking-change flag above, handled by the bump).Not included
B16 (−1,016,192) stays unshipped: it weakens the client hasher's avalanche, which is a security trade rather than a perf one. Eleven refuted probes are documented with numbers in
docs/plans/instruction-veins.md; the clever-local-rewrite class went 0-for-8 and every win was a deletion.🤖 Generated with Claude Code