Skip to content

Resolve lookup-refs via bounded AVET seeks, not full-index scans - #36

Merged
tiensonqin merged 1 commit into
mainfrom
devin/lookup-ref-avet
Oct 5, 2026
Merged

tiensonqin merged 1 commit into
mainfrom
devin/lookup-ref-avet

Conversation

@tiensonqin

Copy link
Copy Markdown
Contributor

Summary

Lookup_refs.entity_id resolved lookup-refs by materializing visible_datoms (a whole-EAVT materialization + filter_pred pass) and then linearly scanning that list for the unique attr/value pair — a full-index scan per resolution. This routes direct resolution through bounded AVET seeks instead, matching upstream DataScript's indexed lookup.

Changes:

  • Lookup_refs.context replaces the visible_datoms : db -> datom list field with entid : db -> attr -> value -> entity_id option; entity_id resolves through it (context.entid is wired to entid_db → find_avet_exact, a bounded PSet.slice on the (a, v) prefix plus duplicate_avet_by_attr). A shared entity_id_of_resolution helper factors the is_unique check + strict_missing error path, so entity_id and entity_id_in_datoms produce identical results/error messages.
  • Tx-staging semantics are untouched: entity_id_in_datoms still scans the supplied datom list (entid_in_datoms over datoms), which is what staged-transaction paths call. This change is resolution-only.
  • find_avet_exact now applies db.filter_pred to its bounded result set. This preserves the old visible_datoms filter behavior on filtered dbs (upstream FilteredDB also filters -datoms), and restores pre-5e3238a parity for entid on filtered dbs.
  • Removed dead code: Transact_datoms.Make's visible_datoms context requirement and its entid helper (zero callers after 5e3238a), plus the now-unused visible_datoms alias in datascript.ml.

Before/after measurement (20k-block outliner db, 100k datoms, native exe on this VM — scratch bench, not committed):

lookup before: entity_id via visible_datoms scan after: bounded AVET seek
hit ~3.58 ms ~2.5 µs
miss ~3.59 ms ~2.1 µs

~1400× faster; scan range drops from the full 100k-datom index to a bounded attribute slice (~1 datom, unique attr). The new entity_id now costs the same as Datascript.entid (~2.3 µs), which already used find_avet_exact.

Tests: all 34 native test suites pass (test_lookup_refs covers both the unresolved-lookup-ref and non-unique-attr error paths). dune build impl/ green.

docs/upstream_differences.md updated: the "Direct Lookup Refs Scan Visible Datoms" divergence is now resolved.

Link to Devin session: https://app.devin.ai/sessions/7705343425064c3a9b38d8b0e4ceb045
Open in Devin Desktop: https://app.devin.ai/desktop/session/7705343425064c3a9b38d8b0e4ceb045?variant=devin
Requested by: @tiensonqin

@devin-ai-integration

Copy link
Copy Markdown

I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".

  • Disable automatic comment, CI, and merge conflict monitoring

Lookup_refs.entity_id materialized visible_datoms and scanned the whole
list for the unique attr/value pair. Route it through the context's entid
function (entid_db -> find_avet_exact), which performs a bounded AVET
slice on the (attr, value) prefix plus the duplicate table. The context
now carries entid instead of visible_datoms; entity_id_in_datoms keeps
its list semantics for staged transaction datoms.

find_avet_exact now applies db.filter_pred so the bounded path preserves
the old visible_datoms filter behavior on filtered dbs (and restores
upstream parity for entid on filtered dbs).

Removes dead Transact_datoms.entid (no callers) and the visible_datoms
context requirement.
@tiensonqin
tiensonqin merged commit fabdb72 into main Oct 5, 2026
2 checks passed
@tiensonqin
tiensonqin deleted the devin/lookup-ref-avet branch October 5, 2026 07:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant