Skip to content

Binding a pushed document's declared phrases counts the base's signatures only for keys not already settled - #1310

Merged
WaylandYang merged 2 commits into
deeplethe:devfrom
gedaiu:perf/bind-declared-skips-settled
Oct 19, 2026
Merged

WaylandYang merged 2 commits into
deeplethe:devfrom
gedaiu:perf/bind-declared-skips-settled

Conversation

@gedaiu

@gedaiu gedaiu commented Oct 18, 2026

Copy link
Copy Markdown
Contributor

Part of #1296 (item 4).

When a pushed document finished extracting, bind_declared gathered the document's signature keys whose phrase names a declared relation. Then it called phrase_bindings::signatures, the GROUP BY over every live open statement in the base, and only after that skipped the signatures whose existing binding was already settled. After the first push of a phrase, every later push with the same phrase and classes is already settled. Each one still grouped the whole base and then wrote nothing.

settled only needs the key and the existing bindings. Through declared and fits_direction it reads only the phrase, the two class ids and whether the object is a value, and those are exactly the four fields of the key. So bind_declared now drops every key whose existing binding is settled before it does anything else. If no keys are left it returns without counting the base's signatures. Otherwise it counts them and filters to the keys that are left, as before. The settled check inside the loop is gone because the keys reaching it can no longer be settled. To run the check from a key, a small keyed helper builds a PhraseSignature with only the key's fields filled in. I picked this over a narrower type for declared, fits_direction and settled because fits_direction has six call sites that all pass a full signature. A new type would touch all of them, while the helper adds one function and leaves every caller as it was.

Binding behaves the same, so the change only removes cost. That means the test cannot look at the bindings to prove anything; it has to show that the whole-base signatures query was not run. It pushes two documents with the same signature: the first one binds, the second arrives already settled. It then calls bind_declared for the second document on a fresh pool with a single connection. It asserts the call returns 0 and that pg_prepared_statements on that connection holds no statement containing WITH live AS. sqlx prepares and caches every statement it runs on a connection, so a query that was skipped is absent. This is the same approach as #1307. The probe excludes its own text, because its own LIKE pattern contains WITH live AS and would otherwise match itself. Before the change the test fails with the signatures query listed; after it, it passes.

I ran cargo test -p utopia-server against a migrated Postgres with UTOPIA_TEST_REQUIRE_DB=1: 674 passed, 0 failed, 2 ignored. The phrase tests alone (phrase_ filter) are 44 passed. cargo fmt --all -- --check and cargo clippy -p utopia-server --all-targets -- -D warnings are clean.

🤖 Generated with Claude Code

…ures only for keys not already settled

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Bogdan Szabo <bogdan@re-cinq.com>

@WaylandYang WaylandYang left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, the reasoning holds: settled only reaches the key fields through declared and fits_direction, so dropping settled keys before signatures changes nothing except the cost, and the test on a fresh one-connection pool shows the whole-base grouping no longer runs on a settled push. If you touch this again, #[derive(Default)] on PhraseSignature would shrink keyed to a struct update, but it's fine as is. Merging.

@WaylandYang
WaylandYang merged commit 5d915e2 into deeplethe:dev Oct 19, 2026
6 checks passed
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.

2 participants