1449659 - make TextEncoder::Encode more efficient for large strings

Status: RESOLVED DUPLICATE of bug 1561573

(Core :: Internationalization, enhancement, P3)

Description

[Nathan Froyd [:froydnj]]

SessionWorker.js has the following code, amidst other things:

    let stateString = JSON.stringify(state);
    let data = Encoder.encode(stateString);

And the Encoder.encode call showed up in a profile of mine:

https://perfht.ml/2GgUozx

(See the event processing delay in the main process's DOM Worker thread.  If I was smarter, I would have gotten the profiler to link directly to the expensive call.)

The time under mozilla::dom::TextEncoderBinding::encode is split into two parts:

* js::CopyStringChars, which I assume is copying the into JSString into a FakeString to pass to TextEncoder::Encode; and
* TextEncoder::Encode and bits under it.

Most of the time under TextEncoder::Encode is spent actually doing the encoding, which seems completely reasonable.  But I think we could be more efficient in this case.  Encode() has:

  nsAutoCString utf8;
  nsresult rv;
  const Encoding* ignored;
  Tie(rv, ignored) = UTF_8_ENCODING->Encode(aString, utf8);
  if (NS_FAILED(rv)) {
    aRv.Throw(rv);
    return;
  }

  JSAutoCompartment ac(aCx, aObj);
  JSObject* outView = Uint8Array::Create(
    aCx, utf8.Length(), reinterpret_cast<const uint8_t*>(utf8.BeginReading()));

We wind up:

1. Encoding to `utf8`;
2. Creating a typed array;
3. Copying the newly-created string into the typed array;
4. Destroying `utf8`.

Step #3 shows up in the profile as about 1/3 of the time under TextEncoder::Encode (and the profiler has decided that memcpy is called __nss_passwd_lookup, which I don't understand).

It seems like we ought to be able to do better by encoding directly to a typed array, or at least encoding into JS-allocated memory from which we can construct the typed array with zero copying and no large intermediate allocation.  Does that seem at all reasonable?

Comment 1

[Henri Sivonen (:hsivonen)]

(In reply to Nathan Froyd [:froydnj] from comment #0)
> * js::CopyStringChars, which I assume is copying the into JSString into a
> FakeString to pass to TextEncoder::Encode; and

If it's actually UTF-16 to begin with, isn't the purpose of a FakeString to avoid the copy? I expect the JS string in actually Latin1 and the time is spent zero-extending it to UTF-16.

I want us to get to a place where we can declare two new kinds of strings on WebIDL:
 * UTF-8: Let the binding layer convert from Latin1 to UTF-8 or UTF-16 to UTF-8 depending on whether the JS string is Latin1 or UTF-16.
 * Latin1-or-UTF-16: Don't convert the string but have some mechanism that allows the C++ implementation of the WebIDL interface to query whether the string in the JS engine is Latin1 or UTF-16 and have processing paths for each without the binding layer converting or copying.

> It seems like we ought to be able to do better by encoding directly to a
> typed array, or at least encoding into JS-allocated memory from which we can
> construct the typed array with zero copying and no large intermediate
> allocation.  Does that seem at all reasonable?

The current code assumes that the creation of the Uint8Array wants to make the backing buffer allocation for the Uint8Array exact. Does the JS engine support inexact Uint8Array backing buffers? Anyway, "inexact" can be very inexact. encoding_rs doesn't do exact output allocations. It assumes the caller either works with a fixed-size intermediate buffer or with a string container whose allocation logic is similar to Rust's String or XPCOM strings (i.e. the buffer isn't exact anyway).

If we don't want reallocations of the target buffer on the JS heap, for an input of N UTF-16 code units, we'd have to allocate 3*N+1 bytes but in the best case (ASCII) only N of those would be used.

How about this:
 * Until we have a Latin1-or-UTF-16 WebIDL type, declare the argument type as "any" so that we can see the JS object.
 * Ask the JS object if it's a string and request the kind of stringification we'd normally get from the WebIDL layer if not.
 * Ask the JS string if it is UTF-16 or Latin1.
 * If it's UTF-16, allocate 3*N+1 intermediate, convert (using encoding_rs::mem) and copy to exactly-sized Uint8Array. (This is better than status quo, because currently UTF_8_ENCODING->Encode(aString, utf8) can allocate twice.)
 * If it's Latin1, check how far it's ASCII only.
 * If it's ASCII to the end, copy directly from the JS string to a Uint8Array. (The common case!)
 * If it's not ASCII all the way, allocate asciiUpTo + (N - asciiUpTo)*2, memcpy the ASCII part and do a Latin1 to UTF-8 conversion (using encoding_rs::mem) and then copy to exactly-sized Uint8Array.

Comment 2

[Henri Sivonen (:hsivonen)]

Filed bug 1449849.

Comment 3

[Emilio Cobos Álvarez (:emilio)]

I think this was fixed by bug 1561573, but please reopen if you think otherwise.