Repository navigation
perf(sdk): hash the Surface package with concurrent reads, same digest - #683
Merged
Merged
Conversation
The CLI hashes the installed @relayflows/surface tree while loading an authored flow, and the authored Node child hashes it again while loading the pinned source. Both awaited one readFile at a time over ~600 files. Read with up to 32 reads in flight and feed each file to the hash strictly in the sorted walk order, so the digest is byte-identical. After a failure no new read starts, and the lowest-index failure is thrown, which is the error the serial walk meets first. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars.
|
[Medium impact] The PR appears safe to merge; both previous findings are addressed.
|
…ads in flight Review (#683): 32 workers bounded reads in flight but not the buffers waiting for an earlier, slow file, so the whole package could pile up in memory. Read through an in-order window of 32 files instead: a slow file stops new reads. After any failure no read starts, every started read settles, and the lowest-index failure is thrown, as before. Tests now pin the read-ahead bound, that no read starts after the first failure, and that the rejection waits for a read still in flight. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
The CLI hashes the installed
@relayflows/surfacepackage while it loads an authored flow (locateSurfaceAuthority→packageTreeSha256). The authored Node child hashes it again while it loads the pinned source. Both passes awaited onereadFileat a time over the ~600 files of Surface 2.0.54.packageTreeSha256now reads up to 32 files at once. It still passes each file to the hash in the sorted walk order, so the digest is byte-identical. After a read fails, no new read starts. Once the reads already in flight settle, it throws the lowest-index failure, which is the error the serial walk would have hit first. The directory walk is unchanged (7 directories).This is one function, so it covers both the CLI pass and the Node child pass.
Same digest
The installed published Surface 2.0.54 hashes to the digest Cloud admits for 2.0.54 (
relayflow-v2-surface-authority.generated.ts:5b9ea420db78f4c76be3c5f97815fb4d7443419ece1b56fa74d60ac05d29b47a), with both the old code and the new code:I also ran 60 fresh processes alternating the old function (a verbatim copy) and the new one, on Bun 1.4.2 and Node 22.23.4. All 60 produced that one digest.
Timing
The hash alone. First call in a fresh process on the real 2.0.54 tree, 15 runs each, macOS arm64, page cache warm:
End to end, per phase. These are standalone
bun-darwin-arm64builds of main and of this branch, each with the same measurement-only marks added (never committed; script below). Each run isflows run bench.flow.tsin a project that has the Cloud Bun envelope around Surface 2.0.54, with relayflowd already started, as Cloud does. The flow's first step writes a timestamp. There were 20 interleaved rounds; the machine was shared, with load average 25–60.The Surface hash in the Node child drops from 41 to 19 ms. The hash in Bun drops by about 3 ms, because Bun's serial reads were already fast on a warm local disk. Spawn to first step improves by 33 ms at the median here. A second batch of 20 rounds gave 703 → 667 ms.
This was measured on macOS with a warm cache, not on E2B. My earlier E2B profile of this path had the Bun load phase at about 356 ms, so the saving there should be larger, but that is an estimate and has not been measured.
Tests (fail on revert)
New file
tests/authored-surface-hash.test.ts:@relayflows/surfaceexactly as an in-test copy of the serial walk doesMutation checks. For each one I reverted or changed the code, ran
npx vitest run tests/authored-surface-hash.test.ts, and restored the file byte for byte withcp/cmp:src/authored-flow-loader.tsback to main (serial reads)× reads files concurrently and still hashes them in path order → expected 1 to be greater than 1(1 failed, 2 passed)× hashes the installed @relayflows/surface exactly as the serial walk does → expected '5312886e…' to be '5b9ea420…'and× reads files concurrently… → expected '5ea2efe3…' to be 'c89ecb55…'× starts no new read after a failure… → expected … 'unreadable dist/group-0/file-03.js' but got 'unreadable dist/group-0/file-05.js'× starts no new read after a failure… → expected 81 to be less than 80Tests 3 passed (3)Verification (Node 22.23.4, Bun 1.4.2, at 999be9c)
authored-flow-lifecycle-executor.test.ts, 3 failures: the threecompletes a signal wait after … round(s)cases. These fail identically on main 2cd9561 on this machine (Tests 3 failed | 48 passed (51)). This PR does not change them.authored-parallel-run-live.test.ts, 1 failure:resumes 'llm' branches without replaying completed effectsfailed once in that run, withrun_admission_conflict, at load average 60–80. Running it alone on this branch it passed 3 of 3 times. A rerun of the whole file on this branch passed:Tests 35 passed | 7 skipped (42).Measurement harness (local only)
apply-trace.pyaddsstartupMark(label)calls, which appendpid label epoch-msto$FLOWS_STARTUP_TRACE. The calls go inrunDirectFlow(start, aroundcheckAuthoredTriggers, afterconnect), aroundassertAuthoredRuntimeAvailableinexecuteDurableAuthoredFlow, after eligibility, after the runner resolves the authority #2, before the Node child is spawned, at the start and end ofpackageTreeSha256, and inauthored-node-entry.ts(modules loaded, ready, aroundloadPinnedAuthoredSource, after the journal connect). The binaries are built withnode scripts/build-standalone-cli.mjs bun-darwin-arm64, and the checkout is restored afterwards.The flow:
Review round 1 (e5053b2)
reads ahead at most 32 files past one that is slowfails against 999be9c withexpected 81 to be less than or equal to 32.expected 35 to be 32when the stop is removed), and the rejection must wait for a read still in flight (expected Infinity to be less than or equal to …whenallSettledis removed).5b9ea420…b47afor Surface 2.0.54 on Bun and on Node 22.Tests 3 failed | 329 passed | 7 skipped (339)over the same suites. The 3 failures are thecompletes a signal wait after … round(s)cases, which also fail on main.linux-x64-artifact: pass at e5053b2. The run at 999be9c failed onlytests/mcp.test.ts > MCP preflight and transports > waits for child close when the transport cannot own a process groupwithexpected 999 to be greater than or equal to 1000. That is a 1 ms timer-boundary assertion in the MCP transport, which this PR does not touch.🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need.Agent Relay sessions
claudesessiona7546818· opened viagh pr create· last active 2026-10-26