Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(custom-mutators): maintain and fork ivm through pull, mutate, refresh #3934

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

tantaman
Copy link
Contributor

@tantaman tantaman commented Mar 6, 2025

Persist requires some more invasive changes and will be in a separate PR.

Calls to advance are the key thing to review. advance replaces our use of experimentalWatch to update the main IVM sources. The main difference from experimentalWatch is that advance sends in the expected hash we're advancing from and the hash we're advancing to. This allows advance to set an invariant that we always advance in order. The IVMBranch also keeps track of its current hash so it can compute diffs to use to patch forks in forkToHead(hash).

I'm surprised we never ran into problems with experimentalWatch as I would imagine there would have been cases where a refresh diff against HEAD-1 jumps ahead of a pullEnd diff against HEAD-0 (due to awaits) or vice-versa.

Copy link

vercel bot commented Mar 6, 2025

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 Mar 7, 2025 3:24pm
zbugs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 7, 2025 3:24pm

Copy link

github-actions bot commented Mar 6, 2025

🐰 Bencher Report

Branchmlaw/call-zero-option
Testbedlocalhost
Click to view all benchmark results
BenchmarkFile SizeBenchmark Result
kilobytes (KB)
(Result Δ%)
Upper Boundary
kilobytes (KB)
(Limit %)
zero-package.tgz📈 view plot
🚷 view threshold
1,007.19 KB
(+0.17%)Baseline: 1,005.50 KB
1,025.61 KB
(98.20%)
zero.js📈 view plot
🚷 view threshold
181.82 KB
(+0.26%)Baseline: 181.34 KB
184.97 KB
(98.30%)
zero.js.br📈 view plot
🚷 view threshold
50.80 KB
(+0.34%)Baseline: 50.63 KB
51.64 KB
(98.38%)
🐰 View full continuous benchmarking report in Bencher

Copy link

github-actions bot commented Mar 6, 2025

🐰 Bencher Report

Branchmlaw/call-zero-option
Testbedlocalhost

🚨 2 Alerts

BenchmarkMeasure
Units
ViewBenchmark Result
(Result Δ%)
Lower Boundary
(Limit %)
src/client/zero.bench.ts > basics > All 1000 rows x 10 columns (numbers)Throughput
operations / second (ops/s)
📈 plot
🚷 threshold
🚨 alert (🔔)
60.10 ops/s
(-23.05%)Baseline: 78.10 ops/s
60.30 ops/s
(100.33%)

src/client/zero.bench.ts > with filter > Lower rows 500 x 10 columns (numbers)Throughput
operations / second (ops/s)
📈 plot
🚷 threshold
🚨 alert (🔔)
61.10 ops/s
(-37.40%)Baseline: 97.61 ops/s
79.34 ops/s
(129.85%)

Click to view all benchmark results
BenchmarkThroughputBenchmark 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
🚨 view alert (🔔)
60.10 ops/s
(-23.05%)Baseline: 78.10 ops/s
60.30 ops/s
(100.33%)

src/client/zero.bench.ts > with filter > Lower rows 500 x 10 columns (numbers)📈 view plot
🚷 view threshold
🚨 view alert (🔔)
61.10 ops/s
(-37.40%)Baseline: 97.61 ops/s
79.34 ops/s
(129.85%)

🐰 View full continuous benchmarking report in Bencher

@@ -65,6 +66,7 @@ export async function persistDD31(
mutators: MutatorDefs,
closed: () => boolean,
formatVersion: FormatVersion,
_getZeroData: ZeroOption['getTxData'] | undefined,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

to be used in the next PR. Getting persist to work requires changes to LazyRead and LazyStore which wouldn't fit well to be reviewed in this context.

tantaman added 2 commits March 7, 2025 10:22
…ot provided

If the user of Zero has not defined any custom mutators, do not fork the IVM sources
and return `undefined` instead.

This is mainly a safety thing. We want there to be a minimal chance of breaking
existing users while we're still stabilizing custom mutators. CRUD mutators
also do not need forks of the IVM sources since they do not issue reads during
mutations.
@tantaman tantaman marked this pull request as ready for review March 7, 2025 15:47
@tantaman tantaman requested review from arv and grgbkr March 7, 2025 15:47
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