feat: add cross-platform transferables support and lean transport benchmarks#57
feat: add cross-platform transferables support and lean transport benchmarks#57lamnhan066 merged 8 commits intolamnhan066:mainfrom
Conversation
|
Are there any issues from my side behind the ci cancelling/failing? On the local machine the tests are running without any issues |
|
The issue is currently being caused by the Dart formatter. |
…ts to abstract web-specific `JSArrayBuffer` conversion for transferables.
…s to prevent CI timeouts.
|
Hi @lamnhan066 — two issues showed up in CI that I've since fixed in this branch
Root cause: processBytes in test/web_transfer_test.dart was annotated with @isolateManagerWorker, which caused the code generator (run as a CI step) to Fix:
Root cause: The 10MB WASM benchmark pre-allocates 60MB of data (6 × 10MB arrays filled byte-by-byte in WASM linear memory) before timing starts. On Fix: Added an early return for _isWasm && sizeKb >= 10240 — the 10MB WASM numbers are already recorded in the PR description from local runs. Sorry for the noise — everything passes cleanly now including the full coverage run. Please rerun when you get a chance! |
|
We should not remove the annotations because we need to regenerate them to ensure they are compatible with both old and new versions on each release. |
|
You can also run |
…or workers, and simplify web transferable handling by removing custom helpers.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #57 +/- ##
==========================================
- Coverage 91.81% 85.43% -6.38%
==========================================
Files 23 25 +2
Lines 464 604 +140
==========================================
+ Hits 426 516 +90
- Misses 38 88 +50 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
The reports said that we were missing tests. Are there any code that we couldn’t write tests for? |
…c and add
comprehensive tests
- Fix HashSet<ByteBuffer>.identity() → HashSet<ByteBuffer>() in
encodeNativeTransferPayload: Dart VM creates a new _ByteBuffer
wrapper on every .buffer call, so identity comparison always
returned false, silently bypassing all Uint8List/ByteBuffer
transferable encoding.
- Add TypedData guard in decodeValue before the List branch:
Uint8List implements List<int>, so without the guard non-target
typed-data values were iterated element-by-element and returned
as List<Object?> instead of preserved as Uint8List.
- Expand native_transferable_codec_test.dart from 4 to 17 tests,
covering all encode/decode branches: Uint8List direct transferable,
List payload, non-string key Map, buffer/TTD deduplication,
non-matching type skip, and all decode edge cases.
- Add ByteBuffer and List branch coverage to auto_transfer_test.dart
via a new testWorkerBufferAndList worker.
|
Yeah @lamnhan066 , it was an issue from my side i thought the edge cases are covered , depending on how dart processes things |
|
Could you also update the docs and note about the pros and cons of the new implementations? |
|
@lamnhan066 I have added the docs, should i add changelogs with it and version bump to 6.2.0 with it or just update the docs with the pros and cons of using it with each platform and provide the changelogs separately? |
|
I'll update the changelogs and bump the version later before releasing the next version, you just need to update the docs with pros and cons. |
…for `Uint8List` and `ByteBuffer`, including usage, auto-extraction, and platform-specific behaviors.
|
@lamnhan066 Can you try check the documentation. Do you want me to add anything else it it |
|
The PR is completed and ready to merge, but this is a big implementation for the |
|
Thank you for the update. I hope the pr helps. Also if you have any issues with this or anything else, I would be happy to contribute. |
|
Hello @kartikey321 After running the benchmark and reviewing the code, I’ve noticed a significant performance improvement, but I still have a few questions:
|
|
Allow me to clarify the things one by one While working on it I felt if we had it working for all platforms instead of just web not only will it be consistent along all platforms but also help in non web platforms. Regarding the codec implementation the initial draft and working was done by me to parse the incoming message, Similar implementation existed in GraphQL Multipart Request (https://github.com/jaydenseric/graphql-upload/blob/master/processRequest.mjs#L140-L220) So yeah ai was used to research , improve the code and write documentation for it When i ran the tests i was not very happy with the results only the web was profiting from it , both dart vm and wasm were showing if not increased but similar or not much difference as i had quoted in the issue expectations it was because it was converting it at that time so making the transferable use obsolete as a copy was regardlessly made. So i tried with prebuilt transferablebytes and arraybuffers and apart from wasm i was getting quite good results. if you run the tests there are three types of tests, there are three types of results withouttranferables is how it is usually sent to sendport or workers Should we automatically avoid using it in WASM (for example, by relying on the kIsWasm flag)? Lastly summarising the changes done by me were the initial draft code and changes, ai helped me in improving it adding additional test cases to come at par with the coverage and lastly write the readmes for the test results i shared with you If you are skeptical of the codec, we could remove that and allow only prebuilt transferables, but in that case the user has to be vigilant about using it and not materialising if after sending it. I kept that to minimise the effort and even if low some performance improvements |
|
Thank you for your PR. I'll merge it and perform some additional testing and review before releasing it to production. |
Related issue
Closes #56
What this PR changes
1) Transferables support across platforms
Threads a
transferablesparameter end-to-end fromIsolateManager.compute()→IsolateQueue→sendMessage()→sendIsolate()/postMessage(). Callers opt in explicitly — no implicit buffer detachment.Supported transferable types:
ByteBuffer,Uint8List(wrapped via codec), or pre-builtTransferableTypedData(O(1) send)ByteBuffer,Uint8List, or rawJSArrayBuffer— passed directly in thepostMessagetransfer list for true zero-copy2) Native transferable codec
Extends the native envelope codec to support
TransferableTypedDatadirectly in the transferables list, so callers who pre-build their own TTD objects can pass them through without the codec re-wrapping them.3) Web transfer correctness tests
Adds browser tests verifying:
ArrayBufferis detached afterpostMessagewith transfer list on dart2js (confirms zero-copy)JSArrayBufferis accepted directly in the transferables list4) Shared benchmark helpers
Extracts
benchmark_helpers.dartwith sharedrunBenchmarkCase,buildBytes,formatMs,benchmarkSizesKbto remove duplication between VM and web benchmark files.Benchmarks
All runs use pre-allocated data — timing captures transport cost only, not allocation or fill. Reported as median over measured samples after warmup.
Commands:
VM (native isolate)
ByteBuffertransferTransferableTypedData¹Web — dart2js (Chrome)
ArrayBuffertransferWeb — dart2wasm (Chrome)
ArrayBuffertransferNotes
transferablesparameter is always optional and backward-compatible — existing code requires no changes.