Repository navigation
fix(ir): make each '_' in a rule body a fresh anonymous variable - #219
Merged
Merged
Conversation
Lowering named a placeholder column after its relation and position
(_ph_{relation}_{i}), so two body atoms over one relation with '_' at the
same position joined on it. w(A,B) <- user(A,_,_), user(B,_,_) returned
only the diagonal and two(X,Z) <- e(X,Y,_), e(Y,Z,_) returned nothing,
for session and persistent rules alike. Name placeholders per atom
(_ph_a{atom}_c{i}), like constants already are.
The naming dates from 2024, so this is long-standing, not a #206
regression.
25_running_total and 30_self_comparison had recorded the wrong rows (counts of 1, no results) produced by joined placeholders. The debug snapshots show the new per-atom placeholder column names.
Contributor
Author
Performance gate (
|
| metric | baseline | candidate | cost ratio | interval | budget | baseline spread | status |
|---|---|---|---|---|---|---|---|
| bound_query.concurrent_latency_us.p99 | 12.872 ms | 12.810 ms | 0.995 | 0.925..1.025 | 1.10 | 6.4% | PASS |
| bound_query.latency_us.p50 | 8.312 ms | 8.294 ms | 1.003 | 0.986..1.021 | 1.05 | 4.3% | PASS |
| bound_query.latency_us.p99 | 9.007 ms | 8.733 ms | 0.970 | 0.941..1.010 | 1.10 | 19.9% | PASS |
| bound_query.queries_per_sec | 836/s | 815/s | 0.975 | 0.957..1.096 | 1.05 | 22.4% | INCONCLUSIVE |
| cheap_query.latency_us.p50 | 0.432 ms | 0.445 ms | 1.006 | 0.975..1.069 | 1.05 | 10.4% | INCONCLUSIVE |
| cheap_query.latency_us.p99 | 0.524 ms | 0.516 ms | 0.954 | 0.829..1.114 | 1.10 | 39.1% | INCONCLUSIVE |
| cheap_query.queries_per_sec | 9467/s | 9392/s | 0.996 | 0.963..1.034 | 1.05 | 12.9% | PASS |
| delta_fanout.delta_us.p50 | 13.256 ms | 12.403 ms | 0.921 | 0.872..0.951 | 1.05 | 519.2% | PASS |
| delta_fanout.delta_us.p99 | 22.252 ms | 22.058 ms | 0.968 | 0.718..1.394 | 1.10 | 864.8% | INCONCLUSIVE |
| delta_fanout.last_agent_us.p99 | 26.940 ms | 25.454 ms | 0.921 | 0.661..1.372 | 1.10 | 860.2% | INCONCLUSIVE |
| delta_first.first_delta_us.p50 | 40.674 ms | 3.835 ms | 0.094 | 0.091..0.096 | 1.05 | 1.1% | PASS |
| delta_first.warm_delta_us.p50 | 4.559 ms | 4.535 ms | 0.980 | 0.951..1.063 | 1.05 | 31.4% | INCONCLUSIVE |
| delta_single.delta_us.p50 | 7.151 ms | 5.476 ms | 0.764 | 0.735..0.884 | 1.05 | 50.5% | PASS |
| delta_single.delta_us.p99 | 17.552 ms | 12.487 ms | 0.687 | 0.576..1.088 | 1.10 | 90.2% | PASS |
| insert_batch.ack_us.p50 | 8.086 ms | 3.376 ms | 0.402 | 0.363..0.463 | 1.05 | 51.0% | PASS |
| insert_batch.facts_per_sec | 43052/s | 192880/s | 0.224 | 0.203..0.235 | 1.05 | 5.1% | PASS |
| insert_single.ack_us.p50 | 7.495 ms | 1.425 ms | 0.186 | 0.143..0.206 | 1.05 | 470.0% | PASS |
| insert_single.ack_us.p99 | 42.136 ms | 4.875 ms | 0.116 | 0.073..0.162 | 1.10 | 37.1% | PASS |
| insert_single.concurrent_facts_per_sec | 178/s | 743/s | 0.237 | 0.224..0.262 | 1.05 | 11.4% | PASS |
| insert_single.facts_per_sec | 47/s | 643/s | 0.072 | 0.069..0.077 | 1.05 | 16.5% | PASS |
| interference.delta_us.p50 | 7.242 ms | 6.018 ms | 0.830 | 0.794..0.877 | 1.05 | 274.6% | PASS |
| interference.delta_us.p99 | 15.208 ms | 13.152 ms | 0.663 | 0.488..1.118 | 1.10 | 896.9% | INCONCLUSIVE |
Diagnostic metrics (not gated)
| metric | baseline | candidate | cost ratio | interval | budget | baseline spread | status |
|---|---|---|---|---|---|---|---|
| bound_query.concurrent_latency_us.p50 | 8.302 ms | 8.552 ms | 1.030 | 1.000..1.038 | 1.05 | 8.2% | PASS |
| cheap_query.concurrent_latency_us.p50 | 0.810 ms | 0.818 ms | 0.995 | 0.961..1.050 | 1.05 | 14.6% | INCONCLUSIVE |
| cheap_query.concurrent_latency_us.p99 | 1.335 ms | 1.313 ms | 0.988 | 0.944..1.080 | 1.10 | 13.1% | PASS |
| delta_fanout.ack_us.p50 | 5.663 ms | 4.315 ms | 0.771 | 0.713..0.783 | 1.05 | 1217.5% | PASS |
| delta_fanout.ack_us.p99 | 11.193 ms | 9.973 ms | 0.856 | 0.707..1.118 | 1.10 | 1547.1% | INCONCLUSIVE |
| delta_fanout.last_agent_us.p50 | 19.027 ms | 16.717 ms | 0.868 | 0.834..0.887 | 1.05 | 445.9% | PASS |
| delta_fanout.subscribe_us.p50 | 3.496 ms | 3.510 ms | 1.006 | 0.923..1.022 | 1.05 | 34.5% | PASS |
| delta_fanout.subscribe_us.p99 | - | - | - | - | 1.10 | - | INVALID: baseline round 1: 64 samples, need 100 |
| delta_first.first_delta_us.p99 | - | - | - | - | 1.10 | - | INVALID: baseline round 1: 40 samples, need 100 |
| delta_first.subscribe_us.p50 | 3.538 ms | 3.510 ms | 0.992 | 0.987..1.017 | 1.05 | 5.4% | PASS |
| delta_first.subscribe_us.p99 | - | - | - | - | 1.10 | - | INVALID: baseline round 1: 40 samples, need 100 |
| delta_first.warm_delta_us.p99 | - | - | - | - | 1.10 | - | INVALID: baseline round 1: 40 samples, need 100 |
| delta_single.ack_us.p50 | 11.139 ms | 3.142 ms | 0.286 | 0.237..0.407 | 1.05 | 92.5% | PASS |
| delta_single.ack_us.p99 | 42.102 ms | 10.171 ms | 0.242 | 0.216..0.349 | 1.10 | 6.4% | PASS |
| delta_single.last_agent_us.p50 | 7.151 ms | 5.476 ms | 0.764 | 0.735..0.884 | 1.05 | 50.5% | PASS |
| delta_single.last_agent_us.p99 | 17.552 ms | 12.487 ms | 0.687 | 0.576..1.088 | 1.10 | 90.2% | PASS |
| delta_single.subscribe_us.p50 | - | - | - | - | 1.05 | - | INVALID: baseline round 1: 1 samples, need 20 |
| delta_single.subscribe_us.p99 | - | - | - | - | 1.10 | - | INVALID: baseline round 1: 1 samples, need 100 |
| insert_batch.ack_us.p99 | - | - | - | - | 1.10 | - | INVALID: baseline round 1: 20 samples, need 100 |
| insert_single.concurrent_ack_us.p50 | 40.119 ms | 4.915 ms | 0.132 | 0.121..0.539 | 1.05 | 86.6% | PASS |
| insert_single.concurrent_ack_us.p99 | 42.045 ms | 13.243 ms | 0.317 | 0.196..0.330 | 1.10 | 1.7% | PASS |
| interference.ack_us.p50 | 10.618 ms | 3.447 ms | 0.338 | 0.238..0.552 | 1.05 | 334.3% | PASS |
| interference.ack_us.p99 | 42.059 ms | 10.220 ms | 0.241 | 0.156..0.260 | 1.10 | 247.7% | PASS |
| interference.long_request_us.p50 | 187.781 ms | - | - | - | 1.05 | 0.0% | INVALID: baseline round 2: 19 samples, need 20 |
| interference.long_request_us.p99 | - | - | - | - | 1.10 | - | INVALID: baseline round 1: 20 samples, need 100 |
jsam
marked this pull request as ready for review
October 3, 2026 22:47
jsam
added a commit
that referenced
this pull request
Oct 4, 2026
…ry request (#249) The release-checkpoint performance gate fails on main: cheap_query serial p50 is 1.110x the approved baseline e732cd5 on the 32-vCPU runner (115 -> 126 us). Concurrent latency and throughput did not move. Bisected to #214 (per-connection request pipelining), made worse by #222. Cause: the pipeline runs requests in a `FuturesUnordered`. Its `poll_next` wakes its own task and returns `Pending` after any poll that polled every future in the set. With one request in flight, which is every serial client, that happens on each request's first poll. The connection task wakes itself, tokio schedules that as a yield, and a yield wakes an idle worker to take the task. Each request therefore costs one more context switch (4.0 -> 5.0 per query) and a hand-off of the connection task to another core. #222 added no work per query but made the hand-off dearer: user cycles per query rose 15% at the same instruction count. Fix: keep one never-woken future in the set. After the pipeline's first poll it is never polled again, so no poll reaches every future and the forced yield no longer fires. The yield after futures that wake themselves still applies. Pipelining, ordering, deadlines, cancellation and the credential fence are unchanged. Nothing is added per request: no lock, atomic or allocation. Each connection gets one more set node. Bisect (cheap_query, `?edge(1, Y)`, serial). Server counters per query from perf stat. Ratios are against e732cd5, from an interleaved probe with one pinned server per commit, 40 rounds: | commit | PR | user instr/query | ctx switches/query | p50 ratio | |---|---|---|---|---| | e732cd5 | baseline | 1.282M | 4.0 | 1.000 | | d75ce6e | #208 atomic programs | 1.279M | 4.0 | 0.993 | | 4578fe6 | #215 notification stream | 1.271M | 4.0 | 0.991 | | 71a4d24 | #216 online backup | 1.275M | 4.0 | 1.016 | | 2f621c5 | #200 first-update latency | 1.272M | 4.0 | 1.009 | | e96df5c | #207 | 1.279M | 4.0 | 0.998 | | 4dd8fe9 | #209 shard index | 1.272M | 4.0 | 0.987 | | 277c7f0 | #206 | 1.274M | 4.0 | 1.008 | | 3554c98 | #214 pipelining, deadlines | 1.296M | 5.0 | 1.020 | | 3492a59 | #213 | 1.294M | 5.0 | 1.030 | | 2ba9a5a | #212 provenance | 1.297M | 5.0 | 1.029 | | 9ed61b4 | #204 key expiry | 1.297M | 5.0 | 1.027 | | be35ee3 | #221 | 1.297M | 5.0 | 1.013 | | c98a590 | #222 bounded streaming | 1.298M | 5.0 | 1.047 | | a6fd628 | #219 wildcard fix | 1.297M | 5.0 | 1.061 | | b7f7ad6 | #220 WAL durability | 1.300M | 5.0 | 1.056 | | de54807 | #223 shared standing eval | 1.299M | 5.0 | 1.071 | | 55ec34d | #225 checkpoint-1 fixes | 1.300M | 5.0 | 1.045 | | 23712b8 | main | 1.300M | 5.0 | 1.064 | | this change | | 1.300M | 4.0 | 1.033 | Confirming A/Bs with the gate's own fixture (fresh pinned servers each round, 40 rounds, ratio [95% interval]): - #214 vs its parent 277c7f0: 1.039 [1.027..1.049] - #222 vs its parent be35ee3: 1.025 [1.006..1.044] - A/A 3554c98: 1.010 [1.002..1.025] - e732cd5 -> main: 1.048 [1.028..1.066] - main -> this change: 0.978 [0.958..0.989] - e732cd5 -> this change: 1.021 [1.009..1.044], PASS (budget 1.05) User cycles per query (perf stat, same instruction count): e732cd5 958K, be35ee3 1053K, c98a590 1206K, this change 1006K. Full gate, all fixtures, 20 rounds, servers pinned to CPUs 2-15: - main -> this change: no metric worse. Every required metric is PASS, or INCONCLUSIVE with an interval that includes 1.0. cheap_query p50 0.979 [0.953..1.007]. - e732cd5 -> this change: cheap_query p50 1.029 [1.019..1.072], INCONCLUSIVE on a busy host (load average up to 22); bound_query p50 1.022, PASS. The insert and delta metrics improved since the baseline. The shared host is about 4x slower per query than the CI runner, so the same hand-off is a smaller share of each query here than there. The remaining ~1.7% of user instructions per query since #214 is the pipeline's own per-request bookkeeping: the boxed request future (one 4-8 KiB allocation), its set node, the request control and the in-flight maps.
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.
Problem
In a rule body, every
_must be a fresh anonymous variable. Lowering named a placeholder column after its relation and position (_ph_{relation}_{i}). When two body atoms read the same relation with_at the same position, both scans got the same column name, and the join used it as a key. Results were silently wrong, for session and persistent rules alike:The plain query
?user(A,_,_), user(B,_,_)looked correct only because the query path renames the goal atom's_to_p{i}before lowering.Fix
build_scannow names each placeholder per body atom,_ph_a{atom}_c{i}, the same scheme constants already use. The_ph_prefix is unchanged, so antijoin key inference still skips placeholder columns.Not a #206 regression
This is a long-standing bug. The
_ph_{relation}_{i}naming dates from 2024 (5cf9a2c). The new regression tests fail 7/7 on 4dd8fe9, the commit just before #206 (277c7f0), the same as on main.Tests
tests/wildcard_rule_tests.rscovers both repros above, plus session rules, persistent rules after a restart, wildcards in negated atoms, wildcards under an aggregate, and wildcards in a recursive rule. All 7 tests fail on main without the fix and pass with it.ir_builderunit test: in a self-join, the_columns are not join keys.examples/iql/06_joins/79_wildcard_fresh_per_atom.iql.18_advanced_patterns/25_running_total: the running count is now 1..5 instead of all 1s.18_advanced_patterns/30_self_comparison:higher_thannow returns its 5 rows instead of none.33_meta/17_debug_joinand26_debug_complexshow the new placeholder column names.Validation
make test: fmt, clippy and unit/integration tests pass. In the snapshot suite, 1149 pass and 3 fail. The 3 failures are already on main and unrelated to this PR:33_meta/04_help,33_meta/11_help_specificand39_meta_complete/07_help_comprehensive. feat(storage): consistent online checkpoint export (.backup) (#48) #216 added.backupto the client help without regenerating them.