Closed Bug 1514664 Opened 6 years ago Closed 6 years ago

Implement TextEncoder.encodeInto() for faster JS/Wasm interop


(Core :: Internationalization, enhancement)

Not set



Tracking Status
firefox66 --- fixed


(Reporter: hsivonen, Assigned: hsivonen)


(Blocks 1 open bug, )


(Keywords: dev-doc-complete)


(2 files, 2 obsolete files)

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. 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.
Method needs adding to MDN.
Keywords: dev-doc-needed
The patch requires some encoding_rs-side changes to how closely encoding_rs fills the available output space.
Depends on: 1515351
Attached patch Manual WPT sync patch (obsolete) — Splinter Review

Test by annevk. r=hsivonen.

Attachment #9035561 - Flags: review+

Comment on attachment 9035561 [details] [diff] [review]
Manual WPT sync patch

Oops. Wrong diff.

Attachment #9035561 - Attachment is obsolete: true
Attachment #9035561 - Flags: review+
Attachment #9035563 - Flags: review+
Attachment #9031909 - Attachment is obsolete: true

Hmm. Somehow annevk wasn't CCed yet. Anyway, attachment 9035563 [details] [diff] [review] was based on IRC discussion.

Attachment #9035592 - Flags: review?(VYV03354)
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
Attachment #9035592 - Flags: review?(VYV03354) → review+

Thanks for the r+. Typo fixed locally. Trying Mac WPT once more with a different base revision before pushing for real:

Pushed by
Implement TextEncoder.encodeInto(). r=emk.
test - Manually sync encoding/encodeInto.any.js from WPT. r=hsivonen.
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66
See Also: → 1519523
See Also: 1519523

Note to MDN team — I've added a note about this update to the 66 rel notes:

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.

I've documented this:

Page added for encodeInto():

PR filed to add compat data entry for it:

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!

You need to log in before you can comment on or make changes to this bug.