perf: remove an unnecessary buffer copy from WASM asset loading pipeline#22310
Open
joseph-gio wants to merge 4 commits intobevyengine:mainfrom
Open
perf: remove an unnecessary buffer copy from WASM asset loading pipeline#22310joseph-gio wants to merge 4 commits intobevyengine:mainfrom
joseph-gio wants to merge 4 commits intobevyengine:mainfrom
Conversation
andriyDev
reviewed
Dec 30, 2025
Contributor
andriyDev
left a comment
There was a problem hiding this comment.
I am a big fan of getting rid of these copies!! Thank you!
The use of unsafe is a little uncomfy, but it's a pretty obvious optimization and avoids assigning a bunch of zeroes for no reason.
Contributor
|
The generated |
mockersf
reviewed
Dec 30, 2025
andriyDev
approved these changes
Dec 31, 2025
Contributor
andriyDev
left a comment
There was a problem hiding this comment.
Approving though this is of course blocked on the git dependency.
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.
Objective
The current asset loading pipeline on WASM looks like this:
fetchweb API.ArrayBuffer, and make a JSUInt8Arrayview into it.Vec<u8>and copy the contents of the JS Uint8Array into it.VecReader, which implementsAsyncRead..read_to_end()on the type-erased VecReader to copy the bytes into another Vec.Step (3.) above is unnecessary, as we can read from the
Uint8Arraydirectly.Solution
Add a new
UInt8ArrayReadertype, which wraps a JS byte buffer to implementAsyncRead.read_to_end()for this type, I had to forkstackfutureto add support for non-send futures.NOTE: before we can merge this PR, we will either have to wait until the following
stackfuturePR is merged, or publish a fork: microsoft/stackfuture#34Testing
many_foxesexample as a smoke test, which did not appear to have any problems.0.15.3release, for the WASM app at my workplace.AsyncSeekfunctionality -- as far as I can tell this functionality is not used by any of ourAssetLoaders.