1815341 - Investigate IPC data transfer efficiency

Status: NEW

(Firefox :: Translations, task, P3)

Description

[Greg Tatum [:gregtatum]]

Currently the memory allocations for a language model looks like this:

1. Load the binary data in the parent process (via downloading, or from disk)
2. Send the data to the child process through an actor.
3. Pass that data to a worker.
4. Copy it into the wasm heap.

We need to reach out to the IPC team and ensure that step 2 is as efficient as possible.

Comment 1

[Greg Tatum [:gregtatum]]

I have a JSActor that is loading in very large binary blobs in the parent process, and then sending them to the content process. I want to make sure I'm doing this as efficiently as possible. For workers I can use transferables to ensure the memory is transferred and not copied. Is there any way to optimize the IPC call that is happening in the JSActor?

Example child request: https://searchfox.org/mozilla-central/rev/f078cd02746b29652c134b144f0629d47e378166/toolkit/components/translations/actors/TranslationsChild.sys.mjs#421-438

Example parent response: hhttps://searchfox.org/mozilla-central/rev/f078cd02746b29652c134b144f0629d47e378166/toolkit/components/translations/actors/TranslationsParent.sys.mjs#104-106

Comment 2

[Greg Tatum [:gregtatum]]

I asked on https://chat.mozilla.org/#/room/#fission:mozilla.org

It looks like it's not really feasible to create transferables via the JSActor mechanism. One thing to investigate is to use an input stream created in the parent process and send that down to the content process. This may be feasible to implement using the RemoteSettings infrastructure, but requires investigation.

nika on Matrix said:

You can create an input stream and send that to the content process, to read it out on the other side, or you could send the messages over native code instead of JS actors, which would have access to other things like shared memory regions

I don't think it will be particularly easy to use native code and interact with Remote Settings, but it could be something to investigate.

Comment 3

[Greg Tatum [:gregtatum]]

Here is a zoomed in view of the 183ms transfer with IPC copying: https://share.firefox.dev/3TurpKV

Here is a zoomed out view, note the GC that happens, and memory behavior of the parent process: https://share.firefox.dev/403Ieif

Comment 4

[Greg Tatum [:gregtatum]]

Current behavior:

wasm:
  file system
  [copy ->] parent process heap
  [copy ->] shared IPC memory
  [copy ->] content process memory
  [copy ->] content process worker memory
  [copy ->] wasm heap
models:
  file system
  [copy ->] parent process heap
  [copy ->] shared IPC memory
  [copy ->] content process memory
  [copy ->] content process worker memory
  [copy ->] wasm heap

Using worker transferables (Bug 1815343):

wasm:
  file system
  [copy ->] parent process heap
  [copy ->] shared IPC memory
  [copy ->] content process memory
  [copy ->] wasm heap
models:
  file system
  [copy ->] parent process heap
  [copy ->] shared IPC memory
  [copy ->] content process memory
  [copy ->] wasm heap

Using streams and WebAssembly.instantiateStreaming:

wasm:
  file system
  [stream ->] wasm heap
models:
  file system
  [stream ->] Worker memory
  [copy ->] wasm heap

Using streams, with Bergamot knowing how to use streams to load in data:

wasm:
  file system
  [stream ->] wasm heap
models:
  file system
  [stream ->] wasm heap

Comment 5

[Greg Tatum [:gregtatum]]

I'm moving this off of the Desktop MVP, since we'll probably not get to it in the next month.

Comment 6

[Greg Tatum [:gregtatum]]

In Bug 1815339 I implemented transferables for actors, so it should be feasible now to send the input stream to the translations process through sendAsyncMessage. The work here is to modify the RemoteSettings API to allow for returning a stream, and passing that via transferables.

Here rather than returning a Promise<ArrayBuffer>, we should return a Promise<Response>. The RemoteSettings API will need to be updated to support this.

I think the bergamot-translator.js generated code will need to be patched to handle a Response. That would happen here with an additional if/else check to see if it was a Response. We can't just feed it a URL like it is wanting, since we may be streaming from disk. This change needs to be handled after Bug 1863793 lands.