Replace ScoredCandidate.signal_snapshot Vec<(String,f64)> with
SmallVec<[(SignalKey,f64); 4]> where SignalKey is Static(&'static str)
| Owned(Arc<str>):
- Compile-time-constant labels (sort bases, relevance, co_engagement,
preference_affinity) -> Static: zero allocation, pointer-copy clone.
- Dynamic {signal}_boost/_penalty/_decay labels built once per query in a
RuleLabels hoist (was format! per-candidate-per-rule), shared by Arc.
Cohort rescore hoisted the same way.
- scored accumulator pre-sized to candidates.len().
- Owned Strings rebuilt only at the two response-assembly sites (<= limit).
Byte-identical output: full 1896-test lib suite green. Measured win
(cargo bench --bench ranking, committed-base vs working-tree):
score_200_hot 23.89->21.04us (-11.9%), score_200_trending
27.66->25.85us (-6.5%), score_200_full_pipeline 27.88->26.97us (-3.3%).
smallvec promoted from the lock to a direct dep (no new dependency
surface). perf-sweep doc updated; T2 (per-term DashMap collapse) next.
184 KiB
Performance Sweep — tidalDB serving hot path (2026-06-13)
Lens: mechanical sympathy (@tidal-performance / Martin Thompson). 14 parallel auditors + dedup synthesis. Overall score: 71/100.
Summary
Two structural themes dominate. (1) A signal_snapshot String cascade: ranking builds Vec<(String,f64)> per candidate (format!/to_string), diversity deep-clones it twice, result-assembly reads only the ~limit page — one root cause filed under ranking, diversity, and query/fusion, ~80-90% of those Strings die unread. (2) Thread-spawn-per-query funnels: scatter-gather and the cluster read path are the SAME spawn-per-shard-per-query code, gated on one global Mutex+Condvar semaphore, when a reused pool already exists adjacent (ClusterWritePool). Secondary: per-candidate redundant DashMap lookups in scoring, whole-bitmap and whole-user-bitmap clones for membership tests, zero fjall tuning, prost bytes=vec copies, and a measurement-honesty gap where criterion MEANS are reported under p99 SLO labels. The hot paths are well-built; the debt is materialization the consumer never reads.
Category scores
| Score | Category |
|---|---|
| 52 | Scatter-gather fan-out & threading model |
| 62 | Ranking score-loop allocation |
| 62 | Diversity/MMR selection allocation |
| 62 | Storage engine mechanical sympathy (fjall) |
| 68 | Latency measurement honesty & bench fidelity |
| 72 | Query retrieve / fusion / candidate-gen materialization |
| 72 | Bitmap / range / filter index intersection |
| 72 | Cluster node per-request path & shard-map locking |
| 74 | WAL write path & group commit |
| 78 | Vector distance & ANN inner loops |
| 78 | Signal hot/warm read path & cache-line layout |
| 78 | Text / BM25 search path (tidal/src/text/**) |
| 78 | Replication shipping funnel & lock hold |
| 78 | Network transport serialization & framing (tidal-net) |
Ranked findings (ROI-ordered, deduped)
| # | Sev | Conf | Category | Finding | Location | Est. gain |
|---|---|---|---|---|---|---|
| 1 | HIGH | high | Ranking + Diversity + Query (deduped) | signal_snapshot String cascade: built per-candidate, deep-cloned twice in diversity, read only for the returned page | tidal/src/ranking/executor/mod.rs:570/602/623/650/681 (build) -> diversity/selector.rs:118/121/140/143/169/250 (2x clone) -> query/executor/pipeline.rs:504-516 + search/pipeline.rs:772-787 (consume page only) |
Eliminates ~80-90% of per-query String allocations on the scoring path plus one-to-two full-result deep clones per constrained query; the single largest allocation win in the serving pipeline |
| 2 | HIGH | high | Scatter-gather + Cluster node (deduped) | OS-thread-per-shard spawned per /sharded read query, gated on one global Mutex+Condvar semaphore | tidal-server/src/scatter_gather.rs:514 (spawn) + :175-233 (global semaphore), invoked per-query from cluster/routes.rs:736/845 and node.rs:5141 |
Removes N thread create/teardown syscalls + N 2MB stack reservations per query (tens of µs/shard) and one cross-core lock-bounce point under storm; certain mechanical win, magnitude workload-dependent |
| 3 | HIGH | high | Signal hot/warm read path | Per-candidate scoring does one DashMap lookup PER gate/exclude/boost term instead of one per (candidate, signal_type) | tidal/src/ranking/executor/helpers.rs:62-111/169/212 + mod.rs:85-89 -> signals/ledger/core.rs:337/388 |
Collapses (E+G+B) shard-lock acquisitions + hash+chase per candidate to ~1-2; on the hottest ~45% query stage, ~800-1200 redundant shard-locks removed per 200-candidate query |
| 4 | HIGH | high | Bitmap/filter index | BitmapIndex::get() clones the entire value-bitmap per leaf per query for a few-hundred membership tests | tidal/src/storage/indexes/bitmap.rs:188-194, consumed at query/executor/pipeline.rs:303-305 |
Eliminates one full-bitmap heap alloc+memcpy per filter leaf (hundreds of KB for 50%-selectivity at 1M); large at scale, invisible at the current 10K bench |
| 5 | HIGH | high | Query candidate materialization | User-context suppression clones up to 4 whole per-user RoaringBitmaps by value per personalized query | tidal/src/query/executor/mod.rs:563-636 -> entities/user_state.rs:144/191/379/399 |
Removes 2-4 O(seen-set-size) heap clones per FOR USER query; dominant Stage 2.5 allocation for power users with large history |
| 6 | HIGH | medium | Storage engine (fjall) | fjall opened with zero tuning — default block cache, no bloom-filter policy, default compaction | tidal/src/storage/fjall.rs:289/315 |
Point-get latency can drop from several SSD reads to one cache hit at 1M+ cold; ~200 point gets/query governed by this config; magnitude needs cold-cache scale bench |
| 7 | HIGH | high | Latency measurement honesty | scale-baselines.md reports criterion MEAN estimates under a p99 SLO label — the tail SLO is never measured as a tail | docs/profiling/scale-baselines.md:29-31 <- tidal/benches/scale.rs:156-158/272-283 |
No runtime gain; prevents signing off a tail SLO with a central-tendency number that hides 10-100x p99 inflation under contention |
| 8 | HIGH | high | WAL write path | No criterion bench measures the real WAL append/group-commit/fsync path — the whole category is optimized blind | tidal/benches/signals.rs:29 (NoopWalWriter) + recovery.rs (read-side only) |
No direct gain; it is the instrument that makes every other WAL finding (dedup double-hash, encode double-copy, per-flush allocs) provable and guards committed-events/s against silent regression |
Dedup notes
-
ROOT CAUSE #1 — signal_snapshot String materialization — filed under THREE categories merged into rank-1: (a) Ranking 'signal_snapshot Strings allocated for EVERY candidate' + 'format!(_boost/_penalty/_decay) per candidate' (executor/mod.rs:602/650/681); (b) Diversity 'surviving candidates deep-cloned twice' + 'relaxation stages deep-clone remainder' (selector.rs) — these clones are expensive ONLY because the snapshot Vec<(String,f64)> they copy is non-empty; (c) Query/Search 'per-result-item String allocations in assembly loops' (pipeline.rs:507-514/772-779). All three are the same field. Fixing the snapshot at its source (defer/intern, carry static-str ids) shrinks the diversity clone cost AND the assembly cost. Sequence: fix ranking source first, then diversity clone-elimination becomes cheap, then assembly reads the page.
-
ROOT CAUSE #2 — thread-spawn-per-shard-per-query — filed under TWO categories (Scatter-gather fan-out HIGH; Cluster node per-request HIGH) describing the IDENTICAL code at scatter_gather.rs:514. Merged into rank-2. The cluster-node report and the scatter-gather report cite the same spawn loop, same format! thread-name alloc, same global semaphore. One pool-conversion task closes both. The cluster-node report's MEDIUM routing-state-hoist (node.rs:5988 sharded_region_ids/entity_shard rebuild) and per-query map clones (routes.rs:736) are SEPARATE, lower-ROI sub-findings of the same file — sequenced to a later wave.
-
CROSS-CUTTING #3 — 'select_nth_unstable when limit<<n' partial-sort pattern appears as a deferred finding in FOUR categories (ranking finalize, fusion RRF, query candidate-gen already-done, vector top-K already-done). The ranking-finalize and fusion ones are BOTH gated on threading an effective working-set budget through a multi-stage pipeline; neither is a local edit. Recorded as one cross-cutting note, NOT duplicated as separate high tasks — both are LOW/MEDIUM and premature relative to the allocation wins.
-
CROSS-CUTTING #4 — global-Mutex-guarding-immutable-state pattern recurs: text_tx Mutex<Option> (text/items.rs:526), scatter semaphore Mutex (scatter_gather.rs:175), commit/peer_applied Mutex (ship.rs:971), id_to_name_map().clone() (routes.rs:736). All are 'a lock or deep-clone guarding data that is immutable after open'. The fix family is identical (ArcSwap / Arc / atomic mirror). Grouped by file into separate waves but flagged as one anti-pattern for the enforcement lint.
-
SEVERITY DOWNGRADES at aggregate level: WAL 'per-RPC payload clone' (ship.rs:1056) and 'feed.collect un-sized Vec' (feed.rs:273) are per-RPC/per-batch off the query path → kept MEDIUM not HIGH. Transport prost bytes=vec is per-segment off the µs serving path → correctly MEDIUM. Histogram 13-RMW observe() is metrics-feature-only → MEDIUM. The seed's '~60 lock-refs on recording path' claim was REFUTED by the auditor (path is lock-free) — not carried. The seed's diversity 'format! in hot loop' (selector.rs:274/295) was REFUTED (cold violation path) — not carried.
Missing categories (lens implies, no auditor owned)
-
Embedding/preference-vector decode on the personalization path — personalization.rs:109 reads a preference vector per scored candidate via storage.get (the value-copy + decode), counted under storage but never profiled as its own per-candidate f32-decode cost; no auditor measured whether vector deserialization competes with l2_distance for the brute path's time
-
Cohort rescore + exploration injection allocation (query/executor/mod.rs:474/480) — the cohort report covers the ledger DashMap but no auditor profiled the rescore reorder + exploration APPEND that runs AFTER finalize and reorders the whole candidate set per personalized query; it is the stage that blocks the finalize partial-sort fix
-
Serialization/deserialization of metadata HashMap<String,String> on the read path (bincode/serde decode after storage.get) — every metadata point-get is followed by a deserialize that allocates Strings; storage auditor flagged the copy-out but not the decode-side allocation of the HashMap itself per candidate
-
Tokio blocking-pool sizing and spawn_blocking saturation under scatter storm — the scatter finding notes 'a blocking thread spawns N more blocking threads' but no auditor measured blocking-pool queue depth / starvation, which is the real tail driver when fan-out and HTTP fetch both contend the same pool
-
Allocator selection under contention (jemalloc/mimalloc vs system) — multiple findings cite 'allocator-lock contention under concurrent ingest' as the mechanism, but no auditor benchmarked the global allocator choice, which is the cheapest lever for every per-candidate/per-write alloc finding combined
-
End-to-end RETRIEVE p99 attribution — every category claims its micro-win 'needs the query/scale bench to confirm it moves end-to-end p99' but NO auditor owns a differential flamegraph of the full RETRIEVE pipeline that apportions the 45%-scoring/storage/diversity/filter split; the stage budget is asserted, never measured at 1M/10M
Wave manifest
Wave 1: Instrument before optimizing — build the missing measurement floor so every later wave has a real before/after (no WAL/ship/scatter bench exists; the p99 SLO is reported as a mean)
-
[low risk] Add tidal/benches/wal.rs: drive WalHandle::append_record_staged + wait on a real-disk dir, parameterized over batch_size {1,10,100} and batch_timeout; report committed-events/s and p50/p99/p999 append latency via iter_custom capture (open-loop intended-send timing). Add pure-CPU wal_encode_batch + wal_encode_embedding micro-benches with no I/O.
- files:
tidal/benches/wal.rs (new), tidal/Cargo.toml (bench entry) - verifies: WAL 'no bench measures the real append/group-commit/fsync path' (rank 8); backs every WAL finding's est_gain
- bench:
tidal/benches/wal.rs::wal_append_throughput (the bench itself)
- files:
-
[low risk] Add tidal/benches/scatter.rs against a SimulatedCluster: drive scatter_gather_retrieve over 4- and 16-shard topologies with a no-op query_one, report per-query wall time + allocation count, parameterized to launch K concurrent queries so the global semaphore is contended.
- files:
tidal/benches/scatter.rs (new), tidal/Cargo.toml - verifies: Scatter-gather thread-per-shard spawn (rank 2) + global semaphore contention; provides the before/after for wave 2
- bench:
tidal/benches/scatter.rs::scatter_fanout (the bench itself)
- files:
-
[low risk] Relabel docs/profiling/scale-baselines.md: stop calling criterion mean-estimates 'p99'; mark them 'isolated per-op cost (mean)'. Add scale.rs iter_custom percentile capture for signal-write and retrieve so a real single-thread p99/p999/max distribution exists, and cite a tidal-stress open-loop run as the authoritative tail.
- files:
docs/profiling/scale-baselines.md, tidal/benches/scale.rs - verifies: Latency-honesty 'mean reported under p99 label' (rank 7) + 'benches are closed-loop mean only'
- bench:
tidal/benches/scale.rs::retrieve_1m + tidal-stress peach-100k ramp
- files:
Wave 2: Source-level allocation kill — fix the signal_snapshot at its origin in ranking and the per-candidate DashMap lookup; these are different files from each other and from wave 1
-
[medium risk] Defer signal_snapshot materialization: replace per-candidate Vec<(String,f64)> built via format!/to_string with a SmallVec<[(&'static str | SignalTypeId, f64); 4]> carrying interned/static signal names + precomputed rule labels (compute boost/penalty/decay label String ONCE per rule before the candidate loop). Build the (String,f64) page rows only in the consumer. Pre-size scored Vec with_capacity(candidates.len()).
- files:
tidal/src/ranking/executor/mod.rs, tidal/src/ranking/executor/scoring.rs - verifies: Ranking signal_snapshot + format!-per-candidate + Vec::new()-no-capacity (rank 1 source)
- bench:
tidal/benches/ranking.rs::score_200_full_pipeline + a new score_200_limit_20 variant (snapshot-eager vs deferred)
- files:
-
[medium risk] Collapse per-term DashMap lookups to one entry.get() per (candidate, signal_type): resolve the profile's distinct signal type ids once at query start, group each candidate's excludes/gates/boosts by type id, take a single entries.get() per distinct type id, evaluate all terms against the held ref before releasing the shard guard.
- files:
tidal/src/ranking/executor/helpers.rs, tidal/src/signals/ledger/core.rs - verifies: Signal hot path 'one lookup per term instead of per (candidate,type)' (rank 3)
- bench:
tidal/benches/signals.rs::bench_200_entity_scoring_pass (add a read_agg/passes_gates variant) + ranking.rs end-to-end RETRIEVE p99
- files:
Wave 3: Consumer-side de-clone — once the snapshot is cheap, eliminate the diversity double-clone and the bitmap/user-state clones; distinct files from wave 2
-
[medium risk] Stop greedy_select cloning ScoredCandidate: change its accumulator to Vec / insert-into-accepted-set; pass the full candidates slice + accepted set by reference into the relaxation stages (skip accepted via O(1) contains, no .cloned().collect() of remainder); clone survivors exactly once at the final emit pass into Vec::with_capacity(max_needed). Key format_counts by &str. Pre-size accepted/creator_counts HashMaps.
- files:
tidal/src/ranking/diversity/selector.rs - verifies: Diversity 'deep-cloned twice' + 'relaxation clones remainder' + 'format_counts String key' + 'default-capacity maps' (rank 1 consumer)
- bench:
tidal/benches/diversity.rs::diversity_200_worst_case_relaxation + a new non-empty-snapshot variant
- files:
-
[low risk] Add borrowing accessors with_seen_bitmap/with_hidden_items/with_saved_bitmap/with_liked_bitmap returning dashmap::Ref (mirror CreatorItemsBitmap::get_ref); have suppression hold the guard across the retain instead of cloning the per-user RoaringBitmap.
- files:
tidal/src/entities/user_state.rs, tidal/src/query/executor/mod.rs - verifies: Query 'user-context suppression clones 4 whole bitmaps' (rank 5)
- bench:
tidal/benches/scale.rs::for_you + a new query.rs FOR USER variant seeding a 50k-entry seen/saved bitmap
- files:
-
[medium risk] Add BitmapIndex::with_value(value, |Option<&RoaringBitmap>|) borrowing accessor; route the executor's retain(contains) consumption to test candidates against the live bitmap under the guard instead of cloning the leaf. In evaluate_and, treat deferred/user-state children as identity (skip, do not synthesize universe.clone()) and materialize index leaves cardinality-first so an empty leaf short-circuits before big siblings.
- files:
tidal/src/storage/indexes/bitmap.rs, tidal/src/storage/indexes/filter/evaluator.rs - verifies: Bitmap 'get() clones whole value-bitmap' (rank 4) + 'evaluate_and materializes every child incl universe.clone()'
- bench:
tidal/benches/filters.rs — add 1M-universe filter_bitmap_and_2 + an AND-with-deferred-child variant
- files:
Wave 4: Storage and transport copy elimination — fjall tuning + borrowing point-read + prost bytes; independent subsystems, none overlap waves 2-3
-
[medium risk] Build one shared fjall BlockCache (deploy-configurable via StorageConfig threaded through open()), set explicit bloom bits-per-key (~10) on KeyspaceCreateOptions, and set leveled compaction explicitly. Add a get_with(key, |Option<&[u8]>|) borrowing accessor to the StorageEngine trait so metadata/preference decode reads fjall's slice in place (skip to_vec()); route hot read callers through it.
- files:
tidal/src/storage/fjall.rs, tidal/src/storage/engine.rs - verifies: Storage 'fjall opened with zero tuning' (rank 6) + 'get forces Vec copy'
- bench:
tidal/benches/storage.rs::bench_random_get — add a cold-cache 1M-row variant + a get_with variant; confirm via query.rs/scale.rs
- files:
-
[low risk] Pre-sort candidate EntityIds ascending before the metadata point-get loop (BE-key layout makes order==id, turning ~200 random LSM descents into a near-sequential cache-resident scan); reuse one scratch key buffer via encode_key_into to kill the per-candidate key Vec alloc.
- files:
tidal/src/query/executor/mod.rs, tidal/src/storage/indexes/keys.rs - verifies: Storage 'per-candidate metadata get not sorted, re-encodes key each iter'
- bench:
tidal/benches/storage.rs (shuffled vs sorted key-order microbench) + query.rs RETRIEVE end-to-end
- files:
-
[medium risk] Add .bytes(&[.ShipSegmentRequest.payload, .SnapshotFileChunk.data]) to the tonic_build config; change WalSegmentPayload.bytes from Vec to bytes::Bytes and thread Bytes through convert.rs + the server ship_segment handler so inbound decode is a zero-copy refcount slice instead of an alloc+memcpy. Coordinate the type change with @tidal-engineer (replication apply path).
- files:
tidal-net/build.rs, tidal-net/src/convert.rs, tidal/src/replication/transport.rs - verifies: Transport 'prost bytes=vec forces alloc+memcpy per inbound segment'
- bench:
tidal-net/benches/transport_throughput.rs — extend to sweep SEGMENT_SIZE {1KB,64KB,1MiB,16MiB}
- files:
Wave 5: Threading-model and lock de-funnel — convert spawn-per-query to a pool and immutable-state-behind-lock to Arc/atomic; touches scatter/cluster/text files untouched by earlier waves
-
[high risk] Replace the per-shard std:🧵:Builder::spawn with submission to a pre-sized reused worker pool (the ClusterWritePool shape) sized to MAX_SHARD_WORKERS; fold the bound into the pool's bounded queue and replace the global Mutex+Condvar semaphore with that queue (or an AtomicUsize CAS-acquire with AcqRel on success). Drop the per-shard format! thread-name. Preserve detached-deadline recv_timeout semantics.
- files:
tidal-server/src/scatter_gather.rs - verifies: Scatter+cluster 'OS-thread-per-shard per query' + 'global Mutex+Condvar semaphore' + 'per-shard format! name' (rank 2)
- bench:
tidal/benches/scatter.rs (wave 1) + tidal-stress open-loop /sharded/feed p50/p99/p999 at S=4 and S=16
- files:
-
[medium risk] Hoist immutable cluster routing state: precompute the sorted region-id Vec + ShardRouter ONCE at node construction (route via cached self.router); wrap id_to_name and peer_http in Arc and Arc::clone into the worker closure instead of deep-cloning per query; read leader once at the top of the write handler (or store as AtomicU32 with Acquire) instead of 3 RwLock reads per write.
- files:
tidal-server/src/cluster/node.rs, tidal-server/src/cluster/routes.rs - verifies: Cluster 'rebuilds ShardRouter + region Vec per write', 'clones region/peer maps per query', 'redundant leader RwLock reads'
- bench:
tidal-stress write+read ramp at S>1 (signals/s + /sharded/feed p99); criterion sharded_route/per_write micro
- files:
-
[medium risk] Replace text_tx Mutex<Option> with ArcSwapOption (immutable post-open, lock-free load); project the metadata HashMap down to declared text-field keys before building PendingWrite (or carry Arc) to drop the per-write deep clone; precompute default_fields Vec / cache the QueryParser at index open; return Cow from preprocess_query to skip the no-op String alloc when no hashtag.
- files:
tidal/src/db/items.rs, tidal/src/text/query.rs - verifies: Text 'per-write Mutex lock', 'metadata deep clone per write', 'per-query QueryParser Vec alloc', 'preprocess_query String alloc'
- bench:
tidal/benches/search.rs::search_text_10k + a new multi-threaded ingest bench through text_tx
- files:
Wave 6: WAL/ship CPU + replication copy trims — apply the now-measurable per-fsync/per-RPC fixes the wave-1 benches prove; distinct files from all earlier waves
-
[medium risk] Hash each event's BLAKE3 once in partition_dedup and thread the precomputed u128 into dedup.record (add DedupWindow::record_hash) so committed events are not hashed twice. Hoist kept_events/kept_replies/batch_seen to writer-thread-local scratch cleared (not reallocated) per flush. In encode_frame, reserve HEADER_SIZE at the front of the payload buffer and write the header in place to eliminate the second full-payload alloc+memcpy+zero-init.
- files:
tidal/src/wal/writer.rs, tidal/src/wal/format/batch.rs, tidal/src/wal/dedup.rs - verifies: WAL 'double BLAKE3', 'per-flush HashSet/Vec allocs', 'encode double-copy' (rank 8 dependents)
- bench:
tidal/benches/wal.rs::wal_append_throughput (batch=1 and 100) + wal_encode_batch/wal_encode_embedding
- files:
-
[medium risk] Move run.bytes by value into range_payload_with_term (drop the per-RPC clone; destructure ClaimedRun into a Copy {first,last,event_count} header + owned bytes for the success/failure branches). In feed.collect, pre-sum contributing batch lengths and Vec::with_capacity; collect Arc<Vec> handles under the tail lock then concatenate outside it to shrink the writer-contending lock hold.
- files:
tidal/src/replication/ship.rs, tidal/src/wal/feed.rs - verifies: Replication 'per-RPC payload clone', 'feed.collect un-sized Vec + long lock hold'
- bench:
tidal/benches/ship.rs (new): push N batches, time collect over run sizes + ns/run send; tidal-stress replicated-writes/s + p99 lag
- files:
-
[low risk] Switch the metrics histogram to a non-cumulative (delta) bucket layout: observe() increments exactly one bucket + count + sum (3 RMWs not up to 13); render_prometheus computes the cumulative prefix-sum at scrape time. Keep Relaxed ordering. Run signals bench WITH --features metrics so the instrumentation tax becomes visible.
- files:
tidal/src/db/metrics/histogram.rs - verifies: Latency-honesty 'observe() up to 13 RMWs on 82ns path' + 'per-op timer self-inflation unmeasured'
- bench:
tidal/benches/signals.rs::bench_single_signal_write run with and without --features metrics
- files:
Wave 7: Enforcement — lock in every structural win with a lint/bench-gate/doc note per category that produced a finding so it cannot regress
-
[low risk] Add a criterion regression-gate (CI-equivalent bench-compare via cargo-criterion baseline, no GitHub Actions) on ranking.rs/diversity.rs/signals.rs/filters.rs/storage.rs/wal.rs/scatter.rs: fail if mean regresses >5% vs the committed baseline. Add an allocation-count assertion (dhat or counting allocator) on score_200 and diversity_200 so the snapshot/clone wins cannot silently revert.
- files:
tidal/benches/* baselines, scripts/bench-gate.sh - verifies: Locks rank 1,3,4 (allocation), rank 8 (WAL throughput), rank 2 (scatter)
- bench:
all wave-1..6 benches as the gate corpus
- files:
-
[low risk] Add a clippy/lint + CODING_GUIDELINES §8 note for the two recurring anti-patterns: (a) 'no per-candidate/per-item heap alloc (format!/String/Vec::new/clone) on the scoring/filter/diversity inner loop — build invariants once, borrow inside, materialize only the returned page'; (b) 'immutable-after-open state must not sit behind a Mutex or be deep-cloned per request — use ArcSwap/Arc/atomic'. Document the borrowing-accessor pattern (get_ref/with_value/get_with) as the canonical point-read.
- files:
CODING_GUIDELINES.md, .clippy.toml or a custom lint note - verifies: Structural lock-in for ranking/diversity/query/bitmap/storage/text/cluster categories (the immutable-behind-lock + per-item-alloc families)
- bench:
n/a (doc + lint enforcement)
- files:
-
[low risk] Document the measurement contract in docs/profiling/: criterion outputs are 'isolated per-op cost (mean)' regression tripwires ONLY; every p99/p999/tail SLO claim must cite an open-loop coordinated-omission-corrected tidal-stress run. Pin the fjall checkpoint-must-not-be-per-write-fsync invariant and the shipper.rs-is-dead-path note so neither funnel silently reactivates.
- files:
docs/profiling/scale-baselines.md, docs/profiling/hotspot-analysis.md, CODING_GUIDELINES.md §8 - verifies: Latency-honesty (rank 7) + WAL fsync guardrail + replication legacy-shipper guardrail
- bench:
tidal-stress as the cited authoritative tail source
- files:
Full per-category findings
Vector distance & ANN inner loops — 78/100
Canonical fast pattern: The top-K reduction is already exemplary: select_top_k (brute/mod.rs:69-82) uses select_nth_unstable_by(k-1) for an O(n) partition, truncates to k, then sorts only the retained k — exactly the "partial sort when limit << candidates" pattern, with cmp_distance #[inline] and a documented NaN-safe weak ordering. That is the fast pattern this category should be measured against. The deviations are concentrated in (a) the scalar distance kernel with no vectorization affordance, and (b) the full-result Vec materialized per query before reduction. Crucially, the brute path is a real production query path for any embedding slot under USEARCH_MIN_VECTORS=10,000 live vectors (registry.rs:35), reached every RETRIEVE/SEARCH via pipeline.rs:431/437 — it is not test-only.
Deviations: n/a
-
[MEDIUM/medium] l2_distance_sq is a scalar reduction with no vectorization affordance and no #[inline] across the per-candidate call boundary
- @
tidal/src/storage/vector/brute/mod.rs:37-46— blast: Medium — per-candidate, inside the brute search/filtered_search loop (brute/mod.rs:154,183). Executes n times per query for any slot under 10,000 live vectors, which is a real production path (pipeline.rs:431/437 over Box). Not on the USearch path (that delegates distance to C++ SIMD). - evidence: l2_distance_sq is
a.iter().zip(b.iter()).map(|(x,y)| {let d=x-y; d*d}).sum()— a plain scalar fold. It carries no#[inline](contrast cmp_distance at :55 which IS#[inline]), nochunks_exact(8)to expose independent lanes, and nomul_add/FMA. There are zero SIMD crate deps and zerochunks_exact/std::simduses anywhere in tidal/src/storage/vector/ (verified by grep). For an L2-normalized f32 fold the iterator chain auto-vectorizes poorly because f32 addition is non-associative, so LLVM keeps a single serial accumulator (one add per element, no-ffast-math) — the dominant cost is the dependency chain on the accumulator, not the multiply. This runs PER CANDIDATE: in brutesearchit executes once for every stored vector (n times), and a 1536-D production slot is 1536 scalar sub/mul/add per candidate. - fix: Restructure the reduction to break the single-accumulator dependency chain: iterate
a.chunks_exact(8).zip(b.chunks_exact(8))accumulating 8 partial squared-distance lanes in parallel, summing the remainder tail separately, then horizontally reduce — this lets LLVM emit packed SSE/AVX without changing numerics meaningfully (still f32, no fast-math). Add#[inline]so it folds into the per-candidate closure and the bounds checks hoist. Confirm with a flamegraph that the brute scan is actually hot at the target slot size before investing; a portable_simd orwidef32x8 kernel is the bigger win but needs a property test bounding distance error vs the scalar reference (per the no-approximation-without-error-bound rule). Leave the USearch path alone — its SIMD is in C++. - bench: tidal/benches/vector.rs::bench_ann_search_unfiltered (ann_search_unfiltered_10k, 128D) and bench_ann_search_brute_force already exercise this exact kernel at 10K/128D; add a direct criterion micro-bench of l2_distance_sq at 1536D (the production dim) to isolate the kernel from the Vec/lock cost, and a recall-delta check is not needed since chunked f32 stays exact-enough (property test the abs error < 1e-4). — est: Plausibly 2-4x on the distance kernel at 1536D with an 8-lane chunked fold; but the end-to-end query win depends entirely on whether the brute scan dominates query time at the served slot size — needs a flamegraph to confirm it moves RETRIEVE p99 before claiming it. Low-to-medium confidence on end-to-end impact.
- @
-
[MEDIUM/medium] brute search materializes a full n-element Vec per query before top-K reduction
- @
tidal/src/storage/vector/brute/mod.rs:150-159 (and 178-188 for filtered)— blast: Low-to-Medium — one alloc per query (not per candidate), on the brute path only. At n=10K that is ~160KB allocated and freed every query; the alloc itself is cheap relative to the scan, but it also evicts the result-set working set from L2 between queries under load. - evidence:
searchdoesguard.iter().map(|(id,vec)| VectorSearchResult{...}).collect()building a Vec sized to the ENTIRE index (n entries, each 16 bytes: u64 id + f32 distance + padding), then hands it to select_top_k which throws away all but k. That is one heap allocation of n16 bytes per query whose lifetime is the whole scan, even though k (=limitmultiplier, typically ≤ a few hundred) is tiny relative to n. The collect is unavoidable for select_nth_unstable (it needs a slice), but the alloc could be reused across queries instead of fresh per call. filtered_search has the same shape but only allocates the post-filter survivors. - fix: This is a smaller win than the kernel. The clean mechanical-sympathy fix is a bounded top-K via a fixed-capacity binary max-heap of size k threaded through the scan: push-or-replace as you score, so you allocate O(k) not O(n) and never materialize the full result set. That also removes the separate select_nth pass. Only do this if a profile shows the n-sized Vec alloc/scan-locality is material — otherwise it is premature; select_top_k's existing partial-sort is already good once the Vec exists. Do NOT touch the USearch result marshalling (usearch_index.rs:210-215) — k there is already small (USearch returns only k matches).
- bench: tidal/benches/vector.rs::bench_ann_search_unfiltered_10k and bench_ann_search_filtered_*; compare alloc count via a heaptrack/dhat run or criterion wall-clock delta. A heap-based top-K would also need the recall to stay identical (it is exact — same comparator), so only a latency delta is required, no recall delta. — est: Modest — reduces per-query allocation from O(n) to O(k) and improves cache residency of the result set, but the dominant cost is the n distance computations, not the one alloc. Needs measurement; likely single-digit percent on the brute path.
- @
-
[LOW/medium] USEARCH_MIN_VECTORS=10,000 threshold means the O(n) brute scan under an RwLock read is the production query path for every sub-10K slot
- @
tidal/src/storage/vector/registry.rs:35 (threshold) + brute/mod.rs:145-159 (RwLock read held across the whole scan)— blast: Low frequency in absolute terms (only sub-10K slots) but Medium impact when hit: per-query O(n) under a read lock, serialized against writes. The lock is read-shared so it is contention on the RwLock word + write-starvation, not query-vs-query serialization. - evidence: build_slot_index (registry.rs:50-89) routes any slot with count < 10,000 to BruteForceIndex. brute search() takes
self.vectors.read()(brute/mod.rs:145-148) and holds that RwLock read guard across the ENTIRE iterate+score+collect (guard dropped at :158, after the collect at :150-156). The guard is a shared read lock so concurrent queries don't block each other, but every concurrent insert (write lock, brute/mod.rs:127) blocks behind all in-flight scans and vice versa. For a 9,999-vector slot at 1536D this is ~15M scalar ops per query under a held lock — well past the <10ms p99 ANN SLO if QPS is high, and the doc comment at registry.rs:28-34 explicitly acknowledges brute 'blows past the latency budget at scale' yet 10K is a large brute scan. - fix: This is a tuning/threshold finding, not a code bug — the lock discipline is correct (read-shared, write-exclusive, guard dropped before select_top_k). The mechanical-sympathy concern is that 10K * 1536D scalar under a lock is a lot of brute work to call 'still fast'. Re-measure the actual brute crossover at PRODUCTION dim (1536D, not the bench's 128D) and consider lowering USEARCH_MIN_VECTORS, or only keep it high at small dims. Independently: if the kernel finding above lands (chunked SIMD), the brute scan gets cheaper and the threshold can stay. No memory-ordering change needed — the RwLock is std, not an atomic hot path.
- bench: tidal/benches/vector.rs::bench_ann_search_brute_force_10k is at 128D and understates the production cost; add a 1536D brute search bench at n=9,999 to measure the actual worst-case sub-threshold query latency against the <10ms p99 SLO. Use scale.rs for end-to-end RETRIEVE p99 with a sub-10K slot. — est: No direct speedup — this is a measurement/threshold validation. The win is catching a slot that sits at 9,999 vectors at 1536D blowing the ANN p99 budget; needs the 1536D brute bench to quantify the crossover honestly rather than trusting the 128D number.
- @
-
[LOW/high] l2_normalize runs three scalar passes over the vector per write (two real + one debug)
- @
tidal/src/storage/vector/lifecycle/normalize.rs:25-39— blast: Low — per-write (per embedding insert/update), not per-query, not per-candidate. The debug pass is compiled out in release. The one Vec alloc is required because the API returns an owned normalized copy. - evidence: l2_normalize does pass 1
v.iter().map(|x| x*x).sum()for norm_sq (:26), then pass 2v.iter().map(|x| x/norm).collect()allocating a fresh Vec (:31), then a debug_assert (:34-37) that does a THIRD full pass recomputing the norm. Same scalar non-vectorized shape as l2_distance_sq. The freshcollect()is one alloc per embedding write. This is per-WRITE, off the read hot path. - fix: Leave it. This is correctly off the read hot path; embedding writes are far rarer than queries and the alloc is semantically necessary (returns owned Vec). The only defensible change is the same chunked-fold treatment for the norm_sq pass IF a write-path flamegraph ever shows normalization hot — but it will not be hot relative to the HNSW graph insert (USearch add) that follows it. Calling this out only so the sweep does not 'fix' it: a clone/alloc off the hot path is not a finding.
- bench: No dedicated bench; bench_ann_insert_single_10k (tidal/benches/vector.rs:183) covers the write path end-to-end and would show if normalization ever surfaced. No action recommended. — est: None recommended — per-write, dominated by the subsequent HNSW insert. Documented here to mark it as deliberately NOT a target, not a missed win.
- @
Ranking score-loop allocation — 62/100
Canonical fast pattern: The codebase already demonstrates the fast pattern in two adjacent places that make the slow spots stand out by contrast: (1) score_candidates hoists per-query invariant work OUT of the per-candidate loop — lowered_session_keywords is lowercased once (executor/mod.rs:412) and now is converted to now_ns once (mod.rs:417); (2) finalize pre-builds the alphabetical titles map once and the comparator borrows from it instead of re-allocating two Strings per comparison (mod.rs:725-741). The canonical rule those two embody — "build the invariant once outside the loop, borrow inside it; allocate to the size you know up front" — is exactly what the snapshot-String and Vec::new() sites violate. The query layer also already does the right top-K thing in candidate_gen.rs (select-and-truncate to max_candidates, mod.rs:83-110), which is the model the final-stage sort should follow once a limit is in scope.
Deviations: n/a
-
[HIGH/high] signal_snapshot Strings allocated for EVERY candidate but read only for the returned page (~limit items)
- @
tidal/src/ranking/executor/mod.rs:439-445 (push), :602/:650/:681 + scoring.rs vec![(...,...)] sites; consumed at tidal/src/query/executor/pipeline.rs:504-516— blast: High — per-candidate, every RETRIEVE and SEARCH query (both score_personalized and score_with_session paths, query/executor/mod.rs:402/415 and query/search/executor/pipeline.rs:693/703). At the spec's 200-candidate scoring target this is hundreds of short-lived String allocations per query, of which only ~limit are ever read. - evidence: compute_raw_score builds a Vec<(String,f64)> signal_snapshot for every scored candidate: score_by_sort returns vec![("view".to_string(), v), ...] (scoring.rs:140-143, 200-201, 292, 337-340, 366-368, 397-398, 427-430, 481-485, 519-523), then format!("{}_boost"/"_penalty"/"_decay") + 3 fixed .to_string() ("relevance", "co_engagement", "preference_affinity") push more. Each is a heap allocation. But pipeline.rs slices page_slice = final_candidates[offset..end] (pipeline.rs:481-483) and only that page's snapshot is turned into Signal structs (pipeline.rs:504-516). For a 200-candidate / limit=20 RETRIEVE, ~90% of these snapshot Strings are allocated, dropped, and never serialized. The snapshot is unconditional — there is no explain/debug flag gating it.
- fix: Defer snapshot materialization until after pagination. Carry the cheap explain inputs per candidate without Strings (e.g. a SmallVec<[(SignalTypeId-or-&'static-str, f64); 4]> referencing interned/static signal names, or store raw (signal_idx, kind, value) and only build the (String,f64) pairs for the page_slice rows that survive to pipeline.rs:504). If full lazy plumbing is too invasive, at minimum gate snapshot construction behind a per-query explain flag so the common non-explain path allocates nothing. The signal names are a tiny fixed vocabulary ('view'/'like'/'share'/'completion'/...), so a &'static str table or Box interner removes the alloc entirely.
- bench: tidal/benches/ranking.rs (score_200_trending / score_200_full_pipeline) measures the alloc-inclusive scoring cost today; add a benches/ranking case that scores 200 candidates with limit=20 and compares snapshot-eager vs snapshot-deferred to isolate the wasted ~180 allocations. Confirm end-to-end via the query bench RETRIEVE p99. — est: Likely the largest single win in this category: eliminating ~80-90% of per-query String allocations on the scoring path. Magnitude needs the criterion delta to confirm, but allocator pressure at 200 candidates × multiple Strings each is real and on the hot path.
- @
-
[MEDIUM/high] format!("{}_boost"/"_penalty"/"_decay") re-allocates an identical String per candidate per rule
- @
tidal/src/ranking/executor/mod.rs:602, 650, 681— blast: Medium — per-candidate × per-rule, every query whose profile has boosts/penalties (most do). Overlaps the snapshot finding above but is independently fixable and a clear mechanical waste. - evidence: Inside the boost loop (mod.rs:587-605) and penalty loop (mod.rs:638-653), format!("{}_boost", b.signal) / format!("{}_penalty", p.signal) runs once per (candidate × rule) whenever the weighted term is non-zero. b.signal is a String already in the profile; the produced label (e.g. "view_boost") is identical for every candidate — it depends only on the rule, not the candidate. Builtin profiles carry 1-4 boosts (for_you has 3: builtins.rs:280-294; trending has 2: builtins.rs:149-160), so this is up to ~4 format! allocations per surviving candidate, each rebuilding the same string, plus running the fmt::Arguments machinery.
- fix: Precompute the label String (or Box) ONCE per boost/penalty rule before the candidate loop — store it alongside the rule or in a small parallel Vec built at score_candidates entry — and clone the precomputed label instead of re-running format! per candidate. Better, fold into the snapshot-deferral fix: store (rule_index, value) per candidate and format the label only for the returned page. The decay label (mod.rs:681) fires at most once per candidate, so it is lower priority than the boost/penalty loops.
- bench: tidal/benches/ranking.rs score_200_full_pipeline (trending, 2 boosts); add a score_200_for_you case (3 boosts) to size it more strongly. — est: Small-to-moderate on its own (a handful of fewer allocations + no fmt machinery per candidate); largely overlaps the snapshot finding. Confirm with criterion — do not claim a number without the before/after.
- @
-
[MEDIUM/high] scored: Vec::new() grows by reallocation instead of reserving the known candidate count
- @
tidal/src/ranking/executor/mod.rs:418— blast: Medium — per-query, the main output buffer of every score path. The reallocs are O(log n) in count and amortized O(n) in bytes; the cost is the repeated memcpy of growing ScoredCandidate elements. - evidence: let mut scored: Vec = Vec::new(); then push once per surviving candidate (mod.rs:439-445). With no with_capacity the Vec reallocates and memcpys its backing buffer at the doubling growth points (0→4→8→16→...→256) as it fills toward ~candidates.len(). ScoredCandidate is a fat element (EntityId + f64 + Vec + 2 Options), so each regrowth memcpys all elements so far. candidates.len() is known at loop entry. The codebase reserves elsewhere (candidate_gen sizes to max_candidates), so this site is an inconsistency.
- fix: Vec::with_capacity(candidates.len()). Excludes/gates only remove candidates, so candidates.len() is a tight upper bound — at worst a small over-allocation, never an under-allocation that forces a regrowth. Eliminates every intermediate realloc+memcpy on the hot output buffer for one trivial change.
- bench: tidal/benches/ranking.rs score_200_trending / score_200_full_pipeline (200 candidates crosses several growth thresholds: 128→256). — est: Small but free and zero-risk: removes ~6 reallocations and their cumulative memcpy for 200 candidates. Confirm direction with the ranking bench; do not overstate.
- @
-
[LOW/medium] finalize does a full sort_unstable_by over all candidates where the caller ultimately keeps only ~limit
- @
tidal/src/ranking/executor/mod.rs:743-771— blast: Low — per-query, but the savings are bounded: n is capped at max(limit*4, 200) by candidate_gen (candidate_gen.rs:66-68), so n·log n vs partial-sort saves at most a constant factor on a few-hundred-element sort — not the dominant cost relative to per-candidate ledger reads and the allocations above. - evidence: finalize sorts the entire scored slice with sort_unstable_by (O(n log n)) before returning. The in-code comment (mod.rs:744-745) notes select_nth_unstable_by would need
limitat this site, which is not threaded in. BUT verifying downstream shows the full sort is NOT trivially replaceable: after finalize the query layer runs cohort rescore (query/executor/mod.rs:474, reorders), exploration injection that APPENDS candidates (mod.rs:480), creator backfill, and a diversity re-ordering pass before pagination/truncation (pipeline.rs:481). Several stages consume more than the toplimitand reorder, so a naive select_nth tolimithere would corrupt them. normalize (helpers.rs:261) also needs the global min/max over all candidates. - fix: Do NOT replace the finalize sort blindly — downstream stages depend on full ordering. The legitimate version threads the effective working-set budget (post-exploration, post-diversity size, not the user
limit) into finalize and uses select_nth_unstable_by only when that budget << n AND the downstream stages are confirmed to need no more than the partitioned prefix. That is a multi-stage query-pipeline change, co-designed with whoever owns diversity/exploration, not a local edit. Until then leave the full sort and spend effort on the allocation findings, which move the number more. - bench: tidal/benches/ranking.rs (sort is inside score_*); to isolate, a microbench over a pre-scored Vec comparing sort_unstable_by vs select_nth_unstable_by at n=200,limit=20. Validate end-to-end via the query bench RETRIEVE p99 — a sort win that does not move end-to-end p99 is not worth the invariant risk. — est: Small and uncertain at n≤~800; needs a profile to confirm the sort is even a measurable fraction of scoring time before any work. Likely premature relative to the snapshot/format!/capacity wins — fix those first and re-profile.
- @
Diversity/MMR selection allocation — 62/100
Canonical fast pattern: The correct shape for this post-scoring reorder is allocation-light and clone-once: accept/reject by EntityId (a cheap Copy newtype over u64) using a HashSet, never by cloning ScoredCandidate; do all per-stage filtering by iterating &ScoredCandidate references (no .cloned().collect() of the remainder); borrow &str for the format-count HashMap key rather than cloning the String; and clone the surviving candidates exactly once — at the final emit pass — into a Vec pre-sized to max_needed. Because select() is always invoked with target_count == candidates.len() (query/executor/pipeline.rs:418, query/search/executor/pipeline.rs:225), max_needed == candidates.len(), so the relaxation path walks the full scored set: every avoidable per-candidate clone is paid once per RETRIEVE/SEARCH query whenever any diversity constraint is set.
Deviations: n/a
-
[HIGH/high] Surviving candidates are deep-cloned twice per query (greedy_select push + final emit)
- @
tidal/src/ranking/diversity/selector.rs:250 and selector.rs:166-170— blast: High — executes once per RETRIEVE and per SEARCH query whenever max_per_creator or format_mix is set (the common feed case). Up to ~200 candidates (scale baseline), each cloned twice, so ~2x the necessary heap traffic of the entire result set on the per-query critical path that feeds the <50ms RETRIEVE p99 budget. - evidence: greedy_select accepts candidates by pushing candidate.clone() into
selected(line 250). select() then NEVER consumes that cloned Vec — it only reads each accepted entity_id into theacceptedHashSet (lines 107-109) and throws the clones away. The real result is rebuilt by a SECOND full deep clone:candidates.iter().filter(...).cloned().collect()(lines 166-170). So every selected ScoredCandidate is heap-cloned twice. ScoredCandidate (ranking/executor/context.rs:46-54) holdssignal_snapshot: Vec<(String,f64)>(non-empty on the RETRIEVE path — populated at ranking/executor/mod.rs:442 / query/executor/helpers.rs:310) andformat: Option<String>; each clone is a Vec alloc + one String alloc per snapshot tuple + one format String alloc. The whole emit pass is pure dead weight in stage 0 since greedy_select already produced the same objects — they were discarded only to be re-cloned. - fix: greedy_select should return the accepted EntityIds (or push directly into the shared
acceptedHashSet) and NOT clone ScoredCandidate at all — change itsselected: Vec<ScoredCandidate>to aVec<EntityId>/insert-into-set, since the caller only consumes entity_ids. Keep the single final emit pass (lines 166-170) as the one and only clone of survivors into a Vec::with_capacity(max_needed). That removes one of the two full-result deep clones outright (the dead stage-0 clone) for every constrained query. - bench: tidal/benches/diversity.rs: diversity_200_max_per_creator_2 and diversity_200_combined directly measure this; add a variant whose make_200_candidates populates a non-empty signal_snapshot to expose the Vec/String clone cost the current empty-snapshot bench hides. — est: Eliminates one full-result-set deep clone per constrained query; rough estimate 30-45% of selector allocation traffic on the stage-0 common case. Needs the criterion delta (with a non-empty-snapshot candidate) to confirm the absolute win against the end-to-end RETRIEVE p99.
- @
-
[HIGH/high] Relaxation stages 1 and 2 deep-clone the entire non-accepted remainder into fresh Vecs
- @
tidal/src/ranking/diversity/selector.rs:118-122 and selector.rs:140-144— blast: Medium — fires only when stage 0 under-fills (constraints bind hard: popular-creator-heavy or skewed-format candidate sets), but on exactly those queries it adds up to two extra full-set deep clones. Per-query on the constrained RETRIEVE/SEARCH path. - evidence: Each of stage 1 and stage 2 builds
remaining: Vec<_> = candidates.iter().filter(|c| !accepted.contains(...)).cloned().collect()— a full deep clone (Vec<(String,f64)> + format String per item) of every not-yet-accepted candidate, purely so greedy_select can iterate them. greedy_select only needs &[ScoredCandidate] to read fields and clone the few it accepts; it never needs to own the rejected ones. Because select() is called with target_count == candidates.len() (query/executor/pipeline.rs:418), max_needed == candidates.len(), so in the worst case (e.g. all-same-creator, the diversity_200_worst_case_relaxation bench)remainingis nearly the whole 200-candidate set, cloned again in stage 1 and again in stage 2 — up to two more full-set deep clones stacked on the stage-0/emit clones. - fix: Do not materialize
remainingat all. greedy_select can take the fullcandidatesslice plus the sharedacceptedset (passed by reference) and skip any candidate already inacceptedinside its own loop — it already maintains creator_counts/format_counts, so adding anif accepted.contains(&id) { continue; }guard is O(1) per item and removes both .cloned().collect() allocations. The score-sorted order is preserved because it still walkscandidatesin order. - bench: tidal/benches/diversity.rs: diversity_200_worst_case_relaxation is the exact stressor (all stages triggered); diversity_200_max_per_creator_2 covers the partial-relaxation case. — est: Removes up to two full-set deep clones on hard-constrained queries; estimate 20-40% of allocation on the relaxation path. Confirm with the worst_case_relaxation bench using non-empty snapshots.
- @
-
[MEDIUM/high] format_counts HashMap clones the format String as its key on every accept
- @
tidal/src/ranking/diversity/selector.rs:204 and selector.rs:248— blast: Medium — one String alloc per accepted candidate-with-format, per constrained query, across all relaxation stages. Lower per-item cost than the candidate clones but on the same per-query hot path. - evidence: greedy_select keys format_counts as HashMap<String,usize> (line 204) and inserts via
*format_counts.entry(fmt.clone()).or_insert(0)(line 248) — a heap String allocation per accepted candidate that has a format, even though the format strings are owned bycandidate.formatfor the whole call and a borrow would suffice. collect_violations (line 283) already does this the right way with HashMap<&str,usize>, so the borrowed-key pattern is established in the same file. Formats are low-cardinality ('video'/'audio'/...), so the map has a handful of distinct entries but reallocates the key String on every repeat insert. - fix: Change format_counts to HashMap<&str,usize> keyed on candidate.format.as_deref() (the candidates slice outlives the function), mirroring collect_violations at line 283. Removes the per-accept key allocation entirely; the value path is unchanged.
- bench: tidal/benches/diversity.rs: diversity_200_format_mix and diversity_200_combined. — est: Small but free: ~1 String alloc per accepted item removed, low-single-digit % of selector time. Borrow-vs-clone, no correctness change.
- @
-
[LOW/medium] Per-query HashSet/HashMap allocate with default capacity and rehash as they grow
- @
tidal/src/ranking/diversity/selector.rs:91 (accepted HashSet), selector.rs:203-204 (creator_counts/format_counts)— blast: Low — a few table growth reallocations per constrained query; dwarfed by the candidate deep-clones above. Per-query. - evidence:
accepted: HashSet<u64>(line 91) andcreator_counts: HashMap<u64,usize>(line 203) are created with ::new() (zero capacity) and grow to ~max_needed / ~n_creators entries over the query, triggering incremental reallocation+rehash. max_needed is known at line 66; n_creators is bounded by candidates.len(). EntityId::as_u64 (schema/entity.rs:19) is a trivial Copy so the keys themselves are cheap — the only cost is the table reallocs. greedy_select already pre-sizesselectedwith Vec::with_capacity(limit) (line 202), so the with-capacity pattern is in-file. - fix: Construct
acceptedwith HashSet::with_capacity(max_needed) and creator_counts with HashMap::with_capacity(max_needed) (a safe upper bound). Consider a faster non-DoS hasher (e.g. FxHashMap) for these internal u64-keyed maps since keys are trusted entity ids, not external input — but only after the clone wins above land, as this is in the noise relative to them. - bench: tidal/benches/diversity.rs: diversity_200_combined; would only show after the dominant clone costs (findings 1-2) are removed and the table reallocs become visible. — est: Marginal (<5%) and only measurable after the deep-clone costs are eliminated. Pre-sizing is a safe, free change; the hasher swap needs a criterion delta to justify.
- @
-
[LOW/high] format!/String allocation in collect_violations is on the cold path, not the hot path
- @
tidal/src/ranking/diversity/selector.rs:273-274 and selector.rs:294-295— blast: Low — bounded by the number of distinct violating groups (typically 0 on satisfied queries, a handful on relaxed ones). Effectively cold. - evidence: The seed flagged format!() at lines 274/295 as a per-candidate hot-loop cost. Reading the code refutes that: those format! calls live in collect_violations, inside
for (cid,count) in &counts { if *count > max { ... format!(...) } }(lines 270-277) and the analogous format branch (lines 289-298). They fire ONCE PER DISTINCT VIOLATING creator/format, only when constraints are actually breached — not per candidate. The per-candidate accept loop (greedy_select, lines 206-251) contains no format!. The.into()on the constraint name (lines 273, 294) is a cheap static-str-to-String. ConstraintViolation strings also feed warnings via format! at the call sites (query/executor/pipeline.rs:422), again only on relaxation failure. - fix: No change warranted on performance grounds — this is correct error-reporting allocation on the failure path and is not a hot-loop cost. Do not micro-optimize it; the deep-clones (findings 1-2) are where the time is. Flagged here to explicitly retire the seed observation about lines 274/295 being hot.
- bench: tidal/benches/diversity.rs: diversity_200_worst_case_relaxation is the only bench that exercises the violation path; the satisfied-constraint benches never reach these format! calls. — est: None — premature micro-optimization; explicitly out of scope. Recorded to correct the seed observation.
- @
Query retrieve / fusion / candidate-gen materialization — 72/100
Canonical fast pattern: The hottest-correct pattern in this category is already present and should be the template for the rest: (1) hoist per-query work out of the stage loops — combined_filter() is computed ONCE in both pipelines (retrieve pipeline.rs:47, search pipeline.rs:118) with an explicit comment that it used to clone the filter Vec up to five times per query; (2) bound the working set to O(cap) instead of O(N) with select_nth_unstable_by + truncate rather than a full sort — signal_ranked_candidates (candidate_gen.rs:98-112) and the ReducedCandidates New truncate (retrieve pipeline.rs:215) both do this; (3) a non-cloning Ref-guard accessor (CreatorItemsBitmap::get_ref, entities/mod.rs:92) exists alongside the cloning get. The deviations below are the spots that have NOT yet adopted that discipline: they clone whole bitmaps by value, materialize the full match set into HashMaps/Vecs before the page slice, or full-sort when only the top-K is wanted.
Deviations: n/a
-
[HIGH/high] User-context suppression clones four whole RoaringBitmaps by value per FOR USER query
- @
tidal/src/query/executor/mod.rs:563-636 (seen/hidden/saved/liked); accessors at tidal/src/entities/user_state.rs:144,191,379,399— blast: High — per-query, on EVERY personalized RETRIEVE and SEARCH (the for_you / feed path, the most common serving query). seen_bitmap + hidden_items clone every time; saved/liked clone whenever those inclusion filters are set. - evidence: apply_user_context_suppression calls user_state.seen_bitmap(user_id) and hidden_items(user_id) UNCONDITIONALLY for every FOR USER query, plus saved_bitmap/liked_bitmap when a Saved/Liked filter is present. Each accessor body is
self.<map>.get(&user_id).map(|r| r.clone()).unwrap_or_default()— a deep heap clone of the entire per-user RoaringBitmap (containers + run/array/bitmap blocks) just to call.contains(i)inside aretain. For a power user with tens of thousands of seen items this is a multi-KB allocation + memcpy on the hot path, immediately discarded after the retain. The non-cloningRef-guard pattern already exists on CreatorItemsBitmap::get_ref (entities/mod.rs:92) but the user_state accessors offer no equivalent, so the caller is forced to clone. - fix: Add
with_seen_bitmap/with_hidden_items/with_saved_bitmap/with_liked_bitmapaccessors that return adashmap::Ref<'_, u64, RoaringBitmap>(mirroring get_ref) and have the suppression stage hold the guard across the singleretaininstead of cloning. The retain only reads.contains(), so a borrow is sufficient; the DashMap shard read-lock is held briefly per bitmap. Where a borrow can't be held across multiple retains, intersect candidates against the bitmap in one pass. Keep the existing clone accessors for callers that genuinely need ownership. - bench: tidal/benches/scale.rs
for_you(1M-item personalized path) is the right end-to-end measure; add a query bench variant that seeds a large seen/saved bitmap (e.g. 50k entries) and runs a FOR USER retrieve so the clone cost is visible — the existing query.rs benches use no user context so they miss this entirely. — est: Eliminates 2-4 whole-bitmap heap clones per personalized query; for large per-user state this is the dominant allocation in Stage 2.5. Needs the scalefor_youbefore/after to quantify, but mechanically it removes an O(seen-set-size) alloc+copy that scales with the user's history, not the result size.
- @
-
[MEDIUM/medium] signal_ranked_candidates scans the entire signal DashMap per query with a sharded-lock walk
- @
tidal/src/query/executor/candidate_gen.rs:88-105 (loop overledger.entries()); iterator source tidal/src/signals/ledger/core.rs:683— blast: Medium — per-query, only for profiles using CandidateStrategy::SignalRanked (e.g. trending). Cost grows linearly with total ledger size (entities x signal-types), independent of how many match the requested signal, so it degrades at the 10M-item scale tier exactly where it matters. - evidence: The loop does
for entry in ledger.entries()over&DashMap<(EntityId, SignalTypeId), EntitySignalEntry>— a full scan of every (entity, signal-type) cell in the ledger, filtering to one type_id and calling hot.current_score per matching cell. DashMap iteration acquires each shard's RwLock in turn and chases per-entry heap pointers (cache-miss per cell). The code's own doc-comment (candidate_gen.rs:46-62) acknowledges this is O(N over live cells) with no per-signal-type index. The working SET is correctly bounded to O(cap) via select_nth_unstable, but the TRAVERSAL is unavoidably O(total ledger cells) and touches every shard lock. - fix: This is an indexing gap, not a micro-opt: add a per-signal-type secondary structure (entity ids, or a small top-K maintained on the write path) so candidate generation reads O(cap) cells instead of scanning O(N). Until then, at minimum confirm with a flamegraph at 1M/10M whether this scan dominates trending-query latency before investing — the doc comment flags it as the place to look first. Do not micro-tune the inner loop; the traversal count is the cost.
- bench: tidal/benches/query.rs
retrieve_200_signal_rankedmeasures the small case; the real signal is tidal/benches/scale.rstrendingat 1M — re-profile there to confirm the scan is the hotspot before adding the index. — est: At small ledgers negligible; at 1M-10M cells a per-type index turns an O(N) shard-walk into an O(cap) lookup — potentially orders of magnitude on the trending candidate-gen stage. Needs the scaletrendingbench to confirm it's actually the bottleneck (scoring may still dominate).
- @
-
[MEDIUM/high] SEARCH builds bm25_map and ann_map over the ENTIRE retrieved match set, used only for the returned page
- @
tidal/src/query/search/executor/pipeline.rs:155-163 (build) and 786-787 (consumed only inside thepageslice in assemble_results)— blast: Medium — per-query, on every SEARCH (hybrid/text/vector). Two HashMap allocations + N inserts where N = retrieval pool size, of which onlylimitlookups are ever used. - evidence: After retrieval, the pipeline eagerly builds
bm25_map: HashMap<u64,f32>andann_map: HashMap<u64,f32>from the full bm25_results and ann_results (up to bm25_cap = limit20 floored at 200, and ANN k = limitmultiplier). These two HashMaps are then consulted ONLY in assemble_results at lines 786-787, and only for the items inpage(offset..end, typicallylimitrows). So for a limit=20 query the code hashes and stores ~hundreds of (u64,f32) pairs to later look up 20 of them. HashMap construction is a per-entry hash + allocation; the over-build is proportional to the retrieval pool, not the page. - fix: Defer the explainability lookup to the page: after the final page slice is known, look up each page item's bm25/semantic score directly from the (already sorted) bm25_results/ann_results via a binary search or a small HashMap built over only the page ids. Alternatively build the two maps lazily only if the result type actually surfaces bm25_score/semantic_score. This drops the work from O(retrieval-pool) to O(page-size).
- bench: tidal/benches/search.rs (
search_text_10k) and tidal/benches/scale.rstext_only/text_filtered; add an assertion-free variant with a small limit over a large match pool to expose the over-build delta. — est: Removes two HashMaps sized to the retrieval pool (hundreds to thousands of entries) in favor of page-sized work (~limit). Small absolute microseconds per query but pure waste on the hottest SEARCH path; confirm with search bench before/after.
- @
-
[LOW/medium] RRF fuse materializes a HashMap then a full Vec sort even when only top-K is consumed
- @
tidal/src/query/fusion.rs:80-109 (HashMap build + into_iter().collect() + sort_by)— blast: Low-to-Medium — per-query on Hybrid SEARCH only. Measured directly by the fusion bench at 1k/list. The map build is inherent to RRF; the full sort is the avoidable part. - evidence: HybridFusion::fuse builds a HashMap<u64,f64> over bm25+ann (capacity = sum of both lens), then
.into_iter().collect()into a Vec, thensort_by(full O(M log M) comparison sort) over the entire fused union. The caller in search pipeline.rs:171-173 immediately maps the full fused list intocandidatesand feeds it to filtering/scoring — but downstream only the top page survives. For Hybrid mode with 1k+1k inputs (the fusion bench fixture) that's a ~2000-entry HashMap and a full 2000-element sort per query. The HashMap is required for the rank-merge, but the final full sort is heavier than necessary when limit << union size. - fix: Where the caller only needs the top-K fused candidates, replace the trailing
sort_bywithselect_nth_unstable_by+ sort of just the survivors (the same pattern signal_ranked_candidates already uses at candidate_gen.rs:108-112). Thread the effective candidate budget into fuse, or expose afuse_top_k. Keep the deterministic ascending-id tie-break. The HashMap build stays; only the sort shrinks from O(M log M) to O(M) partition + O(K log K). - bench: tidal/benches/fusion.rs
rrf_fuse_1k_per_listandroute_hybrid_1kmeasure this exactly — a before/after there is direct. — est: Sort drops from full-union O(M log M) to O(M)+O(K log K) when K (≈limit*20) < M; modest at 1k but grows with retrieval pool. Must measure via fusion bench — at small M the constant-factor win may be in the noise, so confirm before shipping.
- @
-
[LOW/high] Per-result-item String allocations in both result-assembly loops
- @
tidal/src/query/search/executor/pipeline.rs:772-779 and tidal/src/query/executor/pipeline.rs:507-514— blast: Low — per returned item per signal, i.e. O(page-size x signals-per-item), bounded bylimit(≤500) so it's off the candidate-scaled hot path. This is result serialization, not the scoring/filter inner loop. - evidence: Both assembly loops, per returned item, build
signals: Vec<Signal>by cloning each snapshot name (name: name.clone(), snapshot is Vec<(String,f64)>) and allocating a freshsource: "decay_score".to_string()for every signal of every item. The"decay_score"literal is constant — allocating a new String for it on each signal is pure waste. The name.clone() is harder to avoid (Signal owns its name) but the source String is a constant. - fix: Make
Signal.sourcea&'static strorCow<'static, str>and use the literal directly instead of.to_string(), eliminating one allocation per signal. If the Signal type must own a String for the public API, intern the constant once. Leave name.clone() unless the snapshot can be moved out of the ScoredCandidate (it can, in the final assembly, since the candidate is consumed) — moving rather than cloning the name would remove the second alloc. - bench: tidal/benches/query.rs
retrieve_200_trending_with_signals(items carry signal snapshots) and the search benches; the delta is small and only visible with multi-signal snapshots and a full page. — est: Removes one String alloc per signal per returned item (the constant source). Bounded by limit so the absolute win is small; worth doing as a cheap, zero-risk cleanup but it will NOT move end-to-end p99 on its own — do it alongside the bitmap-clone fix, not instead of it.
- @
Bitmap / range / filter index intersection — 72/100
Canonical fast pattern: The fast pattern already present here: evaluate_and intersects with the in-place by-reference operator result &= &child_bitmap (evaluator.rs:144) — which roaring 0.10.12 implements as a container-retaining mutation (ops.rs:259), not a clone-and-rebuild — and it short-circuits the moment the running intersection goes empty (evaluator.rs:141-143). It also orders children smallest-cardinality-first so the running set shrinks fastest. That is the correct skeleton. The deviations below are all about materializing full result bitmaps that the caller never needs in full: every leaf goes through BitmapIndex::get() which clones the entire value-bitmap under the lock, and the AND path eagerly materializes every child (including a full universe.clone() for deferred/user-state children) before it can short-circuit. The canonical fix is to (a) stop cloning out of the index when the consumer only does membership tests, and (b) order/short-circuit BEFORE materializing the expensive children, not after.
Deviations: n/a
-
[HIGH/high] BitmapIndex::get() clones the entire value-bitmap per leaf per query, then the executor only membership-tests a few hundred candidates against it
- @
tidal/src/storage/indexes/bitmap.rs:188-194 (get), consumed at tidal/src/query/executor/pipeline.rs:303-305 and tidal/src/query/search/executor/pipeline.rs:520— blast: High — per-query, per-filter-leaf. Every RETRIEVE/SEARCH with a metadata filter (the common case for a feed) pays this. A 3-leaf AND clones three full bitmaps. The clone is also held across the read lock, lengthening lock hold time under concurrent writes to the same field. - evidence:
get()doesmap.get(value).filter(...).cloned()— a deep clone of the whole RoaringBitmap for that field value, allocated under the read lock and returned by value. Every filter leaf (CategoryEq/FormatEq/CreatorEq/Tag) goes through it viaeval_to_bitmap(evaluator.rs:81-86). At 1M items a 50%-selectivity value (e.g. category="jazz") clones a ~500K-id bitmap (hundreds of KB across roaring containers, malloc + memcpy of every container's array/bitset store). The query executor then uses that bitmap only forcandidates.retain(|id| bitmap.contains(i))against a candidate list that Stage 1 typically capped to(limit*4).max(100)(pipeline.rs:210) — i.e. a few hundred ids. We allocate and copy half a million ids to answer a few hundred membership questions. - fix: Stop returning an owned clone when the caller only needs membership. Two complementary moves, both in prose: (1) Add a borrowing API — e.g.
with_value<R>(&self, value, f: impl FnOnce(Option<&RoaringBitmap>) -> R) -> Rthat runs the closure under the read guard so the caller can intersect/test against the live bitmap without cloning; have the evaluator's leaf-vs-candidate path use it. (2) For the executor's dominantretain(contains)consumption, invert the data flow: instead of materializing the leaf bitmap and testing every candidate, test each candidate against the index directly (the candidate set is the smaller side). Keep the existing ownedget()only for the genuine union/NOT paths that need a working copy. Where an owned working set is unavoidable (the first AND child that becomes the mutable accumulator), clone exactly one — the smallest — not all of them. - bench: tidal/benches/filters.rs — extend with a 1M-universe variant of
filter_bitmap_and_2and a new bench that measures filter-against-candidates with a realistic 200-candidate list (today all filter benches call.into_bitmap()on a 10K universe, which hides the clone cost). Confirm withcargo bench --bench filtersbefore/after. — est: Large for filtered queries at scale — eliminates one full-bitmap heap alloc+memcpy per leaf. Needs the 1M bench to quantify; at 10K the absolute cost is small, which is why the current bench doesn't show it.
- @
-
[MEDIUM/high] evaluate_and materializes EVERY child to a full bitmap before it can short-circuit, including a full universe.clone() for deferred/user-state children
- @
tidal/src/storage/indexes/filter/evaluator.rs:129-146 (eager materialize+sort), with the deferred-child cost at evaluator.rs:99-111 (self.universe.clone())— blast: Medium-High — per-query for any AND that mixes index-backed and deferred filters (the standard personalized-feed query path through executor/pipeline.rs and search/executor/pipeline.rs). The universe clone is O(total items) allocation+copy. - evidence:
evaluate_andfirst doeschildren.iter().map(|c| self.eval_to_bitmap_bounded(...)).collect()— it walks and fully materializes every child subtree up front, then sorts bylen(), then intersects. The short-circuit at line 141 (if result.is_empty()) can only fire AFTER all children are already materialized. Two concrete costs: (a) a non-existent or tiny-selectivity leaf can't prune the work of materializing its big siblings — they're already built. (b) any deferred/user-state child (Unseen, Unblocked, MinSignal, InCollection, SocialGraph, NearLocation) returnsself.universe.clone()at line 110 — a full clone of the 1M-id universe bitmap — purely so the AND can intersect-and-discard it (the real filtering happens later in executor Stage 2.2-2.5). A combined filter that ANDs a metadata predicate with aFOR USERuser-state predicate (the normal feed query) clones the entire universe per such child, every query. - fix: In prose: (1) For deferred/user-state variants inside an AND, treat them as the identity element — skip them entirely rather than synthesizing
universe.clone()and intersecting it away; they are re-applied in executor Stages 2.2-2.5 regardless, so intersecting against the universe is a provable no-op. This removes the universe clone from the hot path. (2) Evaluate index-backed leaves cheaply by cardinality FIRST (BitmapIndex::cardinality / RangeIndex are O(1)-ish vs full materialization), order ascending, and materialize lazily so an empty/tiny leaf short-circuits before the large siblings are ever built. The current code chose eager materialize specifically to avoid a double subtree walk; the middle ground is to materialize only the running accumulator and fold the rest in cardinality order, building each child only when reached and bailing on the first empty result. - bench: tidal/benches/filters.rs — add a bench that ANDs a real metadata leaf with a deferred variant (e.g. Unseen) at 1M universe to expose the universe-clone; and an AND where the most-selective leaf is empty, to measure the wasted materialization of siblings. — est: Removes a full O(universe) clone per deferred AND-child and avoids materializing siblings of an empty/tiny leaf. Significant for FOR USER feed queries; quantify with the new bench.
- @
-
[LOW/medium] sort_unstable_by_key(RoaringBitmap::len) recomputes len() O(n log n) times, and each len() is O(containers)
- @
tidal/src/storage/indexes/filter/evaluator.rs:133— blast: Low — per-query, and only material for wide ANDs (many children) over large bitmaps. Most filters are 2-3 leaves where n log n is trivial. Dominated by the clone costs above. - evidence:
bitmaps.sort_unstable_by_key(RoaringBitmap::len)—sort_unstable_by_keydoes NOT cache the key; it calls the key function on each comparison, solen()runs O(n log n) times for n children. roaring 0.10.12len()isself.containers.iter().map(|c| c.len()).sum()(inherent.rs:628-630) — O(containers), not a cached scalar. For a wide AND (many children, each spanning many 64K-id containers at 1M scale) this is repeated work, on top of being a branch/iterate-heavy comparator. - fix: In prose: compute each bitmap's
len()exactly once into a(u64, RoaringBitmap)pair (or usesort_by_cached_key), then sort by the precomputed key. Better still, if Finding #2's lazy/cardinality-first ordering lands, this sort disappears entirely because ordering is driven by the O(1) index cardinalities before materialization rather than by re-measuring materialized bitmaps. - bench: tidal/benches/filters.rs — a wide-AND bench (e.g. AND of 8-16 leaves) at 1M universe would surface it; today no bench exercises a wide AND so the cost is invisible. — est: Small in absolute terms; meaningful only for wide ANDs. Largely subsumed if Finding #2's reordering lands. Confirm only if a wide-AND bench shows the sort in a flamegraph.
- @
-
[LOW/medium] evaluate_or accumulates into a fresh empty bitmap, missing the chance to seed from (and reuse the allocation of) the largest child
- @
tidal/src/storage/indexes/filter/evaluator.rs:149-158— blast: Low — per-query for OR filters (category IN [...], multi-tag). Less common than AND on the feed path, and OR over a small in-list is cheap. - evidence:
evaluate_orstartsresult = RoaringBitmap::new()then foldsresult |= &childover every child. For OR, the result is at least as large as the largest child, so seeding the accumulator from a fresh empty map means the first|=reallocates/grows the accumulator from zero up to the first child's size, then again toward the union size. roaring's union grows the container vec incrementally. Materializing every child first (via eval_to_bitmap → get() clone) also pays Finding #1's clone for every OR leaf. - fix: In prose: evaluate children, then start the accumulator by taking ownership of the largest-cardinality child (move, not clone) and
|=the rest into it — the union must contain the largest child anyway, so seeding from it avoids regrowing from empty and saves one clone. NoteBitmapIndex::get_union(bitmap.rs:200-212) already does the right thing for the same-field multi-value case (single lock, fold under one guard) — route OR-of-same-field leaves through it instead of N separate get() clones + N unions. - bench: tidal/benches/filters.rs —
filter_bitmap_or_2exists but at 10K universe and only 2 children; extend to a wider OR at 1M to measure. — est: Modest; saves one clone and some reallocation on OR. Route same-field OR through get_union for a clearer win. Quantify with a wider 1M OR bench.
- @
-
[LOW/medium] RangeIndex::range unions every in-range value-bitmap into a fresh result, one container-merge per distinct value — O(distinct values in range) unions
- @
tidal/src/storage/indexes/range.rs:154-168— blast: Low-Medium — per-query for range/timestamp predicates, which are common on feeds (recency filters). Cost scales with the number of DISTINCT values in range, worst when the field is near-unique (timestamps in ns). - evidence:
range()doeslet mut result = RoaringBitmap::new(); for (_key, bitmap) in tree.range(...) { result |= bitmap; }. The BTreeMap is keyed by DISTINCT value, so a wide range (e.g.created_at > 7 days agoover a high-cardinality timestamp field where nearly every entity has a unique ns timestamp) iterates one tree entry per entity and does one|=(union) per single-element bitmap. That is O(N) lock-held tree-node pointer-chases (BTreeMap node traversal = cache-unfriendly) plus O(N) tiny unions, each allocating/locating a container. Theselectivity()path (range.rs:228) callsrange()purely to take.len(), throwing the whole unioned bitmap away. - fix: In prose: (1) For
selectivity(), do not materialize — sumbitmap.len()over the tree range instead of unioning then measuring (avoids building a throwaway union). Caveat for @tidal-engineer: summing lengths double-counts an entity that appears under multiple values; that's fine for a selectivity estimate but would be wrong if used as an exact count — keep it estimate-only. (2) For the materializingrange(), the union itself is unavoidable, but the BTreeMap node-chase is the cache cost; if range scans dominate at scale, a sorted Vec<(V, RoaringBitmap)> with binary-search-to-start + sequential scan is far more cache-friendly than BTreeMap traversal (better prefetch, no node pointer-chasing). That is a larger change — gate it on a flamegraph showing range scans hot at 1M+. - bench: tidal/benches/filters.rs builds a near-unique
created_atindex (ts.insert per id) but no bench calls a wide range/selectivity over it — add arange_wideand aselectivitybench at 1M to measure both the union and the throwaway-in-selectivity cost. — est: selectivity-without-materialize is a clear small win (removes a throwaway union per range predicate). The BTreeMap→sorted-Vec change is speculative — needs a scale flamegraph before committing.
- @
Signal hot/warm read path & cache-line layout — 78/100
Canonical fast pattern: The hot tier is the textbook done right: HotSignalState (hot.rs:48-66) is #[repr(C, align(64))], exactly 64 bytes, compile-asserted (hot.rs:69-70), with immutable fields and 3 AtomicU64 decay scores packed into one cache line so one entity's state is one cache miss and never false-shares with a neighbor. Reads/writes are lock-free CAS with documented, correct memory ordering (Acquire loads pair with Release/AcqRel stores; Relaxed only where the value is discarded). The canonical fast read is: look the entry up ONCE per candidate, then read every aggregation off the single ref. The slow spots are the places that deviate from that single-lookup rule, and one per-read transcendental.
Deviations: n/a
-
[HIGH/high] Per-candidate scoring does one DashMap lookup PER gate/exclude/boost term instead of one per candidate
- @
tidal/src/ranking/executor/helpers.rs:62-111 (read_agg), :169 (passes_gates), :212 (passes_excludes); tidal/src/ranking/executor/mod.rs:85-89 (boost loop); tidal/src/signals/ledger/core.rs:337,388 (entries.get)— blast: High - per-candidate, per-term on the RETRIEVE scoring path (the hottest ~45% of query time). For a 200-candidate query with a 4-6 term profile that is ~800-1200 redundant shard-lock acquisitions and hash+chase sequences per query. - evidence: read_agg() resolves the signal name to a type id and then calls ledger.read_decay_score_at / read_windowed_count_at / read_velocity_at, each of which does a fresh self.entries.get(&(entity_id, type_id)) (core.rs:337 and :388). passes_excludes (helpers.rs:211), passes_gates (helpers.rs:168), and compute_raw_score's boost loop (mod.rs:85) each iterate their term vectors and call read_agg per term. So a candidate with E excludes + G gates + B boosts pays (E+G+B) independent DashMap lookups for the SAME (entity_id, type_id) (or a small set of type_ids). Each lookup is: hash the (u64,u16) key, acquire the shard RwLock read guard (atomic + memory fence), then a pointer chase into the entry bucket -- a likely L2/L3 miss. The hot state itself is one cache line, but the lookup machinery in front of it is paid N times per candidate.
- fix: Look up the entry ONCE per (candidate, signal_type) and read all aggregations off the held dashmap ref, the way the bench at signals.rs:111-117 already does (one entries().get(), then entry.hot.current_score()). Concretely: resolve each profile's distinct signal type ids once at query start; for each candidate, group its excludes/gates/boosts by type id, take a single entries.get() per distinct type id, and evaluate every term that hits that entry against the held ref before releasing the shard guard. This collapses (E+G+B) lookups to (distinct type ids) -- typically 1-2 -- per candidate. Net: fewer hashes, fewer shard-lock atomics, and the entry's hot+warm cache lines are touched while already resident.
- bench: tidal/benches/ranking.rs end-to-end (confirm it moves RETRIEVE p99) + tidal/benches/signals.rs bench_200_entity_scoring_pass is the micro-proxy; add a variant that goes through read_agg/passes_gates (the real per-term path) rather than direct entry access, to isolate the redundant-lookup cost. — est: Medium-to-high on the scoring stage: removes (E+G+B-1) shard-lock acquisitions and hash+chase per candidate. Needs the ranking bench to confirm it moves end-to-end p99, but the redundant work is structural and verified by reading.
- @
-
[MEDIUM/high] current_score() computes .exp() on every read even when dt is small or zero
- @
tidal/src/signals/hot.rs:226-238 (current_score, the score * (-lambda*dt_secs).exp() line at :236)— blast: Medium - per-candidate on the DecayScore read path, and per-cell in the signal_ranked_candidates O(N) full-ledger scan (candidate_gen.rs:88-105) which calls it with lambda=0.0 for every matching cell. - evidence: current_score is the per-candidate DecayScore read (read_agg SignalAgg::DecayScore -> read_decay_score_at -> current_score, helpers.rs:96-98). It unconditionally evaluates (-lambda * dt_secs).exp(). exp() is ~15-40 cycles (libm) and is a hard-to-pipeline transcendental. Two common cases skip it for free: (a) dt_secs == 0 when query_time_ns == last_ns (factor is exactly 1.0), and (b) candidate_gen.rs:95 already calls current_score(0, now_ns, 0.0) with lambda=0.0, where -lambda*dt is 0 and exp(0)==1.0 -- a guaranteed-wasted exp() in the O(N) candidate-generation scan over the whole ledger.
- fix: Branch out the no-op cases before the transcendental: if dt_secs == 0.0 || lambda == 0.0 return stored.max(0.0) directly (the decay factor is exactly 1.0). That is a single predictable branch that removes a guaranteed-redundant exp() on the candidate-gen scan and on any read taken at the stored timestamp. Only reach for a bounded fast-exp approximation if a flamegraph shows exp() still dominant AFTER this guard AND @tidal-engineer signs off on the error bound with a property test (the decay tolerance is the gate).
- bench: tidal/benches/signals.rs bench_decay_score_read (single read) and bench_200_entity_scoring_pass; tidal/benches/scale.rs for the candidate-gen O(N) scan at 1M cells. — est: Low-to-medium: removes one exp() per read on the zero-dt / zero-lambda cases (notably the whole candidate-gen scan). The general-case exp() win needs measurement; the guard itself is unconditionally correct and cheap.
- @
-
[MEDIUM/medium] Warm-tier windowed read walks up to ~227 AtomicU32 loads across ~14 cache lines with modular indexing
- @
tidal/src/signals/warm.rs:206-234 (windowed_count), :645-653 (sum_last_n_buckets), :592-598 (sum_current_hour)— blast: Medium - per-candidate when a profile/sort uses SignalAgg::Value or Velocity over a 24h/7d/30d window (read_agg helpers.rs:83,86). OneHour and AllTime are cheap (60 loads / 1 load); the wide windows are the cost. - evidence: A SevenDays read does sum_current_hour (up to 60 minute buckets) + sum_last_n_hours(167) = up to ~227 AtomicU32 Relaxed loads (the module's own table at warm.rs:14-16 says <=227). sum_last_n_buckets (warm.rs:645-653) walks backward with idx=(current+len-i)%len -- a modulo and a non-monotone index sequence per element, defeating the hardware prefetcher even though the underlying array is contiguous. BucketedCounter is ~1071 bytes (~17 cache lines, measured from the field sizes: 60+168+31 AtomicU32 + the pointers/counters) and is NOT cache-line aligned, so the hour-bucket ring (672 bytes) alone spans ~11 lines that a 7d/30d read touches. This is the warm read cost the seed flagged.
- fix: Two mechanical options, measure first: (1) Replace the modular backward walk with a split into two contiguous forward slices ([start..len] then [0..end]) summed in index order, so the prefetcher sees monotone addresses and the compiler can auto-vectorize the u32->u64 widening sum (4-8 lanes). (2) Maintain a running per-tier rolling sum (updated on increment and on rotation) so 24h/7d/30d become an O(1) read of a precomputed total minus the out-of-window remainder, instead of summing the ring every read -- this trades a few atomics on the write path for collapsing the read from ~227 loads to a handful. Do (1) first (pure read-path, low risk); only do (2) if the windowed read shows up hot at scale.
- bench: Add a warm-tier windowed-count bench under tidal/benches/signals.rs (currently only decay reads are benched) covering OneHour vs SevenDays vs ThirtyDays; validate at scale via tidal/benches/scale.rs. — est: Medium for wide-window profiles; needs a windowed-count bench (which does not exist yet) to confirm magnitude. The contiguous-slice rewrite is a safe always-on win; the rolling-sum is a larger change to justify only by a profile.
- @
-
[LOW/low] DashMap shard count is hardcoded to 16 regardless of core count
- @
tidal/src/signals/ledger/core.rs:52 (DashMap::with_shard_amount(16)); same pattern at tidal/src/cohort/ledger.rs:45,65— blast: Low-to-medium and contention-dependent - per-write and per-candidate-read shard-lock acquisition; only bites under genuine multi-core concurrent load, invisible single-threaded. - evidence: entries is built with a fixed 16 shards. DashMap's default is (4 * num_cpus).next_power_of_two(). On a serving box with >4 physical cores the default would give 32/64/128 shards; 16 caps the number of independent shard RwLocks. Under concurrent write ingestion + read scoring, two threads whose keys hash to the same shard serialize on that shard's RwLock even though they touch different entities. With 16 shards on a 16+ vCPU host the birthday-collision probability that two concurrent ops share a shard is non-trivial. This is the seed's 'shard count vs core count' point -- verified as a fixed 16.
- fix: Either let DashMap pick its core-scaled default (DashMap::new() / with_capacity), or set shard_amount from std:🧵:available_parallelism().next_power_of_two() with a sane floor (e.g. max(16)). Keep it a power of two (DashMap requires it). This is a one-line change but MUST be proven under concurrent load, not single-threaded micro-bench, before claiming a win -- single-thread it is a no-op.
- bench: tidal-stress (open-loop, coordinated-omission-corrected) under concurrent write+read on a multi-core box -- the only honest measure of shard contention; tidal/benches/scale.rs does not exercise cross-thread shard collisions. — est: Low and entirely contention-dependent: zero single-threaded, potentially meaningful tail-latency improvement under high concurrent ingest+query. Do not claim a number without a multi-core tidal-stress run.
- @
-
[LOW/medium] on_signal re-reads last_update_ns with Acquire inside every CAS-loop iteration
- @
tidal/src/signals/hot.rs:159-181 (the per-lambda CAS loop, timestamp load at :164)— blast: Low - per-write (signal ingestion). Single-writer in cluster mode, so contention is rare; this is a constant-factor on the write path, not a contention problem. - evidence: For each of up to MAX_DECAY_RATES (3) lambdas, the inner loop re-loads self.last_update_ns with Acquire (hot.rs:164) AND self.decay_scores[i] with Acquire (hot.rs:165) on every attempt, then CAS-weak with AcqRel/Acquire. The doc (hot.rs:135-146) justifies re-reading the timestamp for retry consistency, which is correct. On the UNCONTENDED common path (single-writer signal ingestion -- the leader is the only writer per the replication model) the loop runs once, so this is 2 Acquire loads + 1 AcqRel CAS per lambda = up to 9 fenced atomic ops per write even with zero contention. On weakly-ordered targets (ARM) each Acquire is a real barrier.
- fix: Only relevant if signal_write shows up hot AND the single-writer invariant holds: the entry-time timestamp snapshot (entry_last_ns, already loaded at hot.rs:152) is sufficient for the first attempt; the per-iteration timestamp re-read is only needed on a CAS FAILURE. Restructure so the loop reads the timestamp once up front and re-reads it only after a losing compare_exchange_weak, saving one Acquire load per lambda on the (dominant) uncontended single-pass. This preserves the documented retry-consistency invariant -- co-sign with @tidal-engineer since he owns the CAS correctness. Do NOT weaken any ordering; this only removes a redundant load on the success path.
- bench: tidal/benches/signals.rs bench_single_signal_write (target <100ns). — est: Low: saves ~1 Acquire load per lambda on the uncontended write. Worth it only if signal_write_single is near its budget; measure before touching, and only with @tidal-engineer's sign-off on the CAS invariant.
- @
Text / BM25 search path (tidal/src/text/**) — 78/100
Canonical fast pattern: The BM25 read hot path is already close to canonical and the strongest part of the module. retrieve_bm25 (pipeline.rs:343-363) uses the cached IndexReader via idx.searcher() (an arc_swap load + Arc bump — Tantivy's intended per-query call, not a reader re-open), and it collects with BoundedScoresCollector (collectors.rs:132-276), a per-segment bounded min-heap that caps peak memory at O(cap) inside Tantivy regardless of corpus match count, with a select_nth_unstable_by partial sort in merge_fruits (collectors.rs:238) instead of a full sort. The AllScoresCollector is correctly retained only for callers that need the full match set and is explicitly documented as not-for-the-hot-path. That is the good pattern: reuse the cached reader, bound the collector to top-K, partial-sort the merge. The deviations below are the spots that have NOT been pulled onto that pattern — per-query parser/preprocess allocation, and the write-path Mutex + deep metadata clone.
Deviations: n/a
-
[HIGH/high] Per-write std::sync::Mutex lock to clone an immutable crossbeam Sender
- @
tidal/src/db/items.rs:526-530 (text_tx), declared tidal/src/db/mod.rs:161 and 164 (creator_text_tx)— blast: Medium — per-write, on the durable item/creator ingest path (write_item_with_metadata). Executes once per indexed entity; under bulk load (load/) and sustained ingest this is every write. Not on the query path. - evidence: Every durable item write does
self.text_tx.lock().ok().and_then(|g| g.as_ref().cloned())to obtain the outbox Sender, thentry_send.text_txisMutex<Option<crossbeam::channel::Sender<PendingWrite>>>(mod.rs:161). The Sender is set once at open (mod.rs:1046) and never mutated afterward, yet astd::sync::Mutexis acquired and released on EVERY write purely to read-and-clone it. crossbeamSenderisSync + Clone; the lock guards nothing that changes on the hot path. Mechanical cost: an uncontendedMutex::lockis an atomic CAS + release store, but under concurrent ingest from multiple writer threads this Mutex serializes the lock/unlock of every write through one cache line — false-shared cache-coherence traffic and a serialization funnel on a path that is otherwise lock-free. Creator writes hit the same pattern on a second Mutex. - fix: Replace
Mutex<Option<Sender>>witharc_swap::ArcSwapOption<Sender>(or, if the Sender truly never changes after open, storeOption<Sender>directly behind the existing Arc and read it lock-free). The Sender is immutable post-open; a load-acquire of an ArcSwap (or a plain field read) replaces the lock entirely. Keep the existing try_send + drop-on-full behavior unchanged. Confirm the swap-on-reconfigure paths (open/close) with @tidal-engineer since they currently rely on taking the Mutex. - bench: tidal/benches/signals.rs / a write-path bench; add a multi-threaded ingest bench under tidal/benches/ (none currently drives concurrent writers through text_tx). search.rs only measures reads. Measure writes/s with N concurrent writer threads before/after. — est: Low-to-medium single-threaded (one uncontended lock removed per write), but materially higher under concurrent ingest where the Mutex serializes writers — needs the multi-threaded ingest bench to quantify; expect the win to scale with writer-thread count.
- @
-
[MEDIUM/high] Deep clone of the entire metadata HashMap per indexed write to feed the syncer
- @
tidal/src/db/items.rs:532-536 (metadata: stored_metadata.clone()); consumed in tidal/src/text/syncer.rs:33-40 (PendingWrite) and applied at writer.rs:71-75— blast: Medium — per-write, on the durable ingest path. Allocation count scales with metadata-map size, not with the number of indexed text fields. - evidence:
PendingWrite { metadata: stored_metadata.clone(), .. }deep-clones the wholeHashMap<String, String>on every write to hand it to the background syncer over the channel. That is one HashMap allocation plus a heap String allocation for every key and every value, per write. The syncer then iteratesself.fields.text_fields(writer.rs:71) and only reads the keys that are declared text fields — so any metadata key that is NOT a text field is cloned (key String + value String allocated) and then ignored. For an item with many metadata keys but few text fields this is pure allocation waste on the write hot path. - fix: Project the metadata down to only the declared text-field keys BEFORE building PendingWrite — clone just the (key, value) pairs the index will actually consume (a SmallVec<[(Arc, String); N]> or a pre-sized HashMap with only text keys). Alternatively make
PendingWrite.metadatacarryArc<HashMap>so the channel send is an Arc bump and the syncer reads through the Arc without a deep copy — viable because the syncer only reads. Pick the projection if most metadata keys are non-text; pick the Arc if most are text. Coordinate the PendingWrite shape change with @tidal-engineer (it is a public-ish struct). - bench: No existing bench isolates this; add a write-path microbench in tidal/benches/ that writes items with a wide metadata map and few text fields, counting allocations (or wall time) per write. signals.rs is the closest existing harness. — est: Medium for wide-metadata workloads (eliminates N string allocs per write where N = non-text keys); near-zero for items whose metadata is entirely text fields. Needs the wide-metadata bench to confirm magnitude.
- @
-
[MEDIUM/medium] Per-query QueryParser construction allocates default_fields Vec every search
- @
tidal/src/query/search/executor/pipeline.rs:343 (idx.query_parser()) -> tidal/src/text/query.rs:38-49 (TextQueryParser::new)— blast: Medium — per-query, every db.search() with query_text. One small Vec heap allocation per query (Field is a u32 newtype, so the Vec is small, but it is still a per-query alloc + the parser construction churn). - evidence:
retrieve_bm25callsidx.query_parser()on every search, which runsTextQueryParser::new(query.rs:38). That filtersfields.text_fieldsand.collect()s a freshVec<Field>of default search fields (query.rs:39-44), then builds a TantivyQueryParser. Verified against tantivy 0.22 source:QueryParser::for_indexis cheap (two Arc bumps — Schema is Arc, TokenizerManager is Arc<RwLock<..>>; the two FxHashMaps start empty/unallocated). So the only real per-query allocation here is thedefault_fieldsVec — which is fully determined at schema time and is identical for every query against this index. It is rebuilt per query for nothing. - fix: Precompute the
default_fields: Vec<Field>once at TextIndex open and store it on TantivyFields (it is already an Arc), then construct the QueryParser from the cached slice — or cache a fully-builtQueryParserbehind the index sincefor_indexonly needs the Arc'd schema + tokenizers that also never change. The parser is immutable afterset_conjunction_by_default; one shared instance can serve all queries (QueryParser is Sync). If a shared parser is awkward because parse() takes &self and is already cheap, at minimum hoist the default_fields Vec out of the per-query path. - bench: tidal/benches/search.rs (search_text_10k / search_keyword_10k) — already measures end-to-end db.search(); a flamegraph under that bench will show the query_parser() frame. Confirm the Vec alloc is non-trivial relative to the Tantivy search before changing. — est: Low — one small Vec alloc removed per query; only worth doing if a flamegraph under search.rs shows query_parser()/preprocess on the profile. Do not micro-optimize unless it moves the search_text_10k p99.
- @
-
[LOW/medium] preprocess_query allocates a fresh String for every query even when no hashtag is present
- @
tidal/src/text/query.rs:60-86 (parse -> preprocess_query)— blast: Low — per-query, every db.search() with query_text. One String alloc + O(len) copy per query, query strings are short. - evidence:
parse(query.rs:60) unconditionally callspreprocess_query, which always allocatesString::with_capacity(query.len())(query.rs:76) and rebuilds the string char-by-char even when the query contains no#and the output is byte-identical to the input. The hashtag strip is rare; the common case pays a full String allocation + copy for a no-op transformation. - fix: Return
Cow<str>: scan for a#followed by an alphanumeric; if none is found, returnCow::Borrowed(query)and pass it straight toparse_querywith zero allocation. Only allocate the owned String on the rare hashtag-present branch.QueryParser::parse_querytakes &str, so a Cow borrows cleanly. - bench: tidal/benches/search.rs — same harness as the parser finding; the two together are the per-query allocation cleanup. Low priority unless the flamegraph shows it. — est: Low — one short String alloc avoided per hashtag-free query; bundle with the QueryParser caching fix rather than shipping alone.
- @
-
[LOW/high] Syncer commit cadence (commit_every_secs default 2s) caps read freshness, not throughput
- @
tidal/src/text/index.rs:155-156 (default commit_every_n_docs=1000, commit_every_secs=2); tidal/src/text/syncer.rs:212-234 (batch + time-based commit) and reader reload via ReloadPolicy::OnCommitWithDelay (index.rs:242)— blast: Low — per-commit (amortized over up to 1000 writes or 2s). Does not affect per-query CPU or per-write CPU; affects only how soon a write becomes visible to search. - evidence: The syncer commits after 1000 buffered docs OR 2 seconds (whichever first), and the reader uses ReloadPolicy::OnCommitWithDelay (index.rs:242), so a freshly written item is not searchable until (a) the syncer commits and (b) the reader's reload delay elapses. This is a correct group-commit / fsync-amortization design (single Tantivy writer lock held for the syncer lifetime, batched commits — exactly the throughput-friendly pattern). The cost is read-after-write latency for text search: up to ~2s + reload delay of staleness. This is a freshness/SLO knob, not a hot-path inefficiency, and the durability story is covered (rebuild-from-store at open). Flagged only so the staleness window is a conscious, measured choice rather than an accident.
- fix: No code change recommended without a measured requirement. If a workload needs tighter read-after-write, expose/lower commit_every_secs and document the fsync-rate tradeoff (more frequent commits = more frequent fsync + segment creation = more merge pressure). Do NOT lower it blindly: smaller batches raise write amplification and segment count, fighting the LogMergePolicy tuning (MERGE_MIN_NUM_SEGMENTS=4). Validate any change against segment_count() staying < 20 and against an open-loop write+search latency test in tidal-stress.
- bench: tidal/benches/tantivy_merge.rs (merge/segment behavior under commit cadence) plus an open-loop read-after-write staleness measurement in tidal-stress; search.rs forces a reload so it does not exercise the staleness window. — est: None as a CPU win — this is a freshness/durability tuning knob. The current cadence is a reasonable group-commit default; changing it trades fsync rate against staleness, so it must be driven by a stated SLO, not by profiling.
- @
WAL write path & group commit — 74/100
Canonical fast pattern: The group-commit core is genuinely good and is the canonical fast pattern in this category: a single dedicated writer thread (single-writer principle) drains a crossbeam channel into one batch, encodes it once, issues ONE fsync per batch (flush_batch -> sync_segment_observed, writer.rs:222-300), and the staged-append API (append_record_staged, mod.rs:311) lets many concurrent callers coalesce into one fsync instead of each paying a solo batch_timeout+fsync. The ship feed shares the already-encoded batch via Arc<Vec<u8>> so durable bytes are never recopied for replication until the wire payload is assembled (feed.rs:47, writer.rs:262/273). fsync is correctly F_FULLFSYNC-backed on macOS (mod.rs:79). The deviations below are all amortized-per-fsync costs (small, since fsync dominates) plus one genuinely redundant per-event BLAKE3 and one measurement gap.
Deviations: n/a
-
[MEDIUM/high] Per-event BLAKE3 hashed twice on the commit path (partition_dedup then dedup.record)
- @
tidal/src/wal/writer.rs:547 and :697 (record loop) via dedup.rs:117— blast: Medium — per-event on every committed signal write (the steady-state ingest path), batch_size events per fsync. Not per-query. - evidence: For every KEPT event,
partition_dedupcomputesformat::event_content_hash(&event)(writer.rs:547) — which callsevent.to_bytes()to build a 32-byte image, thenblake3::hashover it. On the success path the steady-state loop then callsdedup.record(event)for each kept event (writer.rs:696-698), andrecord(dedup.rs:113-118) computesevent_content_hashAGAIN — a secondto_bytes()+ second full BLAKE3 over the identical bytes. So each durably-committed event is BLAKE3-hashed twice and serialized to a 32-byte stack image twice. BLAKE3 of 32 bytes is cheap (~tens of ns) but it is pure redundant CPU on the per-event commit path; at batch_size=100 that is 100 wasted hashes per fsync. The cost is mechanical: one extra hash compression + one extra 32-byte memcpy/serialize per kept event. - fix: Hash once in
partition_dedupand thread the already-computedu128hash alongside each kept event into the success-path record step, sodedup.recordinserts the precomputed hash directly instead of re-hashing. Add aDedupWindow::record_hash(u128)that skipsevent_content_hash. Sinceto_bytes()is also recomputed a third time insideencode_batch_with_shard(batch.rs:454-455), consider computing the 32-byte image once per event and reusing it for hash + encode, though the encode reuse is a larger refactor. - bench: No existing bench covers this (signals.rs uses NoopWalWriter; see the measurement-gap finding). Add a
wal_append_throughputcriterion bench drivingWalHandle::append_record_stagedto measure committed-events/s; this finding would show as a small CPU delta dwarfed by fsync unless fsync is stubbed. — est: Low absolute (one BLAKE3 + one serialize per event); only visible when fsync is amortized across a large batch or stubbed. Needs the new bench to confirm it moves committed-events/s at all — likely <1% end-to-end because fsync dominates.
- @
-
[MEDIUM/high] per-flush HashSet allocation in partition_dedup (batch_seen) and per-flush kept_events/kept_replies Vecs
- @
tidal/src/wal/writer.rs:537-544 (partition_dedup)— blast: Medium — per-fsync (per group commit), so it scales with flush frequency, not per-event. At high batch_size it is amortized; at low batch_size (1-2 events, the latency-sensitive case) the fixed allocator cost is a larger fraction of the commit. - evidence:
partition_dedupallocates THREE fresh heap structures on every single flush:let mut kept_events: Vec::new()(writer.rs:537),let mut kept_replies: Vec::new()(538), andlet mut batch_seen: HashSet<u128> = HashSet::new()(544). All three start at capacity 0 and grow by reallocation as the batch fills (thebatchVec itself IS reused across iterations viabatch.drain(..), writer.rs:687 — but the partition output is not). The HashSet in particular allocates a fresh table per flush and is thrown away immediately after. At batch_size=100 that is a HashSet that grows through several rehash/realloc steps every fsync, plus two Vecs growing to ~100 elements each. This is per-fsync allocator traffic and avoidable rehashing. - fix: Hoist
kept_events,kept_replies, andbatch_seento writer-thread-local scratch buffers owned byrun_writer, passed intopartition_dedupby&mut, andclear()ed (not reallocated) at the start of each call.Vec::clearandHashSet::clearretain capacity, so after warmup there is zero per-flush allocation. Pre-size them tomax_batch. This mirrors the existing intentionalbatchreuse viadrain(..)(writer.rs:685-687) — extend the same discipline to the partition outputs. - bench: Add
wal_append_throughput(signals.rs-adjacent) with a small-batch (batch_size=1-4) variant to expose the fixed per-flush allocation; measure allocations via a counting allocator or the criterion wall-time delta on the small-batch case. — est: Low-to-moderate per fsync; most visible on small batches where allocator/rehash cost is not amortized across many events. Needs measurement — likely a few hundred ns saved per flush, meaningful only at high flush rates with small batches.
- @
-
[MEDIUM/high] encode_batch builds the whole batch via extend_from_slice into one Vec, then encode_frame allocates a SECOND full-size Vec and copies the payload again
- @
tidal/src/wal/format/batch.rs:453-465 (encode_batch_with_shard) and :904-922 (encode_frame)— blast: Medium — per-fsync. The copy is O(batch bytes); for blob batches (embeddings up to 8 MiB, batch.rs:473) the double-allocate-and-copy is over the full blob, executed once per blob write (per-write for items/embeddings, throttled to fsync rate by flush_pending_blobs). - evidence:
encode_batch_with_shardallocatespayload = Vec::with_capacity(event_count * 32)(batch.rs:453 — correctly pre-sized) and fills it withpayload.extend_from_slice(&event.to_bytes())per event (455). It then callsencode_frame, which allocates a SECOND bufferbuf = vec and doesbuf[HEADER_SIZE..].copy_from_slice(payload)(922) — a full memcpy of the entire payload from the first Vec into the second. So every batch is materialized twice: once intopayload, once intobuf, with a full copy between them. For a 100-event signal batch that is a 3.2 KiB redundant copy + a redundant allocation per fsync. Thevec![0u8; total_len]also zero-initializes the whole buffer before overwriting nearly all of it (only bytes 28-31 reserved stay zero), wasting a memset over header+payload. - fix: Have
encode_framewrite the header directly into the front of the caller's payload buffer instead of allocating a second buffer: pre-reserveHEADER_SIZEbytes at the front ofpayload(or buildpayloadasVec::with_capacity(HEADER_SIZE + n*32), push 64 placeholder header bytes first, then the events), and fill the header in place — eliminating the second allocation and the payload memcpy entirely. BLAKE3 is computed overheader[0..32] || payloadwhich is then contiguous. Also replacevec![0u8; total_len]withVec::with_capacity+extend/resizeonly where needed, orunsafe-freeVec::with_capacity+ write, to drop the redundant zero-init memset (note crate is unsafe-forbid, so use safe in-place writes). - bench: Add a
wal_encode_batchmicro-bench in benches (criterion) over encode_batch_with_shard for batch sizes {1, 100} and awal_encode_embeddingover a 1536-dim vector to capture the blob double-copy. This is directly measurable without I/O. — est: Moderate for large/blob batches (eliminates one full-payload alloc+memcpy+memset per encode); small for tiny signal batches. Confirm with the encode micro-bench — the blob path (8 MiB ceiling) is where the saved copy is largest.
- @
-
[HIGH/high] No criterion bench measures the real WAL append / group-commit / fsync write path — the category is optimized blind
- @
tidal/benches/signals.rs:29 (NoopWalWriter) and tidal/benches/recovery.rs (read-side only)— blast: High — this is the measurement gate for the entire WAL write category. Without it, no group-commit tuning (batch_size, batch_timeout) or the encode/dedup fixes above can be proven, and a regression in committed-events/s or append p99 ships silently. - evidence: Grep across tidal/benches/*.rs finds NO bench that drives
WalHandle::append,append_record_staged, group-commit batching, or fsync on the write path.signals.rsconstructs the ledger withBox::new(NoopWalWriter)(recovery.rs:8/signals.rs setup) so the signal-write bench (signal_write_single, signals.rs:38) explicitly EXCLUDES the WAL.recovery.rsmeasures cold-start replay and WAL-backlog recovery (the READ/replay side), never the steady-state append+fsync. Per the project's own discipline ('an optimization without a before-and-after number is a guess'), every finding above is currently unfalsifiable: there is no baseline for committed-events/s, p99 append latency, or fsync amortization vs batch_size. The m11p1 throughput floor (~90/s replicated) and the SyncObserver fsync-timing hook (config.rs:29) exist precisely because fsync cost is 'the load-bearing unknown' — yet there is no criterion harness to track it or to validate that any encode/dedup change actually moved the number. - fix: Add a
wal.rs(orwal_append.rs) criterion suite: (1)wal_append_throughputdriving N concurrentappend_record_staged+wait, reporting committed-events/s, parameterized over batch_size {1,10,100} and batch_timeout, on a tmpfs-or-real-disk dir so fsync is real; (2) a fsync-amortization curve (events-per-fsync vs latency) using the SyncObserver hook to attribute fsync wall time; (3)wal_encode_batch/wal_encode_embeddingpure-CPU micro-benches (no I/O) so the encode/dedup CPU findings above are measurable in isolation. Report p50/p99/p999 of append latency, not the mean (open-loop intended-send timing where the harness drives a fixed rate). - bench: This finding IS the bench to add: a new
tidal/benches/wal.rssuite. It backs the est_gain/measurement claims of every other finding in this category. — est: No direct latency gain — this is the instrument, not the optimization. Its value is making the other three findings provable and guarding committed-events/s and append p99 against regression. Highest-leverage item in the category.
- @
-
[LOW/high] SegmentWriter writes through a raw File with one write_all syscall per batch (no user-space buffering) — acceptable, flagged for completeness
- @
tidal/src/wal/segment.rs:283 (OpenOptions...open, no BufWriter) and :329-334 (write_batch_bytes)— blast: Low — per-fsync (one write syscall per batch). Already amortized by group commit. - evidence:
SegmentWriterholds a bareFile(segment.rs:247-254), opened withOpenOptions::new().create(true).append(true).open(&path)(283) — noBufWriter.write_batch_bytesissuesself.file.write_all(bytes)(331) directly, so each batch is ONEwrite(2)syscall followed by the fsync insync(). Because group-commit already coalesces many events into one batch = one write + one fsync, the per-event syscall count is already amortized and aBufWriterwould add no value here (it would just add a copy and need flushing before fsync). This is the CORRECT design — flagged only to record that the absence of buffering is intentional and not a defect, and to note the singlewrite_allis the right granularity given the batch is already assembled in one contiguous buffer. - fix: No change. Do NOT add a BufWriter — it would interpose a redundant copy between the encoded batch and the file and complicate the fsync ordering. If the encode-frame fix (writing the header in-place into one contiguous buffer) lands, the single
write_allof that buffer remains exactly one syscall, which is optimal. - bench: Covered by the proposed
wal_append_throughputbench (syscall count is implicit in committed-events/s vs batch_size); no separate bench needed. — est: None — confirming the current design is correct. Listed to prevent a future 'add buffering' change that would regress by adding a copy.
- @
Replication shipping funnel & lock hold — 78/100
Canonical fast pattern: The subsystem already embodies most of the canonical fast pattern for a single-leader replication funnel, and it should be measured against itself: (1) shipping is decoupled from the request path — the staged-write split (relay.rs:18-31) holds the seqno lock only across the bump+stage+log-push (microseconds) and pushes the fsync wait + cross-region ship off the request path into per-peer windowed sender threads (ship.rs sender_loop); (2) batching/coalescing is free — the group-commit batch IS the ship batch (feed.rs push), and the receiver re-coalesces a drained backlog through ONE shared follower group-commit fsync (receiver.rs:197-217, MAX_COALESCED_SEGMENTS/EVENTS) instead of one solo fsync per segment; (3) the commit-index recompute is allocation-free and sub-linear — compute_commit uses a reused scratch buffer + select_nth_unstable_by, never a per-fold sort (commit.rs:484-505); (4) quorum waits are condvar-parked with an async watch-channel bridge, never thread-per-wait (commit.rs:507-547); (5) the rate limiter is a lock-free CAS token bucket (tenant.rs:126); (6) the retained ship tail holds Arc<Vec> so retention/eviction never copies (feed.rs:47). The deviations below are the residual copies and the one global mutex that every RPC funnels through — the places the otherwise-clean pipeline still serializes or allocates per-RPC.
Deviations: n/a
-
[MEDIUM/high] Per-RPC full clone of the run's payload bytes that is never reused after the send
- @
tidal/src/replication/ship.rs:1056 (range_payload_with_term(..., run.bytes.clone(), ...))— blast: Medium — executes per-RPC (per shipped run) on every active leader sender thread; scales with replication byte throughput and window*peers. Not per-event (runs coalesce), so the constant is per-batch, not per-signal. - evidence: sender_loop builds the wire payload with run.bytes.clone() — a full heap copy of the entire collected run (up to max_batch_bytes = 16 MiB). After the send, run is consumed only by record_success(&run) / record_failure(&run, &e), both of which read run.first/run.last/run.event_count but NOT run.bytes (record_failure stores only first->last in the retry BTreeMap; the retry re-collects fresh bytes from the source at ship.rs:852). The loop then drops run and re-enters claim_and_collect. So the clone is pure waste: the bytes could be MOVED into the payload (run.bytes by value), with the seqno span retained separately for the retry path. Cost: one memcpy of the whole run + one heap alloc + one free, per ship RPC, per window thread, per peer. At window=4 and N peers this is 4N copies of every replicated byte on the steady-state hot path.
- fix: Restructure ClaimedRun / record_failure so the bytes are not needed after the send: move run.bytes into range_payload_with_term by value (drop the .clone()). record_failure already only needs (first, last) for the retry BTreeMap and re-collects bytes on retry, so destructure run into (first, last, event_count) + bytes before the send and pass owned bytes to the payload. If the borrow checker fights the success/failure branches, split ClaimedRun into a Copy header {first,last,event_count} kept on the stack plus an owned Vec consumed by the payload constructor.
- bench: No replication/ship bench exists today (only tidal/benches/recovery.rs touches WAL). Add tidal/benches/ship.rs: drive a WalFeedSource through ShipQueue with an in-process/no-op Transport at a fixed batch size, measure ns/run and allocations (with a counting allocator) before/after; confirm end-to-end with tidal-stress sustained-replicated-writes/s and p99 replication lag (open-loop, coordinated-omission-corrected). — est: Removes one full-payload memcpy + alloc/free per RPC. For signal-sized runs (tens of 32-byte events) the absolute bytes are small but the alloc/free pair and the cache traffic are real; for blob-heavy runs (embeddings, up to 16 MiB) this is a large copy eliminated outright. Needs the new bench to quantify; expect a clear allocation-count drop and a measurable ns/run win on blob runs.
- @
-
[MEDIUM/high] feed.collect allocates a fresh un-sized Vec and copies every batch under the tail mutex
- @
tidal/src/wal/feed.rs:273 (let mut bytes = Vec::new()) and the copy loop 276-287, all under the tail Mutex taken at 225— blast: Medium — per-RPC on the leader's sender threads; the lock-hold contends the single writer thread that feeds ALL replication. Hold time grows with run size (up to max_batch_bytes). - evidence: collect() is the production ship source's per-RPC work unit (WalFeedSource::collect -> WalShipFeed::collect, called from claim_and_collect at ship.rs:893/852). It (a) allocates bytes = Vec::new() with NO with_capacity, so it reallocs repeatedly as extend_from_slice grows it across the run; (b) copies every retained batch's bytes (batch.bytes is Arc<Vec>) into that new Vec via extend_from_slice — defeating the zero-copy Arc retention for the duration of the collect; and (c) holds the tail Mutex across the entire scan+copy loop (the guard spans 225..294, flagged with allow(significant_drop_tightening)). The writer thread's push() (feed.rs:164) also takes that same tail Mutex post-fsync, so a large collect copy serializes against the group-commit writer's hand-off.
- fix: Two independent improvements: (1) size the buffer — the loop can pre-sum the contributing batches' lengths (the tail is already walked once to find the start index) and Vec::with_capacity to kill the realloc chain; (2) shrink the lock hold — collect the set of contributing Arc<Vec> handles (cheap Arc clones) under the tail lock, release the lock, then do the byte concatenation outside it. Better still, carry the run as a small Vec<Arc<Vec>> and let the transport write them as a gathered/vectored payload (or concatenate once at the convert boundary), so the leader never materializes a second contiguous copy at all when the wire layer can take a slice list.
- bench: Add a feed-collect micro-bench in the same new tidal/benches/ship.rs (push N batches, time collect over varying run sizes, count allocs). Validate the lock-hold reduction under tidal-stress by watching sustained replicated-writes/s and writer-thread stall — the writer's push latency under concurrent collect is the observable. — est: with_capacity removes log2(run_bytes) reallocs per collect (clear win, measurable in alloc count). The Arc-handle-then-copy-outside-lock change cuts the tail-mutex hold from O(run_bytes copy) to O(batch count pointer walk), reducing contention with the writer — quantify via the writer push p99 under load.
- @
-
[MEDIUM/medium] Every successful ship folds through one global CommitIndex mutex plus a global peer_applied mutex (the commit funnel)
- @
tidal/src/replication/ship.rs:971-976 (peer_applied_hint then commit.update_peer) -> tidal-net/src/transport.rs:945-952 (peer_applied Mutex) and tidal/src/replication/commit.rs:404-410 (CommitInner Mutex)— blast: Medium — per successful RPC, contended by window*peers sender threads. Hold times are nanoseconds, so this bites only at high ship-RPC rates (large clusters / high write throughput); at today's small RF it is mild. - evidence: record_success runs on every successful send_segment. It (a) calls transport.peer_applied_hint(peer), which locks the GrpcTransport.peer_applied std::Mutex (transport.rs:945), and (b) calls commit.update_peer(peer, reported), which locks the single CommitInner Mutex (commit.rs:404 -> fold_locked at 443, which on an advance also recomputes compute_commit and notify_all). With window>1 sender threads per peer and N peers, ALL of them contend these two single global mutexes once per RPC. update_peer/fold_locked is short and compute_commit is allocation-free, so this is contention/cache-line bouncing on the mutex word, not algorithmic cost — but it is the one point the whole multi-peer, multi-window funnel serializes through on the success path. record_failure (ship.rs:1001) similarly takes the per-peer state mutex; the per-peer split there is fine, the GLOBAL commit mutex is the shared one.
- fix: Keep the commit index a mutex (its fold is genuinely shared state and short), but cut the two per-RPC lock acquisitions to the minimum: (1) skip peer_applied_hint's lock entirely when its value cannot have advanced — it is mirrored, so a per-peer AtomicU64 mirror (like acked_atomic) read with Acquire avoids the HashMap mutex on the common no-change path; (2) skip commit.update_peer when reported <= the peer's last-folded mark (cache the last folded value per peer in an AtomicU64 and compare before taking the CommitInner lock — fold_locked already early-returns on durable <= mark, but only AFTER taking the lock). Both turn the steady-state success path into two relaxed atomic loads with no global lock when nothing advanced.
- bench: Add a contended-fold micro-bench (M threads hammering update_peer with monotonic and with stale marks) to measure lock-acquire cost vs the atomic-pre-check fast path. End-to-end: tidal-stress at higher RF / window to expose the contention as a knee in sustained replicated-writes/s. — est: At small RF the win is negligible (uncontended mutex is ~20ns); the value is removing a serialization knee that would appear at larger window*peers. Needs a contended bench to confirm it matters before changing — classic measure-before-optimize: do not pre-emptively shard the commit index.
- @
-
[LOW/high] Legacy poll-based shipper re-reads segment files from disk and clones bytes per peer (dead-ish path, but a footgun if rewired)
- @
tidal/src/replication/shipper.rs:205-410 (spawn_shipper loop; std::fs::read at 300, segment_bytes.clone() at 329)— blast: Low — not on the active ship path today (node.rs:776 uses ShipQueue). Per-poll, per-peer if ever activated. Flagging as a latent funnel, not a live hotspot. - evidence: shipper.rs is the m11p1 poll-loop shipper: it wakes every poll_interval, lists segments, std::fs::read()s each sealed segment from disk (300), and clones the cached segment bytes once per peer per segment (329). The production cluster node wires ShipQueue::spawn (the m11p2 feed path) at tidal-server/src/cluster/node.rs:776, not spawn_shipper — so this is not on the live serving funnel. But it is still pub-exported (mod.rs:58) and structurally a polling, disk-re-reading, per-peer-cloning funnel: if a future change rewires it, it reintroduces a poll-latency floor (poll_interval) and a per-peer byte clone the feed path was built to avoid.
- fix: If shipper.rs is genuinely superseded by the WalFeedSource/ShipQueue path everywhere except tests, gate it behind a feature or mark it clearly deprecated so it cannot silently become the hot path again; if it is still used by any deployment mode, give the per-poll bytes_cache entries an Arc<Vec> value so the per-peer clone (329) becomes an Arc clone instead of a full byte copy. Do not invest in poll-interval tuning — the event-driven feed path is the right pattern; converge on it.
- bench: Covered indirectly by tidal/benches/recovery.rs (segment read-back) for the fs::read cost; no dedicated bench warranted unless it is confirmed live. Verify liveness first by grepping the server wiring (already done: node.rs uses ShipQueue). — est: Zero on the live path (not wired). Value is risk-removal: prevents a future regression back to a polling, disk-re-reading, byte-cloning funnel. Confirm it is dead before spending effort.
- @
Scatter-gather fan-out & threading model — 52/100
Canonical fast pattern: The codebase already contains the canonical fast pattern, immediately adjacent in offload.rs: ClusterWritePool (offload.rs:130-266) is a fixed-size, pre-spawned, reused OS-thread pool draining a bounded crossbeam MPMC channel, with backpressure->429 on saturation. That is exactly the Disruptor/thread-pool discipline the read fan-out lacks: threads are created once at startup and reused, the queue is bounded, and load is shed cleanly instead of by spawning. The read path (dispatch_shards) should be expressed against the same kind of reusable executor (a pool, a rayon scope, or — since the work is already offloaded to a blocking thread and is IO/blocking — a small persistent pool keyed by shard), not by std:🧵:spawn-per-shard-per-query gated on a single global Mutex+Condvar.
Deviations: n/a
-
[HIGH/high] OS-thread-per-shard spawned per query on every /sharded read (no pool, no reuse)
- @
tidal-server/src/scatter_gather.rs:510-549 (spawn loop), invoked per-query from cluster/routes.rs:738-747 and :847-856— blast: High — per-query on every /sharded/feed and /sharded/search; multiplies by N=shard count; the cost is paid on the read serving path that the SLO (RETRIEVE <50ms) governs. - evidence: dispatch_shards does
std::thread::Builder::new().name(...).spawn(move || ...)once per live shard, every query. Thread creation is ~10-30us of syscalls (clone/mmap) plus a default ~2MB stack reservation, and a matching teardown cost, all on the request critical path. For an N-shard topology that is N thread create+join-equivalent cycles per RETRIEVE/SEARCH. Worse, this runs inside a tokio blocking-pool thread (routes.rs offload_cluster_read -> offload.rs:54 spawn_blocking), so the model is 'a blocking thread spawns N more blocking threads', and that blocking-pool thread then sits parked on rx.recv_timeout (scatter_gather.rs:564) for the whole budget doing nothing. The identical work on the write path was already converted away from per-request spawn to a reused pool (offload.rs:18-24 explicitly calls out 'The previous design spawned a fresh OS thread per request — unbounded growth on the hottest cluster path'); the read fan-out still has that exact anti-pattern. - fix: Replace per-shard std:🧵:spawn with a persistent, reused executor: either a fixed pool of runtime-free OS threads (the existing ClusterWritePool shape) sized to MAX_SHARD_WORKERS and fed a bounded queue, or — because each query already owns a spawn_blocking thread — drive the fan-out from a small per-process rayon/threadpool scope so threads are recycled across queries instead of created and destroyed per shard per query. Keep the detached-deadline semantics by submitting jobs to the pool and draining results with recv_timeout; a job that the pool cannot start within budget is reported degraded exactly as today. This removes N thread create/teardown syscalls and N 2MB stack reservations per query.
- bench: No server-side criterion bench exists today (tidal/benches/* cover only the engine crate; there is no scatter/sharded bench). Add a tidal-server bench (or a tidal/benches/scatter.rs against SimulatedCluster) that drives scatter_gather_retrieve over a 4- and 16-shard SimulatedCluster and reports per-query wall time and allocations; confirm end-to-end with tidal-stress open-loop against /sharded/feed reporting p50/p99/p999, not a mean. — est: Removing N thread spawn/teardown cycles + N stack reservations per query should cut fixed per-query overhead by tens of microseconds per shard and sharply reduce tail latency under a query storm (where spawn-storm + scheduler pressure dominate). Needs the new bench + tidal-stress p99 to quantify; the mechanical win (no syscalls, no stack churn, no allocator pressure) is certain, the magnitude is workload-dependent.
- @
-
[HIGH/medium] Single process-global Mutex+Condvar semaphore serializes every shard worker of every concurrent query
- @
tidal-server/src/scatter_gather.rs:175-233 (ShardWorkerSemaphore), 249-261 (process-global OnceLock), acquired at :527— blast: High under load, Low under light load — per shard worker (requests x shards), only contended when many sharded reads fan out concurrently; on a quiet node the lock is uncontended. - evidence: SHARD_WORKER_SEMAPHORE is one process-wide ShardWorkerSemaphore { permits: Mutex, available: Condvar }. Every shard worker thread of every in-flight sharded query calls acquire_timeout, which takes the same Mutex and may wait/notify on the same Condvar. The acquire loop holds the lock across the count check only, but under a fan-out storm (requests x shards workers all launching at once) this single lock+condvar is the one cache line every worker thread CAS/locks against — classic single-point contention on the coherence bus, and every release does notify_one which wakes exactly one waiter and re-contends the lock (:221-232). This is a counting semaphore implemented as a hot global lock, the opposite of the lock-free/atomic-permit discipline the engine uses elsewhere.
- fix: Make the permit count an AtomicUsize (or AtomicIsize) and acquire via a relaxed CAS loop with a bounded backoff/park, eliminating the Mutex on the fast path; keep a Condvar/park only for the genuinely-blocked waiter path. Or, better, fold the bound into the reusable pool from the previous finding — a bounded job queue is the concurrency cap, so the separate global semaphore disappears entirely. Document the ordering: a permit acquire needs Acquire on success / the release needs Release so the query's writes to the result channel are visible — today the Mutex provides that fence; an atomic version must replicate it (Mara Bos: AcqRel on the successful CAS).
- bench: Same scatter bench as above but parameterized to launch K concurrent queries x N shards so the semaphore is contended; measure p99 acquire latency and total fan-out wall time vs an atomic-permit variant. perf stat for the cache-miss/lock spike on the permits line. — est: Under a fan-out storm, replacing one global Mutex+Condvar with an atomic permit (or a bounded queue) removes the cross-core serialization point; expect a meaningful p99/p999 improvement under concurrency, negligible change single-threaded. Needs the contended bench to confirm — mark medium because the win only materializes under the storm the cap exists to handle.
- @
-
[LOW/high] Per-shard heap allocation just to name each spawned thread
- @
tidal-server/src/scatter_gather.rs:515 (format!("scatter-shard-{}", shard.0))— blast: Low — per shard per query, small fixed allocation; dwarfed by the spawn cost itself but on the same hot loop. - evidence: Inside the per-shard spawn loop, every iteration does format!(...) -> a heap-allocated String purely to set the thread name, executed per shard per query. shard_name(...) at :380-385 also does a format!/clone fallback per degraded/timed-out shard. These are small but they are inside the fan-out loop and add allocator traffic exactly where the goal is to minimize per-query work.
- fix: This allocation disappears entirely with the pool conversion (finding 1) — a reused worker is named once at pool construction, not per job. If the per-query spawn is kept short-term, drop the dynamic thread name (the diagnostic value is marginal versus the per-query alloc) or build it only when tracing is enabled. Not worth a standalone change; fold it into the pool refactor.
- bench: Allocation count delta in the scatter bench (dhat or a counting allocator) before/after the pool conversion; should show N fewer String allocs per query. — est: Negligible on its own (a few small allocs per query); only worth noting because it vanishes for free with finding 1. Do not chase it independently.
- @
-
[LOW/high] Per-query Arc::new(query.clone()) clones a large Retrieve/Search struct off, but on, the request path
- @
tidal-server/src/scatter_gather.rs:919 and :1026 (SimulatedCluster path), :1369 and :1472 (HTTP path)— blast: Low — once per query (not per shard); the Arc clones per shard are cheap refcount bumps, which is correct. - evidence: Each scatter entry point does
let shared_query = Arc::new(query.clone()). Retrieve (tidal/src/query/retrieve/types.rs:104-136) carries Vec, Vec exclude, Option context, Option cohort_predicate, Option, etc.; Search (search/types.rs:55-87) carries Option<Vec> query_vector (an embedding — potentially hundreds of floats), Vec, Vec. The clone deep-copies all of these. It is ONCE per query (the Arc is then cheaply cloned per shard at :512-513), so it is off the per-shard hot loop — but it is a non-trivial heap copy on the serving path, and for SEARCH with a query_vector it copies the whole embedding. - fix: The caller already owns the query by value in the handler (sharded_feed builds
retrievelocally at routes.rs:727 then passes&retrieve). Thread the owned value through into scatter_gather_* by value and Arc::new it without the clone, eliminating one deep copy per query. For SEARCH specifically, ensure the query_vector embedding is moved, not copied. Low priority — it is one allocation per query, not per candidate or per shard, so it is firmly secondary to findings 1 and 2. - bench: Allocation/byte-copied delta in the scatter bench for a SEARCH query carrying a realistic query_vector; confirm the embedding is moved not copied. — est: Small — one fewer deep struct clone per query; meaningful only for SEARCH-with-embedding. Do not prioritize over the thread-model findings; a clone off the per-candidate/per-shard hot path is barely a finding.
- @
Cluster node per-request path & shard-map locking — 72/100
Canonical fast pattern: The write hot path is already the right shape: ClusterNode.groups is an immutable BTreeMap<ShardId, Arc> read lock-free (node.rs:3566 route_entity does a BTreeMap::get + Arc::clone, no lock); the signal write stages on a bounded write pool then completes on the blocking pool with follower shipping fully off-path (node.rs:5148-5184); and the team already learned the lesson once — they replaced a per-request std::thread spawn on the write/heal path with a fixed-size ClusterWritePool (state.rs:53-58). The canonical fast pattern here is therefore: resolve the owning replica through one lock-free immutable-map lookup, do per-request work on a pre-sized reused pool, and never allocate routing state per request. The deviations below are the places that did NOT inherit that discipline — chiefly the cross-process scatter-gather read path, which still spawns one OS thread per shard per query and rebuilds/clones routing state on every request.
Deviations: n/a
-
[HIGH/high] Cross-process scatter-gather spawns one OS thread per shard per query
- @
tidal-server/src/scatter_gather.rs:514 (std::thread::Builder::spawn inside the per-live-shard loop at 510-549)— blast: High — executes per-query on every /sharded/feed and /sharded/search, once per live shard. The thread-spawn cost scales linearly with shard count and request rate; under a query storm at S>1 it is the dominant non-IO cost before the blocking HTTP fetch even starts, and it competes with the engine's own worker threads for the scheduler. - evidence: For every /sharded/feed and /sharded/search request, the coordinator loops over live_shards and calls std:🧵:Builder::new().name(format!("scatter-shard-{}", shard.0)).spawn(...) for each shard (scatter_gather.rs:510-538). Each spawn is a clone() syscall + a fresh 2MB-default thread stack reservation + a scheduler enqueue, and each thread is joined implicitly via the sync_channel before the response returns. The format! on line 515 also heap-allocates a thread name per shard per query. The mechanical cost is N thread creations + N teardowns on the per-query critical path (N = live shard count), ~10-30µs of pure spawn overhead each plus stack-page faulting, none of which is the actual query work. The module comment at 164-168 explicitly acknowledges 'each sharded request still fans out one detached thread per live shard' — the semaphore caps the AGGREGATE in-flight workers but does NOT remove the per-query spawn. This is the same anti-pattern the write path already retired (state.rs:53-58 notes the old per-request std::thread spawn was 'unbounded growth on the hottest cluster path' and was replaced with a pool); the read scatter never got that treatment.
- fix: Replace the per-query std::thread spawn with a pre-sized, reused worker pool exactly as the write path did (the ClusterWritePool precedent in state.rs:53-58 and offload.rs). Submit one closure per shard to a fixed Rayon-style pool or a dedicated scatter pool whose worker count is the existing MAX_SHARD_WORKERS bound, and collect over the same sync_channel. The semaphore (shard_worker_semaphore) then becomes the pool's queue/capacity rather than a gate layered on top of fresh threads. Hoist the format! thread-name out — a pooled worker does not need a per-query name. This removes N spawn/teardown syscalls and N stack reservations per query, turning fan-out into N cheap channel sends onto already-running threads.
- bench: No existing bench covers this (tidal/benches/ has no cluster/scatter suite; grep confirmed). Measure end-to-end with tidal-stress open-loop against an S>1 cluster, reporting p50/p99/p999 of /sharded/feed under increasing shard count — the spawn cost shows up as a per-query fixed tax that grows with N. A focused criterion micro-bench 'scatter_fanout/N_shards' wrapping just the coordinator with a no-op query_one would isolate the spawn-vs-pool delta from the HTTP fetch. — est: Removes ~N×(10-30µs) of spawn/join overhead per query plus stack-fault and scheduler-contention costs; for S=8 that is ~80-240µs of pure overhead off every sharded read's tail. Needs tidal-stress measurement to confirm the absolute p99 win, but the mechanical saving (N syscalls + N stack reservations → N channel sends) is certain.
- @
-
[MEDIUM/high] /sharded/* write rebuilds a ShardRouter and allocates a region-id Vec on every write
- @
tidal-server/src/cluster/node.rs:5988-5989 (sharded_write_route calls sharded_region_ids then entity_shard); sharded_region_ids at node.rs:5949-5953; entity_shard at tidal-server/src/scatter_gather.rs:293-304— blast: Medium — per-write on the /sharded/* ingest path. A Vec alloc + sort + a ShardRouter construction per write is small in absolute terms versus the WAL fsync that follows, but it is pure waste on the hottest cluster ingest surface and it touches the allocator on every write, adding allocator-lock contention under concurrent ingest. - evidence: Every /sharded/items, /sharded/embeddings, /sharded/signals write calls sharded_region_ids(state) (node.rs:5988), which iterates state.id_to_name.keys().copied().collect() into a Vec and sorts it (node.rs:5950-5951) — a heap allocation + sort per write. It then calls entity_shard(EntityId, &shards) (node.rs:5989), which on every call does u16::try_from(shards.len()) and ShardRouter::hash(num_shards) — constructing a fresh ShardRouter from scratch (scatter_gather.rs:298-300) purely to call .route() once and throw it away. The routing topology (region set and shard count) is fixed for the node's lifetime, so both the sorted region-id Vec and the ShardRouter are recomputed identically on every single write. The same Vec allocation also happens in routes.rs (single-process sharded handlers) via state.shard_ids() at routes.rs:538,570,599, which calls cluster_ref().regions() returning a freshly-allocated Vec (testing/cluster.rs:734).
- fix: Compute the sorted region-id list and the ShardRouter ONCE at node/state construction and store them (the ShardRouter is already const-constructible — ShardRouter::hash is a const fn). Replace the per-write sharded_region_ids + entity_shard pair with a single cached self.router.route(entity) lookup against a precomputed shards slice (mirroring how route_entity at node.rs:3567 already uses the cached self.router for the in-process path). This is a routing-state-hoist, not an algorithm change: the FNV-1a hash + modulo itself (shard.rs:172-194) is already optimal — only the per-call reconstruction around it is the waste.
- bench: No cluster ingest bench exists. A criterion micro-bench 'sharded_route/per_write' timing sharded_region_ids + entity_shard for a fixed region set would show the alloc+construct cost directly; the end-to-end effect is best confirmed via tidal-stress write throughput (signals/s) against an S>1 cluster, p50/p99 of the write ack. — est: Eliminates one Vec allocation + sort + one ShardRouter construction per write. Small per-write (sub-microsecond) but it is per-write allocator traffic on the ingest hot path; the win is reduced allocator contention under concurrent writers more than raw single-thread latency. Measurement via tidal-stress write ramp to confirm contention relief.
- @
-
[MEDIUM/high] Per-query clone of the full region-name HashMap and peer_http map on the scatter read path
- @
tidal-server/src/cluster/routes.rs:736 and 845 (state.id_to_name_map().clone()); tidal-server/src/cluster/node.rs:5957 (sharded_region_names clones id_to_name) and node.rs:5969 (http_shard_context clones peer_http)— blast: Medium — per-query on every sharded read. The clone is O(regions) allocations per request; cheap at small region counts but it is allocator traffic on the read hot path, and it grows with cluster size. The reqwest::blocking::Client clone (node.rs:5970) is a cheap Arc bump, so that one is fine. - evidence: sharded_feed (routes.rs:736) and sharded_search (routes.rs:845) each do state.id_to_name_map().clone() per request, deep-cloning a HashMap<RegionId, String> (every region name String reallocated) so it can be moved into the offloaded closure. http_shard_context (node.rs:5961-5974) clones state.peer_http (HashMap<RegionId, String>) and the blocking reqwest client per sharded read, and sharded_region_names (node.rs:5957) clones id_to_name again. These maps are immutable for the node's lifetime, so each clone is a per-query heap allocation of the whole map plus a String allocation per region entry, done only because the data is passed by value into a 'static worker closure.
- fix: Wrap the immutable region-name and peer_http maps in Arc<HashMap<...>> once at construction and clone the Arc (a refcount bump) into the worker closure instead of deep-cloning the map. The scatter context already lives behind Arc (node.rs:5964); push the same Arc discipline down to the maps so no per-query map/String allocation occurs. Pass &Arc and Arc::clone rather than .clone() on the owned HashMap.
- bench: Covered only end-to-end. tidal-stress /sharded/feed throughput at increasing region count will show the O(regions) per-query alloc as a slope; a criterion 'scatter_ctx/build' micro-bench timing http_shard_context + id_to_name_map().clone() isolates it. — est: Turns O(regions) String+HashMap allocations per query into one Arc refcount bump. Sub-microsecond at small clusters, but removes per-query allocator pressure that scales with cluster size — measure with tidal-stress at the target region count to confirm the tail improvement.
- @
-
[LOW/medium] Redundant leader RwLock read-lock acquisitions per signal write
- @
tidal-server/src/cluster/node.rs:1090 is_leader() -> current_leader() -> read_recovered(&self.leader) at node.rs:1084-1085; called at node.rs:5141, 5145, and again inside stage_signal_local at node.rs:1382— blast: Low — per-write, but three times per write. The absolute cost is small (uncontended RwLock read is a handful of ns) and is dwarfed by the WAL fsync; the concern is the repeated atomic on one shared cache line under high write concurrency, not single-thread latency. - evidence: is_leader() (node.rs:1090) reads the std::sync::RwLock<Option> self.leader via read_recovered (node.rs:1084-1085, 2903-2914) and compares to self.region. On a single /signals write it is invoked at least three times: the route handler's non-leader-forward check (node.rs:5141), the immediate not-leader reject (node.rs:5145), and again inside the staged write job's stage_signal_local (node.rs:1382). Each call takes and releases the RwLock read lock. std::sync::RwLock read acquisition is an atomic RMW (and on contention a futex), and the guarded payload is just a Copy u16 Option. Three lock round-trips to read one u16 per write is redundant atomic traffic on the write hot path; under concurrent writers the read-lock atomic on a single shared word is a cache-line ping-pong point.
- fix: Read the leadership view ONCE at the top of the write handler (one current_leader() call, capturing the Option) and thread the boolean/RegionId through, rather than re-acquiring inside stage_signal_local. Better still, since the value is a single u16+tag, store the leader as an AtomicU32 (region id + a 'no leader' sentinel) and read it Relaxed/Acquire — a leadership read does not need a full RwLock, only a single atomic load, removing the lock entirely from the read side (writes to leadership are rare, on election only). Document the ordering: a write-path leader check needs Acquire to observe a just-installed leader; the rare election write uses Release. Take any ordering change to @tidal-engineer since the leader field interacts with election/commit invariants.
- bench: No micro-bench isolates this. tidal-stress concurrent write ramp (many writers, one leader) would surface the shared-cache-line contention as a throughput ceiling; a criterion 'leader_check/contended' spawning N threads hammering is_leader() would measure the atomic ping-pong directly. — est: Negligible single-thread (a few ns × 2 saved calls), but under heavy concurrent ingest collapsing three RwLock reads to one atomic load removes a shared-cache-line contention point. Needs a contended benchmark to confirm it actually moves write throughput rather than being noise behind the fsync.
- @
-
[LOW/medium] BTreeMap::get for shard routing on the in-process write path (pointer-chasing vs a slot array)
- @
tidal-server/src/cluster/node.rs:3568 (self.groups.get(&shard)) and node.rs:3355-3357 (placement/groups are BTreeMap<ShardId,...>)— blast: Low — per-write/per-read routing lookup, but only meaningful at S>1; at S=1 (the shipped default) the BTreeMap holds one entry and the lookup is trivial. The cost is a few pointer-chase cache misses at large S, not a lock or allocation. - evidence: route_entity (node.rs:3566-3575) maps an entity to a shard via the cheap FNV-1a router (shard.rs:172) then does self.groups.get(&shard) on a BTreeMap<ShardId, Arc> (node.rs:3568). BTreeMap::get is a logarithmic walk down heap-allocated, pointer-linked nodes — each level is a potential cache miss to a separately-allocated node. ShardId is a dense u16 in [0, S) (the code asserts dense ids at node.rs:3490-3499), so a BTreeMap is the wrong container for an O(1) dense-key lookup. For S=1 (today's default) this is a one-entry tree and effectively free; the cost only matters at larger S. forward_candidates (node.rs:3584) and replica_for similarly BTreeMap::get.
- fix: Since route()'s output ids are guaranteed dense in [0, S) (asserted at node.rs:3494-3499), index a Vec<Option<Arc>> (or a boxed slice) by shard.0 as usize instead of BTreeMap::get — an O(1), single-cache-line, branch-predictable array index with no tree walk. Keep the BTreeMap only for the rare ordered-iteration paths (status rows) if needed, or sort on demand. This is a layout change, not a semantics change.
- bench: No bench covers routing-lookup latency. A criterion 'route_entity/lookup' over varying S (1, 8, 64) comparing BTreeMap::get vs Vec-index would quantify the cache-miss difference; immaterial at S=1, so only worth it once S>1 is a real deployment shape. — est: Effectively zero at S=1 (current default). At large S, replaces a logarithmic pointer-chase (each level a possible cache miss) with one array index — single-digit-ns per lookup saved per write/read. Premature until S>1 deployments exist; flagged for completeness, not as an urgent win.
- @
Network transport serialization & framing (tidal-net) — 78/100
Canonical fast pattern: The fast pattern for a WAL-shipping wire path is: (1) prost bytes = "bytes" so a bytes proto field decodes as a zero-copy prost::bytes::Bytes slice of tonic's Bytes-backed receive buffer (refcount bump, no memcpy, no per-segment heap allocation); (2) carry Bytes end-to-end so the domain WalSegmentPayload.bytes is a refcounted view rather than an owned Vec<u8>; (3) clone cheap connection handles out from under a brief read guard and never hold the peer-map lock across an await; (4) amortize the sync/async block_on bridge over a COALESCED batch rather than paying it per segment. tidal-net already does (3) correctly (PeerPool::handle_for releases the RwLock read guard before the RPC await, client.rs:108-116, 235-247) and (4) correctly on the receive side (recv_segment parks once via block_on, then try_recv_segment drains the backlog with no runtime hop, transport.rs:1069-1106; receiver.rs:189-216 coalesces). It does NOT do (1)/(2): every bytes field is bytes = "vec" → owned Vec<u8>, forcing an allocation + memcpy on every decode.
Deviations: n/a
-
[MEDIUM/high] prost
bytes = "vec"forces a heap-alloc + memcpy of the whole WAL payload on every inbound segment decode- @
tidal-net/build.rs:2 (defaulttonic_build::compile_protos, no.bytes(&["."])); proto field tidal-net/proto/wal_shipping.proto:14 (bytes payload = 2); generated type confirmed at target/.../out/tidal.replication.v1.rs:17-18 (#[prost(bytes = "vec")] pub payload: Vec); decode consumed at tidal-net/src/server.rs:220 and the catch-up pull at tidal-net/src/transport.rs:263— blast: Medium — executes once per inbound replicated segment on every follower: theship_segmenthandler (server.rs:220) per live ship, and theStreamSegmentscatch-up pull (transport.rs:263) per chunk during follower catch-up. Frequency is the cluster's segment rate (thousands/s under the m11 sharded path), not the µs serving path, so the allocator/memcpy cost is a constant tax on replication throughput and a GC-of-Vecs pressure point, not a query-latency item. Magnitude scales with segment SIZE: negligible at 1 KB, dominant at multi-MiB coalesced segments. - evidence: Because the proto
bytesfields compile toVec<u8>(notprost::bytes::Bytes), prost'sMessage::decodecannot slice the payload out of tonic's already-Bytes-backed h2 receive buffer; it mustcopy_to_bytesinto a freshly-allocated, exactly-sizedVec<u8>for EACHShipSegmentRequest. That is one heap allocation plus onememcpyof the entire segment body on the receive path of every replicated segment. With the default 16 MiB (and up to 64 MiB) segment ceiling configured in server.rs:902-911, a full segment is a 16-64 MiB memcpy + allocation that the existingByteswire buffer makes entirely avoidable. The convert.rstry_from(convert.rs:48-54) and the server handler (server.rs:220) then MOVE thatVecinto the domainWalSegmentPayload(no further copy), so the single decode copy is the whole cost — but it is pure waste relative to aBytesslice. The outbound encode copy is NOT avoidable with prost (encode always serializes into the output buffer), so this finding is the receive/decode side only. - fix: Add
.bytes(&["."])(or scope it to the two hot fields.tidal.replication.v1.ShipSegmentRequest.payloadand.SnapshotFileChunk.data) to thetonic_build/prost_buildconfig in build.rs so those fields generate asprost::bytes::Bytes. Then changeWalSegmentPayload.bytes(tidal/src/replication/transport.rs:19) fromVec<u8>tobytes::Bytesand threadBytesthrough convert.rs and the server handler — theFrom/TryFromimpls become zero-copy moves of a refcounted slice. The WAL read-back side that buildsSegmentChunk.bytes(sources.rs:26) can also becomeBytesover the read buffer. This removes the per-segment allocation + memcpy on decode entirely. Coordinate theWalSegmentPayloadtype change with @tidal-engineer (it touches the replication apply path and the in-process transport which currently moves aVec). - bench: tidal-net/benches/transport_throughput.rs already measures end-to-end
send_segmentforgrpc_localhost— extend it to SWEEPSEGMENT_SIZE(1 KB / 64 KB / 1 MiB / 16 MiB) so the decode copy is visible above localhost syscall noise; the win will appear only at the larger sizes. Report segments/s and (ideally) per-op allocation count before/after. — est: At 1 KB segments: ~0 (the copy is noise vs the h2/syscall round-trip — do NOT claim a win there). At 1-16 MiB coalesced segments: removes one alloc + one full-payload memcpy per segment on the receive path; plausibly a measurable single-digit-to-low-double-digit % of decode-bound receive throughput, but it MUST be confirmed by the size-swept bench above before any claim — this is a hypothesis until the criterion delta exists.
- @
-
[LOW/high] Per-ship-ack global
Mutex<HashMap>fold for peer-applied hints- @
tidal-net/src/server.rs:29-41 (fold_peer_applied), called from the ship hot path at tidal-net/src/transport.rs:930 after everysend_to; the symmetriclast_reported/report_failingmutexes at transport.rs:965-968— blast: Low — per-RPC (per ship ack), but the hold is a HashMap op with no syscall/await inside, and per-peer acks are serialized by the ShipQueue worker model (tidal/src/replication/ship.rs), so real contention requires many peers acking in the same instant. At the cluster's thousands-of-RPCs/s rate this is not a funnel. - evidence: After each successful
send_to,send_segmentcallsfold_peer_applied, which takes a single process-wideArc<Mutex<HashMap<ShardId,u64>>>(server.rs:26) to do a HashMap entry lookup + monotonic max. The lock is held for a sub-microsecond, await-free critical section, so it is correct and cheap in the common case (one ShipQueue thread per peer serializes that peer's acks). It is a global lock shared across ALL peers, so it can contend only when many peers' ship threads ack concurrently — and even then the contention is on a nanosecond-scale critical section, dwarfed by the gRPC round-trip that precedes it. - fix: Not worth changing now — it is not on the serving query path and the network round-trip dominates. IF a future flamegraph under a wide fan-out (many peers) shows lock contention here, replace the single
Mutex<HashMap>with a sharded-by-ShardIdmap or a per-peerAtomicU64updated with afetch_max/CAS loop (the value is a monotonic max, which maps cleanly onto a lock-freefetch_max). Document this as a deferred option, not a fix. - bench: Would need a new tidal-net bench that fans
send_toout across N peers concurrently and counts lock-wait; the existing transport_throughput.rs is single-peer so it cannot surface this. Do not add the bench unless a profile flags the lock. — est: ~0 at current scale; this is explicitly a premature-optimization NOTE, not a recommended change. Listed only to record that it was verified and consciously left alone.
- @
-
[LOW/high] Snapshot file-chunk path allocates a fresh 1 MiB
Vecper chunk (off the serving hot path — by design)- @
tidal-net/src/server.rs:96-114 (read_file_rangedoesvec![0u8; max]per call withmax = SNAPSHOT_FILE_CHUNK_BYTES = 1 MiB, server.rs:90), driven by theFetchSnapshotloop at server.rs:578-620; per-chunkpath.clone()/rel_path.clone()at server.rs:583, 618— blast: Low — per-snapshot-chunk during a rare reseed/join, never per query, never per live ship. Bounded count (file_size / 1 MiB), bandwidth-bound by disk + network regardless of the allocation. - evidence: Each 1 MiB snapshot chunk read allocates a fresh zeroed
vec![0u8; 1 MiB]and clones the file path String into the proto. This is a bulk reseed/joiner-catch-up transfer (m11p5), not the live serving or live-ship path: it runs once when a node joins or reseeds, off the reactor onspawn_blocking, and the per-chunk allocation + path clone is completely amortized by the disk read and network send it bookends. Thevec![0u8; max]also pays a needless zero-fill (the bytes are immediately overwritten byread), but again at snapshot cadence this is irrelevant. - fix: Leave as-is for correctness clarity. IF snapshot transfer time ever becomes an operational concern, reuse one
1 MiBscratch buffer across chunks of the same file (allocate outside the per-chunk loop) and read into the reused buffer to drop the zero-fill — but this is not a serving-path win and should not be prioritized over the decode-copy finding. - bench: No criterion bench exists or is warranted for the snapshot path; it would be validated by a tier-3 reseed-throughput stress run, not a microbench. Do not add a microbench. — est: Negligible for the serving SLOs; recorded as verified-and-intentionally-left. Not a recommended change.
- @
Storage engine mechanical sympathy (fjall) — 62/100
Canonical fast pattern: The good pattern in this module is already visible and should be the template for every finding: (1) byte-lexicographic key encoding with a fixed 8-byte BE entity_id prefix so all of an entity's rows sort contiguously and a prefix scan is one sequential range walk (keys.rs:186-203, entity_prefix returns a stack [u8;9] / entity_tag_prefix a [u8;10] — zero heap); (2) streaming scan_prefix that owns the snapshot nonce and yields owned pairs lazily instead of collecting the keyspace into RAM (fjall.rs:150-166); (3) fsync amortization done right in flush_all — rotate all three memtables with the non-fsyncing rotate_only, then ONE persist(SyncAll) covering the shared journal instead of three (fjall.rs:346-353); (4) atomic multi-op batching through a single OwnedWriteBatch under one seqno + checksum envelope (fjall.rs:181-197). The deviations below are all places that fail to follow one of these four already-established patterns — chiefly that the fjall Database/Keyspace is opened with zero tuning (default block cache, default bloom, default compaction), which the existing in-module discipline never reaches down to configure.
Deviations: n/a
-
[HIGH/medium] fjall opened with zero tuning — default block cache, no explicit bloom-filter policy, default compaction
- @
tidal/src/storage/fjall.rs:289 (Database::builder(path).open()) and :315 (db.keyspace(name, fjall::KeyspaceCreateOptions::default))— blast: High — per-query AND per-candidate. RETRIEVE fans the metadata get over the full candidate set (up to ~200), so one query issues up to ~200 point gets; the cache/bloom config governs how many of those touch disk. This is the dominant storage cost on the serving path. - evidence: The whole storage backend is opened with builder defaults and KeyspaceCreateOptions::default — there is no block-cache sizing, no shared BlockCache across the three keyspaces, no explicit bloom-filter bits-per-key, and no compaction strategy override. On the query read path every candidate's metadata is fetched with a point get (encode_key(eid, Tag::Meta, b"") + storage.get) — see mod.rs:348-355, helpers.rs:357-358, pipeline.rs:668/756. A point get that misses the (default-sized, possibly tiny) block cache walks every LSM level; without a tuned bloom filter each level that does not contain the key still pays a block read + decompress before the bloom rules it out. At 1M+ items with a cold cache this is the difference between an L3/page-cache hit and several SSD reads PER CANDIDATE. The mechanical cost is per-candidate disk reads + per-block decompress that a right-sized block cache and a bloom-bits policy would elide.
- fix: Build a single shared fjall BlockCache sized to the deployment (a few hundred MB by default, configurable) on the Database builder so all three keyspaces share one cache budget instead of three uncoordinated defaults, and set an explicit bloom-filter bits-per-key on the keyspace create options (10 bits ~= 1% FPR is the standard point-lookup sweet spot). Make compaction strategy explicit (leveled is the right default for this read-heavy point-get workload). Thread the cache size through the open() signature / a StorageConfig so it is tunable at deploy time. All three knobs are fjall 3.x KeyspaceCreateOptions / builder APIs.
- bench: tidal/benches/storage.rs::bench_random_get (fjall_10k) measures it today but only at 10k with a warm cache; extend it to a cold-cache, 1M-row variant (drop the cache between setup and the measured loop) so the bloom/cache win is visible, and confirm end-to-end via benches/scale.rs and benches/query.rs RETRIEVE p99. — est: Potentially large on cold/large datasets (point-get latency can drop from multiple SSD reads to one cache hit), negligible on the current warm 10k bench — must be measured at 1M+ with a cold cache before claiming a number. The win is real in principle (this is textbook LSM tuning); the magnitude needs the cold-cache scale bench to confirm.
- @
-
[MEDIUM/medium] Per-candidate metadata point-get loop is not a batched multi-get and re-encodes the key each iteration
- @
tidal/src/query/executor/mod.rs:346-356 (candidates.iter().filter_map -> encode_key + storage.get per candidate), mirrored at tidal/src/query/search/executor/pipeline.rs:668-672 and :756-757— blast: Medium-High — per-candidate, per-query. Up to ~200 independent LSM point gets + ~200 key allocs + ~200 value-copy allocs per RETRIEVE that needs metadata (session context, alphabetical/duration sort, or creator-grouping/notification caps). - evidence: The metadata pre-load loops over every candidate, calling encode_key(eid, Tag::Meta, b"") (a fresh 10-byte Vec heap allocation per candidate — keys.rs:186-193, Vec::with_capacity(10)) and then storage.get(&key), which itself does value.to_vec() (fjall.rs:132) copying the value out of fjall's internal slice into a fresh Vec. So per candidate: one key Vec alloc + one independent LSM descent (each acquiring/releasing fjall's internal snapshot machinery) + one value-copy alloc. The candidate EntityIds are NOT sorted before the loop, so successive gets jump around the keyspace, defeating the block-cache/page-cache locality that the contiguous BE-key layout was designed to provide.
- fix: Pre-sort the candidate EntityIds ascending before the metadata loop so the point gets walk the keyspace in key order — this turns ~200 random descents into a near-sequential scan that stays in the block cache and the OS page cache (the BE-key layout already guarantees order == numeric id). Better still, since all the keys share the entity-id ordering and the same Tag::Meta, replace the N point gets with a single bounded prefix/range scan when the candidate set is dense, or reuse one scratch key buffer (encode into a reused Vec via a slice-writing encode_key_into to kill the per-candidate key alloc). The value-copy is forced by the StorageEngine::get -> Vec trait signature; a get_with(key, |&[u8]|) closure variant on the trait would let metadata deserialization read fjall's slice in place and skip the copy entirely on this path.
- bench: benches/query.rs (RETRIEVE end-to-end) and benches/scale.rs at 1M; add a microbench to benches/storage.rs that does N point gets with shuffled vs sorted key order to isolate the locality win from the alloc win. — est: Sorting for locality is a cheap, likely 1.1-1.5x win on the metadata stage at scale (cache-locality, no API change); the alloc + value-copy elimination is smaller per-op but multiplied by ~200/query. Needs the query/scale bench to confirm it moves end-to-end p99, not just the micro.
- @
-
[MEDIUM/medium] StorageEngine::get forces a value-copy (Vec) out of fjall on every point read
- @
tidal/src/storage/fjall.rs:127-133 (get -> .map(|value| value.to_vec())) constrained by the trait at tidal/src/storage/engine.rs:17— blast: Medium — per point-get, i.e. per-candidate on the metadata path and per-query on profile/preference/user-meta lookups. One alloc + one memcpy + one free per get. - evidence: fjall::get returns an Arc-backed Slice that already points at decompressed bytes in the block cache; the backend immediately calls .to_vec(), allocating a fresh heap buffer and memcpy-ing the value out, purely to satisfy the trait's Result<Option<Vec>> signature. Every caller on the read path (mod.rs:351, helpers.rs:358, pipeline.rs:670/757, personalization.rs:109) then deserializes from that owned Vec and drops it — the owned allocation lives only for the length of a deserialize call. The copy + alloc + free is pure overhead the cache already did the work to avoid.
- fix: Add a borrowing accessor to the StorageEngine trait — e.g. fn get_with(&self, key, f: impl FnOnce(Option<&[u8]>) -> R) -> Result — and route the hot read callers (metadata deserialize, preference vector decode) through it so they read fjall's slice in place and never materialize a Vec. Keep the existing Vec-returning get for callers that genuinely need ownership. InMemoryBackend implements the closure variant under its read lock trivially. This is the one place the otherwise-good streaming discipline (scan_prefix already avoids materialization) is not applied to point reads.
- bench: benches/storage.rs::bench_random_get — add a get_with variant alongside the existing get bench to measure the alloc/copy delta directly; confirm at the query level via benches/query.rs. — est: Small per-op (one short-lived alloc+memcpy avoided) but on a hot per-candidate path; realistically a few percent on the metadata stage. Worth doing as part of the metadata-loop rework, not on its own. Measure before claiming.
- @
-
[LOW/high] Single-keyspace flush() pays an LSM rotate + full-database SyncAll fsync per call; checkpoint path uses it instead of the amortized flush_all
- @
tidal/src/storage/fjall.rs:200-214 (FjallBackend::flush = rotate_only + persist(SyncAll)) called from tidal/src/signals/checkpoint/mod.rs:166 (storage.flush())— blast: Low — per-checkpoint (periodic, not per-signal-write; the 82ns signal write is the in-memory DashMap ledger, fjall is only touched at checkpoint cadence). One fsync per checkpoint flush. Not on the per-query or per-signal hot path. - evidence: FjallBackend::flush rotates the active memtable then issues persist(SyncAll) — a full fsync of the shared journal — for one keyspace. The signal checkpoint path (checkpoint/mod.rs:165-166) does write_batch then storage.flush() on a single backend. The module already documents (fjall.rs:330-353) that the three backends share ONE journal, so a per-keyspace flush fsyncs the whole shared database; flush_all exists precisely to rotate all three and fsync once. Because the checkpoint writes only Tag::Sig rows it correctly flushes one backend, so this is not a triple-fsync bug today — but the fsync is the single most expensive syscall on the write path (an SSD fsync is ~hundreds of microseconds to low milliseconds), and there is no group-commit/coalescing across concurrent checkpoint or entity-write flushes: each flush is its own fsync barrier.
- fix: Confirm the checkpoint cadence keeps this off the hot path (it does today). If checkpoint frequency or multi-keyspace entity writes ever rise, introduce a group-commit coalescer that batches pending rotations and issues a single persist(SyncAll) per fsync window (the Disruptor-style amortize-the-barrier pattern), and route multi-keyspace checkpoints through flush_all so they pay one fsync, not three. No change needed now beyond a comment pinning the invariant that the periodic checkpoint must not be made synchronous-per-write.
- bench: benches/recovery.rs (checkpoint + flush cadence) and benches/signals.rs to confirm fsync stays off the per-signal path; a fsync-count assertion in the checkpoint test would pin it. — est: None on the current serving path (correctly off-hot-path). The finding is a guardrail: it documents the one real fsync funnel so a future change that moves checkpointing toward per-write does not silently reintroduce a per-write fsync. No measured win to claim.
- @
-
[LOW/high] Checkpoint WriteBatch allocated with no capacity hint, growing by reallocation across the full live signal set
- @
tidal/src/signals/checkpoint/mod.rs:84 (WriteBatch::new()) feeding fjall.rs:169-197 write_batch— blast: Low — per-checkpoint (periodic), amortized O(n) reallocation over n live rows. Not per-query, not per-signal-write. - evidence: The checkpoint builds one WriteBatch::new() (no capacity) and pushes every live signal row plus every stale-key delete plus the meta key into it (checkpoint/mod.rs:142-158). With up to the 5M-entry trimmer cap of live rows, the batch's internal Vec of ops grows by repeated doubling/realloc from zero, copying the accumulated op vector each time it crosses a capacity boundary. The kv_pairs Vec just above (line 90) has the same no-capacity issue and is additionally sorted (line 101). The storage bench's batch_write helper already shows the right pattern — WriteBatch::with_capacity (benches/storage.rs:140,159,178) — which the production checkpoint path does not use.
- fix: Size both Vecs up front: kv_pairs = Vec::with_capacity(self.entries().len()) and WriteBatch::with_capacity(entries_len + estimated_stale + 1). The live entry count is known from the DashMap len before the loop. This removes the log(n) reallocations and their memcpys during checkpoint assembly.
- bench: benches/recovery.rs (checkpoint assembly + write) — measure checkpoint wall time at a large ledger size before/after the capacity hint. — est: Small and off-hot-path — a handful of reallocations saved per periodic checkpoint at large ledger sizes. Trivial, safe, and matches the bench's own pattern; do it, but it will not move serving p99.
- @
Latency measurement honesty & bench fidelity — 68/100
Canonical fast pattern: The honest pattern already exists in this repo and is excellent: tidal-stress/src/scheduler.rs runs OPEN-LOOP at a fixed arrival rate, measures each request from its INTENDED send time (Instant::now().saturating_duration_since(intended), scheduler.rs:86), and SHEDS rather than blocks when the in-flight cap is hit (scheduler.rs:90-92) so a server stall inflates the very percentiles a closed-loop test would hide. tidal-stress/src/metrics.rs records the full distribution (p50/p90/p99/p999 + EXACT max, metrics.rs:74-104), keeps min/max/mean exact and bucket-estimates only the percentiles (~3-4%), and excludes 429/503 fast-rejects from the latency picture (metrics.rs:177-181) so backpressure does not flatter the tail. This is the Gil-Tene/HdrHistogram discipline done right. The canonical fast RECORDING pattern is also present: the engine's LatencyHistogram is lock-free Relaxed atomics (histogram.rs:27-29) with a documented, correct ordering argument. The gap is that the criterion benches under tidal/benches/ and the doc that quotes them do NOT inherit this discipline.
Deviations: n/a
-
[HIGH/high] scale-baselines.md reports criterion MEAN estimates under a "p99" label — the SLO is stated as p99 but never measured as one
- @
docs/profiling/scale-baselines.md:29-31 (sourced from tidal/benches/scale.rs:10-12,142-286)— blast: Medium — not a runtime path, but it is the source of truth every reader (and the SLO table in CODING_GUIDELINES §8) trusts. Executes once per release-validation, gates the 'PASS' claim for the two headline SLOs. - evidence: The acceptance table reads
RETRIEVE p99 < 50ms | 152µs | PASS,Signal write p99 < 100µs | 82ns | PASS. But the underlying numbers are criterion's mean-estimate confidence interval: line 40 isretrieve_1m/for_you time: [151.88µs 152.13µs 152.40µs]and line 66[82.033ns 82.286ns 82.535ns]— that triple is criterion's [lower, point-estimate, upper] of the MEAN of a single-threaded closed-loopb.iter()loop (scale.rs:156-158, 272-283), not a p99. Criterion does not compute or expose a p99; sample_size is 10 (scale.rs:152). A p99 is by definition the worst 1% — the exact thing a mean over a quiescent single-threaded loop discards. Mechanical cost: none at runtime; the cost is epistemic — a tail SLO is being signed off with a central-tendency number. Under contention/GC/compaction the real p99 can be 10-100x the mean and this methodology would never see it. - fix: Either relabel the doc to 'mean (criterion)' and stop calling it p99, OR measure a real p99. Two honest options: (1) make scale.rs use
b.iter_customto time each iteration into a Vec/HdrHistogram and report the p99/p999/max yourself — this gives a real single-threaded distribution; (2) better, drive the p99 SLO from tidal-stress (which already does it correctly, open-loop + CO-corrected) and cite that run in scale-baselines.md. The criterion mean is still useful as a regression tripwire, but it must not be presented as the p99 the SLO names. - bench: tidal/benches/scale.rs (add iter_custom + percentile capture) and/or tidal-stress peach-100k ramp for the authoritative p99 — est: No latency gain — this is measurement honesty. Prevents a silent tail-SLO miss in production; needs the tidal-stress run to quantify the gap between mean (152µs) and real p99.
- @
-
[MEDIUM/high] Every tidal/benches/*.rs is closed-loop single-threaded and reports mean only — coordinated omission for any tail claim
- @
tidal/benches/scale.rs:156-158,272-283; tidal/benches/query.rs:199-201; tidal/benches/signals.rs:46-57,107-121— blast: Medium — per-query / per-write micro-cost is measured correctly; the gap is only when these numbers are quoted as serving-tail SLOs. The benches run per-PR as regression guards (good); the risk is interpreting their mean as the production p99. - evidence: All 15 bench suites use criterion's default
b.iter(|| ...)/iter_batched, which is a closed-loop loop: it issues the next op only after the previous returns, on one thread, with the machine otherwise idle. Verified no bench usesiter_custom,Throughput, or any percentile (grep across tidal/benches/*.rs returns only sample_size and the scale.rs doc-comment 'p99' strings). This is correct and useful for measuring per-op CPU cost in isolation (e.g. signals.rs:46 'signal_write_single' target <100ns), but it structurally cannot observe a tail under load: there is no concurrent writer contending the DashMap shard, no checkpoint thread, no fsync, no queueing delay. The mechanical reality it misses: at the 200-candidate scoring path the real tail comes from DashMap shard-lock collisions and L3 misses under concurrent ingest, none of which a single idle thread reproduces. - fix: Keep the criterion micro-benches as CPU-cost regression tripwires — that is what they are good at. Do NOT extend them into multi-threaded tail measurement; that is tidal-stress's job and it already does it honestly. The concrete change is documentation/process: in scale.rs and the baselines doc, label criterion outputs as 'isolated per-op cost (mean)' and route every p99/p999/tail SLO claim to a cited tidal-stress open-loop run. Optionally add one criterion bench that records its own distribution via iter_custom for the signal-write path so a single-thread p999 exists as a floor.
- bench: tidal-stress (authoritative tail); tidal/benches/* stay as mean regression guards — est: Measurement honesty, not speed. The win is not shipping a tail SLO backed by a mean; magnitude of the hidden tail needs the tidal-stress run to confirm.
- @
-
[MEDIUM/medium] Histogram observe() does up to 13 separate atomic RMWs per signal write — a Relaxed cumulative-suffix loop on an 82ns hot path
- @
tidal/src/db/metrics/histogram.rs:72-81 (called per write at db/signals.rs:234, per query at db/query_ops.rs:148,312)— blast: Medium — per-write and per-query, but ONLY when themetricsfeature is enabled (the field/observe call are#[cfg(feature="metrics")], signals.rs:223). Standalone embedded users without metrics pay nothing. Server deployments pay it on every signal write and every retrieve/search. - evidence: observe() binary-searches for the start bucket (good, O(log n)) then does
for bucket in &self.buckets[start..] { bucket.fetch_add(1, Relaxed) }plus count.fetch_add and sum.fetch_add. For a fast write that lands in bucket 0 (an 82ns write -> 0µs afteras_micros()),start=0 so ALL 11 WRITE_LATENCY_BOUNDS buckets are incremented, + count + sum = 13 atomic read-modify-writes. Each fetch_add is a locked RMW (~tens of cycles even uncontended, and a cache-line ping if two ingest threads share the histogram's bucket lines). The cumulative-bucket representation is inherent to Prometheus, but it makes the COMMON case (fast writes) the WORST case for RMW count, because fast values fall at the low end and increment the entire suffix. On the 82ns measured write path, 13 locked RMWs is plausibly a double-digit-percent tax on the metrics-enabled build. Note: the seed claim of '~60 lock-refs on the recording path' is FALSE — the recording path is lock-free; this is the real mechanical cost. - fix: Switch to a NON-cumulative (delta) bucket layout: observe() increments exactly ONE bucket (the one the value falls in) plus count+sum = 3 RMWs instead of up to 13, and render_prometheus() computes the cumulative prefix-sum once at scrape time (cold path, ~11 adds, runs per Prometheus scrape not per write). This is the standard 'sparse histogram' trick — the cumulative semantics Prometheus needs are reconstructed at read time where cost does not matter. Verify with a before/after on signals.rs:38 'signal_write_single'. Keep Relaxed (the ordering argument at histogram.rs:27-29 is correct and unaffected).
- bench: tidal/benches/signals.rs::bench_single_signal_write (run with --features metrics before/after; today it runs without metrics so the cost is invisible — that is itself a gap) — est: ~13 RMWs -> 3 RMWs on the metrics-enabled write path; needs a
--features metricscriterion run to confirm the absolute µs, but the RMW-count reduction is mechanical and certain. Also closes the gap that the signal bench does NOT enable metrics, so this tax is currently unmeasured.
- @
-
[LOW/medium] Per-op Instant::now() pair straddles the entire signal write / query on the metrics build — an unmeasured timing tax
- @
tidal/src/db/signals.rs:217 + 230 (write); tidal/src/db/query_ops.rs:45+147 (retrieve), 175+311 (search)— blast: Low — per-write/per-query, metrics-feature builds only; the clock read is cheap relative to a retrieve (152µs) but not relative to an 82ns write. - evidence: On the metrics build each signal() captures
Instant::now()before record_signal and.elapsed().as_micros()after (signals.rs:217,230).Instant::now()on macOS/Linux is a VDSO clock read (mach_absolute_time / clock_gettime) — tens of ns, no syscall, but non-trivial relative to an 82ns write: two clock reads could be a meaningful fraction of the measured write itself, and worse, the measured write latency INCLUDES the cost of reading the clock + the 13-RMW observe (Finding 3) — i.e. the metric inflates the very number it reports. This is correctly feature-gated so the no-metrics build is clean, and the timer placement (after backpressure admission, signals.rs:214) is deliberate and right. The issue is only that this overhead is invisible because the signal bench runs WITHOUT the metrics feature. - fix: No code change required to the timing itself (you must time to observe). The concrete action is to (a) run tidal/benches/signals.rs WITH
--features metricsso the clock+observe overhead is included in the baseline and the '82ns' number is honest about whether it includes or excludes instrumentation, and (b) document in scale-baselines.md whether the quoted write latency is the metrics-on or metrics-off path. If observe() is made single-bucket (Finding 3) the self-inflation shrinks to the two clock reads, which is acceptable. - bench: tidal/benches/signals.rs::bench_single_signal_write run twice (--features metrics vs default) to quantify instrumentation overhead — est: Honesty, not speed: clarifies whether the 82ns headline is the instrumented or uninstrumented path. Quantify with the two-run delta.
- @
Wave 1 — measurement floor (DONE 2026-06-13, verified with real runs)
The instrument-first wave is complete. Two trusted before/after instruments now
exist where none did, and the dishonest p99 label is fixed.
tidal/benches/wal.rs (new) — real fsync, real group commit
| bench | result | reads as |
|---|---|---|
wal_append_throughput/writers8_batch1 |
230 events/s | every event its own fsync — coalescing off |
wal_append_throughput/writers8_batch10 |
2.3 Kelem/s | partial coalescing |
wal_append_throughput/writers8_batch100 |
22.1 Kelem/s | full group-commit coalescing (≈100×) |
wal_encode_batch/events1 |
87 ns | per-event BLAKE3 + v3 pack |
wal_encode_batch/events256 |
5.85 µs (≈23 ns/ev) | full-batch encode kernel — Wave-6 baseline |
tidal-server/benches/scatter.rs (new) — spawn-per-shard fan-out
| bench | result | reads as |
|---|---|---|
scatter_fanout/regions4 |
214 µs | 4 thread-spawns + merge over near-instant reads |
scatter_fanout/regions16 |
500 µs | 16 thread-spawns — spawn cost dominates |
scatter_fanout_concurrent/regions4_q8 |
870 µs | 32 workers, semaphore-bounded |
scatter_fanout_concurrent/regions16_q8 |
3.03 ms | 128 workers contending one global semaphore — Wave-5 baseline |
docs/profiling/scale-baselines.md — latency-honesty fix
Relabeled the three p99 rows as isolated per-op cost (mean, closed-loop) and
added a Measurement Contract: criterion time: = mean regression tripwire only;
every p99/p999 tail SLO is signed off by the open-loop tidal-stress ramp.
Intentionally did not add a single-thread percentile capture to scale.rs — a
closed-loop p99 under no concurrency re-introduces the exact coordinated-omission
fallacy the relabel removes; the open-loop tidal-stress run is the cited tail authority.
Remaining: Waves 2–7 (allocation kill → de-clone → storage/transport → threading → WAL/ship CPU → enforcement). Each lands against the benches above as the before/after gate.
Wave 2 — source-level allocation kill (IN PROGRESS 2026-06-13)
T1 — signal_snapshot carrier (DONE, verified)
Replaced ScoredCandidate.signal_snapshot: Vec<(String,f64)> with
SmallVec<[(SignalKey,f64); 4]> where SignalKey = Static(&'static str) | Owned(Arc<str>):
- All compile-time-constant labels (sort bases,
relevance,co_engagement,preference_affinity) →Static— zero allocation, pointer-copy clone. - Dynamic
{signal}_boost/_penalty/_decaylabels → built once per query in aRuleLabelshoist (wasformat!per-candidate-per-rule) and shared byArc— refcount-bump clone, not a string copy. Cohort rescore hoisted the same way. scoredaccumulator pre-sized tocandidates.len().- Owned
Strings rebuilt only at the two response-assembly sites (≤ limititems). - Files:
ranking/executor/{context,mod,scoring,helpers}.rs,query/executor/{helpers,pipeline,candidate_gen}.rs,query/search/executor/pipeline.rs,tidal/Cargo.toml(smallvec promoted from lock).
Correctness: byte-identical — full 1896-test lib suite green (existing tests assert snapshot keys AND scores).
Measured win (real before/after, cargo bench --bench ranking, committed-base vs working-tree):
score_200_hot 23.89→21.04 µs (−11.9%), score_200_trending 27.66→25.85 µs (−6.5%),
score_200_full_pipeline 27.88→26.97 µs (−3.3%). Win scales with per-candidate snapshot
width; the personalized for_you path (boosts+penalties+decay) benefits most and is not yet benched.
T2 — per-term DashMap lookup collapse (NEXT, not started)
Group each candidate's exclude/gate/boost/penalty/decay terms by SignalTypeId, take ONE
entries.get() per distinct type (held Ref serves all that type's terms), collapsing
(E+G+B+P+sort+decay) shard-lock+hash+lookups per candidate to T distinct types. HIGH correctness
risk (short-circuit order, per-term degradation-window substitution, the gates-propagate /
boosts-swallow UnknownSignalType split, None→default mapping) — gated by an A/B property test
asserting identical ScoredCandidate (score + snapshot) across random profiles before the loop is switched.