chore: bulk-write outgoing CRDT payload across the JS boundary#8969
Open
pravusjif wants to merge 1 commit into
Open
chore: bulk-write outgoing CRDT payload across the JS boundary#8969pravusjif wants to merge 1 commit into
pravusjif wants to merge 1 commit into
Conversation
The outgoing CRDT payload was marshaled to V8 one byte at a time (PoolableByteArray fast-proxy enumerator consumed by new Uint8Array(...)), and the pooled array handed to JS was never disposed so the shared serialization pool never got its arrays back (a fresh byte[] allocation per scene tick). The wrapper now copies the payload into a script-owned Uint8Array with a single WriteBytes and returns the pooled array deterministically; empty results return null so idle ticks allocate nothing on the JS side. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
Contributor
|
PR #8969, run #27391756867 Builds: Windows change, Windows baseline, macOS change, macOS baseline How to read this table
Framework 13 i7
Exception breakdown
|
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
Fixes the two highest-impact findings of the CRDT pipeline audit (series: #8967, #8968), on the renderer→scene direction of the JS boundary:
EngineApi.jsdidnew Uint8Array(UnityEngineApi.CrdtSendToRenderer(...)). The returnedPoolableByteArrayis anIV8FastHostObjectwhose only operation is an enumerator factory, so theUint8Arrayconstructor fell back to the iterator protocol — oneMoveNext()+GetCurrent()host transition per byte. The inbound direction was already a single bulk memcpy; outbound now is too: the wrapper copies the payload into a script-ownedUint8ArrayviaIJsOperations.NewUint8Array+WriteBytes(the exact pattern already used byClientWebSocketApiImplementationand the comms controller).PoolableByteArrayhanded to JS was never disposed (ClearScript doesn't dispose collected host objects), soSharedPoolsProvider.SERIALIZED_STATE_BYTES_POOLnever got an array back and every tick of every scene allocated a freshbyte[]of outgoing-payload size. The wrapper now disposes the pooled array deterministically right after the bulk copy.nulland the JS module maps it todata: []— no JS-side allocation at all, and the SDK's no-oponmessage(emptyArray)round-trip disappears.Touches:
EngineApiWrapper(+ SDK-observables variant ctor),ISceneRuntime/SceneRuntimeImpl(exposesJsOperationsfor wrapper construction),EngineApi.js. TheIEngineApiimplementation layer is unchanged.Why
Per-byte fast-proxy transitions cost microseconds per KB on every scene tick, and tens of milliseconds for
CrdtGetStateat scene load (full state, potentially hundreds of KB). The pool leak added steady-state GC pressure proportional to outgoing traffic across all loaded scenes.QA / verification
DCL.EditMode.Testscompile clean locally (dotnet build, 0 errors).SceneRuntimeShould.EngineApi_CrdtSendToRenderer/..._CrdtGetStateexecute the real V8 path end-to-end in the EditMode suite (CI) — they cover the new null/empty contract.CrdtGetStatewith a large payload).OutgoingMessages/CrtdGetStatesamplers should show reduced time;SERIALIZED_STATE_BYTES_POOLno longer churns allocations (GC Alloc column in the scene tick).🤖 Generated with Claude Code