perf(prefilter): skip 0-cardinality auto-promoted prefilters - #226
Merged
Conversation
The auto-prefilter promotion block at concurrent_engine.rs:2831 inserts every hot filter shape that compute_filters succeeds on, including ones whose resulting bitmap matches 0 slots. A 0-cardinality prefilter can never substitute a real query (the substitute path produces an empty result, equivalent to running the query and finding nothing), so registering it just: 1. Burns a slot in the bounded prefilter registry (default cap = 32). With cap reached, useful candidates get rejected with `prefilter registry full`. 2. Adds another stale entry to the merge thread's prefilter-refresh loop, each refresh holding the FilterField read lock for 80-180 ms while compute_filters re-runs against the current snapshot. After PR #224 unlocked the previously-dead auto-promotion + refresh blocks in the merge thread, this manifested as a measurable apply-path regression: flush apply mean climbed from 5.3 ms (post-#222 alone) back to ~146 ms when all four PRs stacked, with `top_filter=[userId=...]` indicating the apply write lock was waiting on prefilter-refresh read locks. Filtering 0-card shapes at promotion time stops the registry filling with garbage and lets auto-promotion converge on actually-useful prefixes instead. Add a `if card == 0 { continue; }` guard immediately after compute_filters returns Ok(bitmap), before the insert call. Logs at tracing::debug level so it's discoverable but not noisy. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Reviewed. Approved. Surgical 16-line guard: No correctness risk: a 0-cardinality prefilter can never substitute a real query (intersection with anything is still empty), so skipping registration is strictly correct. Same self-approve caveat — treat this comment as approval. Ready to merge after Aidan tag/CI confirm. Pairs with PR #227 to address the regression #224 exposes. |
4 tasks
JustMaier
added a commit
that referenced
this pull request
Apr 25, 2026
Captures the full causal chain + measurement data for the post-#224 apply-mean regression and its resolution via PRs #226/#227/#228: - Bisect (mut_us 4-15μs vs lock_us 8-235ms in [remove_bulk_slow]) showed apply was waiting for a long-held FilterField read. - bitmap_memory_cache::scan_tick → FilterField::bitmap_bytes() iterating 769K userId entries under one read lock was the holder. - Pre-#224 the merge thread's mark_stale calls were unreachable, so the scanner had nothing to re-read and bitmap_bytes was effectively dormant. Post-#224 it ran at full cadence. - PR #227 chunked bitmap_bytes to bound the read-lock-hold; lock_us dropped 80-235ms → 5-11ms. - PR #228 fixed a separate latent bug in the warm-persist path where query.filters had been overwritten with the post-prefilter-substitution form (containing FilterClause::BucketBitmap, #[serde(skip)]) and serialization was silently failing every batch. - Final time-to-first-warm-restore: 163s total (159s dump restore + 4.6s auto-warm of 187 shapes), down from the handoff's reported 30-60min — a 100-300x reduction. Full session ship list captured at the bottom (v1.0.165 through v1.0.171 — seven perf PRs all stacked cleanly). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Auto-prefilter promotion was registering every hot filter shape whose
compute_filtersreturnedOk(bitmap)— including bitmaps with 0 slots. A 0-cardinality prefilter can never substitute a real query (substitute path produces empty results, same as running the query directly), so registering it just:prefilter registry full.compute_filtersre-runs.Impact (when paired with #224)
Pre-#224 the auto-promotion code was dead, so this never fired. After #224 unlocks it, registry fills with garbage within minutes of startup. Measured locally: flush apply mean regressed from 5.3 ms (post-#222 alone) back to ~146 ms (full stack), with
top_filter=[userId=…]indicating the apply write lock was waiting on prefilter-refresh read locks.Filter 0-card shapes at promotion time → registry stops filling with garbage → fewer stale prefilters per cycle → less read-lock-hold per merge cycle.
Change
Single guard:
before the
merge_prefilter_registry.insert(…)call. 16 lines total including the explanatory comment.Stacking
compute_filters) is the architectural fix for the same surface.Test plan
cargo build --profile fast --features server,pg-sync)🤖 Generated with Claude Code