8000 perf(engine): keep every query's stream in the allocator's thread cache (#721 follow-up) by adityamukho · Pull Request #726 · project-minigraf/minigraf · GitHub
[go: up one dir, main page]

Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- **A float constant in a pattern matches equal floats only** (#706). A float written in a pattern's value position, or bound to a `$slot` of a prepared query, matched every stored float within an absolute `f64::EPSILON` (about `2.2e-16`), so `[?e :m/x 0.0]` returned an entity whose `:m/x` is `1e-17`, while `[?e :m/x ?v] [(= ?v 0.0)]` did not. Above 1.0 the epsilon is smaller than the gap between adjacent floats, so only floats smaller than about `2.2e-16` in magnitude were affected: every one of them matched `0.0`, and `1e-20` matched `5e-20`. Float constants now compare by IEEE `==`, as joins, predicates and the indexes do: `0.0` matches `-0.0` and nothing else changes. The fix covers patterns in queries, rule bodies and `not` / `or` bodies, on the executor and the streaming engine. Nothing stored was wrong. The same fix ships in v2.0.10.
- **WAL entries of more than 1,000,000 facts replay** (#686). A WAL entry of more than 1,000,000 facts was taken as damage: replay stopped there, and the next open cut the WAL off at that entry, so after a crash, kill or close without a checkpoint that transaction and every later one in the WAL were lost, though each had committed. The writer had no such limit. Since #550 no v3 transaction writes such an entry, but a WAL left by v2.x can hold one. Replay now takes an entry's fact count as damage only when the bytes after it cannot hold that many facts. Affected every release since v0.5.0. Also fixed in v2.0.10.
- **A recursive rule query no longer joins a rule's new facts with every magic-set fact** (#694). On the engine (#548), each round after the first runs a recursive rule from the facts the last round derived, then joins the rest of its body. The rest kept the order the planner chose for the whole body, so the next clause could share no variable with the new facts: after the magic-set rewrite of `(reach :n0 ?to)`, the body `(magic ?from) [?from :next ?mid] (reach ?mid ?to)` joined each new `reach` fact with every `magic` fact, then probed `:next` once per pair. The rest of the body is now planned with the new facts' variables bound, each next pattern sharing a variable with those bound before it when one can. Answers are unchanged. Nightly `recursion` benchmarks (bench profile): `fanout/w10_d3` 4.94 s → 22.6 ms (it was about 94 ms on the executor before #548), `chain/depth_100` 699 ms → 41.5 ms, `chain/depth_10` 1.15 ms → 0.68 ms.
- **A join on a bound value probes the index instead of reading the whole attribute** (#721). #694's fix ordered a recursive rule's delta run so that the next pattern shares a variable with the new facts, but when that variable is the pattern's value (`[?x :manager ?y]` after the delta `(boss ?y ?z)`) the join was a `HashJoin` holding every `:manager` fact, rebuilt every round: `(boss :leaf ?b)` over a management tree took 1.8 s at 1M facts and 35 s at 10M instead of 2–3 ms. Such a join, on a constant attribute, is now an `IndexNestedLoopJoin` on the value: per row, the AVET ranges of the values that join-equal it (a keyword and a ref to the entity it names, either way; `0.0` and `-0.0`) and the attribute's uncheckpointed facts with those values, read once per query. A keyword's e 10000 ntity is a hash of its name, so a row whose value is an entity finds the keyword that names it only where the query made the entity from that keyword (a rule head, or a keyword its rule calls name); otherwise, on an attribute with keyword values, the join becomes the hash join it was before. Answers are unchanged. Measured with the #394 envelope (bench machine, p50 and peak RSS at 100K / 1M / 10M facts): `recursive_ancestors` 100 ms / 1.80 s / 35.0 s → 1.1 / 1.4 / 1.9 ms, 610 MiB → 4 MiB at 10M; `join_3_selective` (`[:p :manager ?b] [?s :manager ?b] [?s :name ?n]`) 10.4 ms / 144 ms / 2.36 s → 0.21 / 0.20 / 0.20 ms; other shapes unchanged. Nightly `recursion` benchmarks (in memory, bench profile): the new `ancestors/w10_d4` 97.7 → 15.1 ms, `chain/depth_100` 41.4 → 20.8 ms, `fanout/w10_d3` 22.7 → 20.8 ms. In memory every fact is uncheckpointed, and the first value probe reads the attribute's uncheckpointed facts once per query (12 ms for 11,110 facts), so `ancestors` stays above its 0.27 ms before #694.
- **A join on a bound value probes the index instead of reading the whole attribute** (#721). #694's fix ordered a recursive rule's delta run so that the next pattern shares a variable with the new facts, but when that variable is the pattern's value (`[?x :manager ?y]` after the delta `(boss ?y ?z)`) the join was a `HashJoin` holding every `:manager` fact, rebuilt every round: `(boss :leaf ?b)` over a management tree took 1.8 s at 1M facts and 35 s at 10M instead of 2–3 ms. Such a join, on a constant attribute, is now an `IndexNestedLoopJoin` on the value: per row, the AVET ranges of the values that join-equal it (a keyword and a ref to the entity it names, either way; `0.0` and `-0.0`) and the attribute's uncheckpointed facts with those values, read once per query. A keyword's entity is a hash of its name, so a row whose value is an entity finds the keyword that names it only where the query made the entity from that keyword (a rule head, or a keyword its rule calls name); otherwise, on an attribute with keyword values, the join becomes the hash join it was before. Answers are unchanged. Measured with the #394 envelope (bench machine, p50 and peak RSS at 100K / 1M / 10M facts): `recursive_ancestors` 100 ms / 1.80 s / 35.0 s → 1.1 / 1.4 / 1.9 ms, 610 MiB → 4 MiB at 10M; `join_3_selective` (`[:p :manager ?b] [?s :manager ?b] [?s :name ?n]`) 10.4 ms / 144 ms / 2.36 s → 0.21 / 0.20 / 0.20 ms; other shapes unchanged. The value-probe state is made only by a query that probes by value, and a scan with no uncheckpointed record allocates no shadow set: kept inline, the state had grown every query's stream past the 1,032 bytes glibc serves from its per-thread cache, and with the extra allocation a point query took about 1 µs (5%) longer. Nightly `recursion` benchmarks (in memory, bench profile): the new `ancestors/w10_d4` 97.7 → 15.1 ms, `chain/depth_100` 41.4 → 20.8 ms, `fanout/w10_d3` 22.7 → 20.8 ms. In memory every fact is uncheckpointed, and the first value probe reads the attribute's uncheckpointed facts once per query (12 ms for 11,110 facts), so `ancestors` stays above its 0.27 ms before #694.
- **`BrowserDb` errors carry their error code** (#673). The browser API (`@minigraf/browser`) rejected with the error's top-level message only, such as `Unknown command: nosuchcommand`, while every other API reports `[PRS-001] Unknown command: nosuchcommand`. Every `BrowserDb` rejection now goes through `MinigrafError`, so it starts with its code. Host errors that have no code yet (no IndexedDB, an import blob whose length is not a multiple of the page size) are unchanged.
- **Prepared queries use the database's rule limit** (#549). `PreparedQuery::execute` and `query` ran a rule query with the default `max_derived_facts` of 1,000,000 whatever `OpenOptions` set; they now use the database's limit, as `execute` does, and its `query_memory_limit`.
- **An `or` branch in a rule body knows the registered predicates** (#548). A rule with `or`/`or-join` evaluated its branches with the built-in functions only, so a predicate registered with `register_predicate` in a branch (`(or [(big? ?v)] [?x :special true])`) was unknown there. Branches now get the database's functions, as the rest of the rule body does.
Expand Down
2 changes: 1 addition & 1 deletion CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ Auto-migrates v7 → v8 on open (spec §9, with a backup meta page). v1–v6 are

## Test Coverage

**1713 tests** (1701 passing, 12 ignored; unit + integration + doc). Engine answers once checked by the executor are frozen in `tests/answers/` (#551).
**1714 tests** (1702 passing, 12 ignored; unit + integration + doc). Engine answers once checked by the executor are frozen in `tests/answers/` (#551).
See `docs/TEST_COVERAGE.md` for the full per-file breakdown.

**Testing conventions** — see the Testing Conventions section below before writing any tests.
Expand Down
7 changes: 4 additions & 3 deletions docs/TEST_COVERAGE.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@

| | Default features | With `--features test-hooks` |
|---|---|---|
| **Passing** | 1,701 | 1,717 |
| **Passing** | 1,702 | 1,718 |
| **Ignored** | 12 | 13 |
| **Total** | 1,713 | 1,730 |
| **Total** | 1,714 | 1,731 |

**Verified**: 2026-10-23 with `cargo test --quiet`.
**Verified**: 2026-10-24 with `cargo test --quiet`.

#551 PR 2 deleted the executor's query path, with the 73 unit tests of its internals and the oracle. Two engine tests replace shapes no other test covered, and a storage unit test covers the reads only the executor made.

Expand Down Expand Up @@ -415,6 +415,7 @@ These tests compare the database against an independent answer rather than a sin
- Engine unit tests check routing (`:as-of` timestamps, a rule body with no pattern, a three-argument call and a write transaction) and derived rows read across batches before and after a checkpoint. `relation.rs` unit tests check row identity and the `Old` / `Delta` / `Full` ranges.
- Every delta run of the magic-set rewrite of a recursive rule joins each clause on a variable bound before it, with no keyless join (`rule_delta_runs_join_on_a_bound_variable`, #694). An optimizer unit test checks `plan_after`'s connected order.
- A join on a bound value probes its AVET ranges and pending facts (#721): the delta run of the ancestors shape probes `[?x :manager ?y]` by value, in memory and on a half-checkpointed file (`rule_delta_probes_a_bound_value`); a value join reads only the probed keys, through a pending retraction (`value_probe_reads_only_the_probed_keys`); keyword and ref from either side, an entity cell no keyword was recorded for (the hash-join fallback), `0.0`/`-0.0`, long strings, `:as-of` and `:valid-at`, in memory, checkpointed and pending (`value_probe_matches_as_joins_do`); a fallback in the middle of a batch joins the rows before it, the rest of the batch and later batches once each (`value_probe_falls_back_mid_batch`).
- A query's boxed `Stream` stays within the 1,032 bytes glibc serves from its per-thread cache (`stream_fits_the_allocator_thread_cache`, 64-bit glibc): #721's value-probe state, kept inline, had grown it to 1,056 bytes and a point query by about 5% (#721 follow-up).
- **`:db/valid-at` in rule bodies** (`tests/recursive_rules_test.rs`, #617): a `:db/valid-at` pattern in a rule definition, in its `not` and `not-join` bodies and in its `or` branches binds the query's valid time, on the semi-naive, stratified and magic-sets paths.
- **Rules calling `not` / `or` rules** (`tests/not_join_test.rs`, #628)
- A rule calling a rule with `not-join`, `not` or `or` returns that rule's rows, with a free and a literal argument and through a forwarding rule.
Expand Down
22 changes: 18 additions & 4 deletions src/query/engine/ids.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,10 @@ pub(crate) struct IdSpace {
/// What `provisional` and `dict` hold (#549 spec §3.2).
held_provisional: Held,
held_dict: Held,
/// What value probes read once for the whole query (#721).
pub(crate) probes: super::scan::Probes,
/// What value probes read once for the whole query (#721), made by the
/// first query that needs it: boxed, it keeps every `Stream` within the
/// allocator's thread cache.
prob 10000 es: Option<Box<super::scan::Probes>>,
}

#[derive(Default)]
Expand Down Expand Up @@ -66,7 +68,7 @@ impl IdSpace {
dict: Maps::default(),
held_provisional: Held::new(meter, "IdSpace"),
held_dict: Held::new(meter, "IdSpace"),
probes: super::scan::Probes::new(meter),
probes: None,
}
}

Expand All @@ -81,7 +83,19 @@ impl IdSpace {
self.end_pull();
self.provisional = Maps::default();
self.held_provisional.release();
self.probes = super::scan::Probes::new(self.held_provisional.meter());
self.probes = None;
}

/// What value probes have read so far, if any.
pub(crate) fn probes(&self) -> Option<&super::scan::Probes> {
self.probes.as_deref()
}

/// What value probes read, to add to.
pub(crate) fn probes_mut(&mut self) -> &mut super::scan::Probes {
let meter = self.held_provisional.meter();
self.probes
.get_or_insert_with(|| Box::new(super::scan::Probes::new(meter)))
}

/// Count an entry of `bytes` about to go into the maps that hold
Expand Down
2 changes: 1 addition & 1 deletion src/query/engine/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ pub(crate) fn open_rules(
)
.map_err(|e| err_coded!(ErrorCode::Int049, format!("query engine: {e}")))?;
let (e, i) = (ids.eid(keys, &e)?, ids.iid(keys, k)?);
ids.probes.note(e, i)?;
ids.probes_mut().note(e, i)?;
}
let mut rels = relation::Relations::new(iids, &meter);
for (e, a, v) in seeds {
Expand Down
4 changes: 2 additions & 2 deletions src/query/engine/recursive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ fn entity<'c>(
// entity (#721).
if let EdnValue::Keyword(name) = c {
let i = ctx.ids.iid(ctx.keys, name)?;
ctx.ids.probes.note(e, i)?;
ctx.ids.probes_mut().note(e, i)?;
}
Cell::Eid(e)
}
Expand All @@ -253,7 +253,7 @@ fn entity<'c>(
Some(Cell::Eid(e)) => return Ok(Cell::Eid(*e)),
Some(Cell::Iid(i)) => {
let e = ctx.ids.keyword_eid(ctx.keys, *i)?;
ctx.ids.probes.note(e, *i)?;
ctx.ids.probes_mut().note(e, *i)?;
return Ok(Cell::Eid(e));
}
Some(Cell::Val(v)) => match v {
Expand Down
34 changes: 17 additions & 17 deletions src/query/engine/scan.rs
10BC8
Original file line number Diff line number Diff line change
Expand Up @@ -124,8 +124,9 @@ struct Committed {
resume: Option<Vec<u8>>,
/// The value constant when the ranges do not fix it.
test: Option<KeyTest>,
/// Triples a pending record decides, as [`keys::entity_attribute_value_prefix`].
shadow: Arc<HashSet<Vec<u8>>>,
/// Triples a pending record decides, as [`keys::entity_attribute_value_prefix`];
/// `None` for none, so a scan with no pending record allocates nothing for it.
shadow: Option<Arc<HashSet<Vec<u8>>>>,
}

/// The attribute and value constants of a scan, against the DICT.
Expand Down Expand Up @@ -237,10 +238,7 @@ impl Scan {
self.state = if ranges.is_empty() {
self.bucket()
} else {
let shadow = self
.values
.as_ref()
.map_or_else(Arc::default, |v| v.shadow.clone());
let shadow = self.values.as_ref().map(|v| v.shadow.clone());
State::Committed(Committed {
ranges,
next: 0,
Expand Down Expand Up @@ -273,10 +271,10 @@ impl Scan {
/// and the committed triples its pending records decide (#721): read once
/// per query.
fn values(attribute: &str, ctx: &mut Ctx<'_>) -> Result<Arc<Values>> {
if let Some(v) = ctx.ids.probes.values.get(attribute) {
if let Some(v) = ctx.ids.probes().and_then(|p| p.values.get(attribute)) {
return Ok(v.clone());
}
let mut held = Held::new(ctx.ids.probes.held.meter(), "IndexNestedLoopJoin");
let mut held = Held::new(ctx.ids.probes_mut().held.meter(), "IndexNestedLoopJoin");
let mut pending: HashMap<Vec<u8>, Vec<Fact>> = HashMap::new();
let mut keywords = false;
let mut resume = None;
Expand Down Expand Up @@ -326,7 +324,7 @@ fn values(attribute: &str, ctx: &mut Ctx<'_>) -> Result<Arc<Values>> {
shadow: Arc::new(shadow),
});
held.charge(HASH_ENTRY_OVERHEAD + size_of::<(String, Arc<Values>)>() + attribute.len())?;
let probes = &mut ctx.ids.probes;
let probes = ctx.ids.probes_mut();
probes.held.absorb(held);
probes.values.insert(attribute.to_string(), v.clone());
Ok(v)
Expand All @@ -353,7 +351,7 @@ fn candidates(
}
Cell::Eid(e) => {
let mut v = vec![Value::Ref(ctx.ids.uuid(ctx.keys, *e)?)];
match ctx.ids.probes.named.get(e).copied() {
match ctx.ids.probes().and_then(|p| p.named.get(e)).copied() {
Some(i) => v.push(Value::Keyword(ctx.ids.name(ctx.keys, i)?.to_string())),
None if pending_keywords => return Ok(None),
None => match a {
Expand All @@ -372,7 +370,7 @@ fn candidates(
/// Whether any live committed value of attribute `a` is a keyword: read
/// once per query.
fn has_keywords(a: u32, ctx: &mut Ctx<'_>) -> Result<bool> {
if let Some(&k) = ctx.ids.probes.keywords.get(&a) {
if let Some(&k) = ctx.ids.probes().and_then(|p| p.keywords.get(&a)) {
return Ok(k);
}
let Some(keys) = ctx.keys else {
Expand All @@ -388,7 +386,7 @@ fn has_keywords(a: u32, ctx: &mut Ctx<'_>) -> Result<bool> {
&mut buf,
)?;
let k = !buf.is_empty();
let probes = &mut ctx.ids.probes;
let probes = ctx.ids.probes_mut();
probes
.held
.charge(HASH_ENTRY_OVERHEAD + size_of::<(u32, bool)>())?;
Expand Down Expand Up @@ -445,10 +443,12 @@ impl Operator for Scan {
let mut row = vec![None; self.spec.width];
for kf in &self.keys_buf {
if !visible(valid, kf.vf, kf.vt)
|| (!c.shadow.is_empty()
&& c.shadow.contains(&keys::entity_attribute_value_prefix(
kf.e, kf.a, &kf.v,
)))
|| c.shadow.as_ref().is_some_and(|s| {
!s.is_empty()
&& s.contains(&keys::entity_attribute_value_prefix(
kf.e, kf.a, &kf.v,
))
})
{
continue;
}
Expand Down Expand Up @@ -739,7 +739,7 @@ fn open(
next: 0,
resume: None,
test,
shadow: Arc::new(shadow),
shadow: (!shadow.is_empty()).then(|| Arc::new(shadow)),
}))
}

Expand Down
11 changes: 11 additions & 0 deletions src/query/engine/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2958,3 +2958,14 @@ fn aggregate_finish_keeps_sum_distinct_members_charged_once() {
"members not counted twice"
);
}

/// Every query boxes its `Stream` (`Answer::Stream`). glibc serves blocks of
/// up to 1,032 bytes from a per-thread cache and larger ones through its
/// locked bins, which cost a point query about 5% (#721 grew the stream to
/// 1,056 bytes). State a query may not need belongs behind a `Box`.
#[cfg(all(target_os = "linux", target_env = "gnu", target_pointer_width = "64"))]
#[test]
fn stream_fits_the_allocator_thread_cache() {
let size = std::mem::size_of::<super::Stream>();
assert!(size <= 1032, "Stream is {size} bytes");
}
Loading
0