Implement TextEncoder.encodeInto() for faster JS/Wasm interop
Categories
(Core :: Internationalization, enhancement)
Tracking
()
Tracking | Status | |
---|---|---|
firefox66 | --- | fixed |
People
(Reporter: hsivonen, Assigned: hsivonen)
References
(Blocks 1 open bug, )
Details
(Keywords: dev-doc-complete)
Attachments
(2 files, 2 obsolete files)
5.00 KB,
patch
|
hsivonen
:
review+
|
Details | Diff | Splinter Review |
5.39 KB,
patch
|
emk
:
review+
|
Details | Diff | Splinter Review |
TextEncoder.encode() returns a DOM implementation-created buffer. This involves coping internally in the implementation (in principle, this copy could be optimized away with internal-only API changes) and then yet another copy from the returned buffer into Wasm memory (optimizing away this copy needs a Web-visible change anyway). TextEncoder.encodeInto() avoids both copies. https://bugzilla.mozilla.org/show_bug.cgi?id=1449849 combined with TextEncoder.encodeInto() is expected to avoid yet more copying. The expectation is that passing strings from JS to Wasm is / is going to be common enough to be worthwhile to optimize.
Assignee | ||
Comment 2•5 years ago
|
||
Assignee | ||
Comment 3•5 years ago
|
||
The patch requires some encoding_rs-side changes to how closely encoding_rs fills the available output space.
Assignee | ||
Comment 5•5 years ago
|
||
Comment on attachment 9035561 [details] [diff] [review]
Manual WPT sync patch
Oops. Wrong diff.
Assignee | ||
Comment 6•5 years ago
|
||
Updated•5 years ago
|
Assignee | ||
Comment 7•5 years ago
|
||
Assignee | ||
Comment 8•5 years ago
|
||
Assignee | ||
Comment 9•5 years ago
|
||
Hmm. Somehow annevk wasn't CCed yet. Anyway, attachment 9035563 [details] [diff] [review] was based on IRC discussion.
Assignee | ||
Comment 10•5 years ago
|
||
Assignee | ||
Updated•5 years ago
|
Comment 11•5 years ago
|
||
Comment on attachment 9035592 [details] [diff] [review] Implement encodeInto Review of attachment 9035592 [details] [diff] [review]: ----------------------------------------------------------------- ::: xpcom/string/nsReadableUtils.h @@ +153,5 @@ > + * The output is always valid UTF-8 ending on scalar value boundary > + * even in the case of partial conversion. > + * > + * Returns the number of code units read and the number of code > + * units written written. nit: fix typo
Assignee | ||
Comment 12•5 years ago
|
||
Thanks for the r+. Typo fixed locally. Trying Mac WPT once more with a different base revision before pushing for real:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=83a33bf4bd42008da76126847476262013cb9c86
Comment 13•5 years ago
|
||
Pushed by hsivonen@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/f7ecf49bb4c0 Implement TextEncoder.encodeInto(). r=emk. https://hg.mozilla.org/integration/mozilla-inbound/rev/3c73631c8237 test - Manually sync encoding/encodeInto.any.js from WPT. r=hsivonen.
Comment 14•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/f7ecf49bb4c0
https://hg.mozilla.org/mozilla-central/rev/3c73631c8237
Comment 15•5 years ago
•
|
||
Note to MDN team — I've added a note about this update to the 66 rel notes: https://developer.mozilla.org/en-US/docs/Mozilla/Firefox/Releases/66#APIs
When actually documenting this, you'll need to add a page for the new method, but you'll also want to consider if we need to talk about it somewhere suitable in the WebAssembly docs.
Comment 16•5 years ago
|
||
I've documented this:
Page added for encodeInto():
https://developer.mozilla.org/en-US/docs/Web/API/TextEncoder#Methods
https://developer.mozilla.org/en-US/docs/Web/API/TextEncoder/encodeInto
PR filed to add compat data entry for it:
https://github.com/mdn/browser-compat-data/pull/3581
I'm leaving WebAssembly doc additions for now, as we are aiming to do an in-depth update later on in the year anyway.
Let me know if that's OK. Thanks!
Description
•