refactor(zql): callable relationships

[grgbkr]

The Exist operators #fetchNodeForRow is quite expensive and is a work around for the fact that Node's relationships can only be consumed a single time. In order to read the relevant relationship, Exist has to fetch from source the parent Node. This fetch is pure overhead, we are fetching a row we already have.

This change updates Node's relationships to be callable, so that they can be consumed multiple times.

Node's type changes from

export type Node = {
  row: Row;
  relationships: Record<string, Stream<Node>>;
};

to

export type Node = {
  row: Row;
  relationships: Record<string, () => Stream<Node>>;
};

This change also updates ChildChange to have a node: Node rather than a row: Row so that the relationships needed by Exist are also available on ChildChange. Now all Change types use Node rather than Row.

Finally this addresses an issue with cleanup where Node's filtered by Exists or Filter were not being drained (as they never reach view-apply-change which does the draining). Added draining to cleanup in Exists and Filter for Nodes not yielded to downstream.

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
replicache-docs	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jan 30, 2025 2:19am
zbugs	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jan 30, 2025 2:19am

[github-actions]

Bencher Report

Branch	grgbkr/callable-relationships-2
Testbed	localhost

Click to view all benchmark results

Benchmark	File Size	Benchmark Result kilobytes (KB) (Result Δ%)	Upper Boundary kilobytes (KB) (Limit %)
zero-package.tgz	📈 view plot 🚷 view threshold	919.01 (-0.04%)	937.79 (98.00%)
zero.js	📈 view plot 🚷 view threshold	171.32 (-0.07%)	174.87 (97.97%)
zero.js.br	📈 view plot 🚷 view threshold	47.71 (-0.09%)	48.71 (97.95%)

🐰 View full continuous benchmarking report in Bencher

[github-actions]

Bencher Report

Branch	grgbkr/callable-relationships-2
Testbed	localhost

Click to view all benchmark results

Benchmark	Throughput	Benchmark Result operations / second (ops/s) (Result Δ%)	Lower Boundary operations / second (ops/s) (Limit %)
src/client/zero.bench.ts > basics > All 1000 rows x 10 columns (numbers)	📈 view plot 🚷 view threshold	73.93 (+2.16%)	69.71 (94.29%)
src/client/zero.bench.ts > with filter > Lower rows 500 x 10 columns (numbers)	📈 view plot 🚷 view threshold	93.41 (-1.12%)	91.05 (97.47%)

🐰 View full continuous benchmarking report in Bencher

[tantaman]

I realize this isn't done yet but it currently looks amazing in terms of perf gains.

before:

after:

[aboodman]

What's one more layer of indirection between friends 🙃

[tantaman]

LGTM.

Doesn't look like much logic changed (except for exists and some wrapping of the relationship in join with a lambda), mostly just changing how invocations of relationships work.

lmk if I missed a significant change.

[tantaman]

🎉

[arv]

Added draining to cleanup in Exists and Filter for Nodes not yielded to downstream.

Do we need to do something with FanIn/FanOut when it has no output?

[grgbkr]

Added draining to cleanup in Exists and Filter for Nodes not yielded to downstream.

Do we need to do something with FanIn/FanOut when it has no output?

Good point... yeah I think my change is not quite right because I wasn't thinking about FanIn/FanOut... a single Filter shouldn't decide to drain the node because other Filter's may let it through... hmmm

[tantaman]

had a stale pending comment about drain

[tantaman]

oh yeah, I don't understand why we have to drainStreams here and in filter.

[grgbkr]

This was an existing problem with cleanup, that I realized while updating tests for this change.

Cleanup is lazy/stream based so that it can be limited by take. Cleanup relies on something downstream draining the streams for the cleanup to actually happen.

If an operator filters out a node (as Exist or Filter does) that node is not going to reach the thing downstream that normally would drain the streams, and the cleanup for that part of the tree won't happen. The idea behind this change was to ensure the cleanup by draining the node's stream at the point it is filtered out.

My change is not quite right though, as now the node may get drained multiple times when we have a FanIn/FanOut as multiple filter/exists operators may filter it (each of them draining it), but then it may still get passed downstream if another branch does not filter it (for it to be drained another time).

Still thinking about how to address it.

cc @arv