1934031 - 600ms of Parent-process jank on running inference via about:inference

Status: RESOLVED DUPLICATE of bug 1932407

(Core :: Machine Learning: General, defect)

Description

[Mayank Bansal]

Profile: https://share.firefox.dev/4eWIupQ

Maybe something to improve? Feel free to dupe to existing bugs..

Comment 1

[Mayank Bansal]

Attachment: about:support

Comment 2

[Mayank Bansal]

Attachment: inference settings.png

Comment 3

[Tarek Ziadé (:tarek)]

Interesting, thanks. We should definitely dig in this

Notice that:

• the usual number of threads is your number of cores / 2. So unless you have 32 cores, you can probably lower it and things will be much faster
• we are working on avoiding too much duplication via https://bugzilla.mozilla.org/show_bug.cgi?id=1930844 which should reduce a bit the friction

Comment 4

[Tarek Ziadé (:tarek)]

So the jank is happening because the main thread sends via IPC the model files as array buffers after they are stored into

Aristide, I remember we moved the hub instance from MLEngineChild to MLEngineParent, in https://bugzilla.mozilla.org/show_bug.cgi?id=1910116

So putting back the hub in MLEngineChild should remove this jank, at least in the Parent process

Comment 5

[Aristide Tossou]

Hi,

Since the root cause is sending the big files as array buffers via IPC, I believe this improvement: https://bugzilla.mozilla.org/show_bug.cgi?id=1932407 will fix it both in the parent process and child process. It will only leave out sending data from Javascript to wasm (handled in the ONNXPipeline). And that one will be handled later by: https://bugzilla.mozilla.org/show_bug.cgi?id=1932408

Tarek, if you can confirm this will resolve the issue in this bug, I can send a patch today for https://bugzilla.mozilla.org/show_bug.cgi?id=1932407

Comment 6

[Andrew Sutherland [:asuth] (he/him)]

I saw this in #developers, Aristide's proposal in https://bugzilla.mozilla.org/show_bug.cgi?id=1932407#c0 of "I suggest we download the model directly to a file while keeping the memory constant." sounds like what you want, but to provide some context in particular since a lot of the following is non-obvious:

• Blobs/Files can be sent over structured serialization quite efficiently in a way that ArrayBuffers cannot. Specifically, a file-backed Blob/File can be sent just as a file descriptor. More-complicated streams can be sent as an IPCStream/DataPipe which creates an IPC actor that can stream the data with backpressure.
• If you use fetch/XHR and request a Blob, the Blob will be spilled to a temporary file on disk which can make for particularly efficient transfer to the content process. A memory-backed Blob/File created via new Blob/File currently will never spill to disk (although we have a bug on file about doing that), so it will be memory-backed and sent as an IPCStream, but this is still better than an ArrayBuffer.
• If you put a Blob/File into IndexedDB, it gets stored on disk as part of the Quota storage. The Blob/File instance you put into IDB does not get a brain-transplant to reference that file, however! Your Blob/FIle is still whatever it was to start with. You need to get() the value back out of IDB to have a disk-backed Blob/File stored under storage/ and owned by IDB.
  • That said, note that IDB does a clever trick with WeakMaps so that if you put() that Blob/File multiple times, it will only be stored once in the database; IDB Blobs/Files are reference counted and so we just reuse the ref. Note that this heuristic is based on object identity, not the contents of the Blob, so this heuristic can be defeated if you start shipping the Blob between processes. But any Blob/File that is retrieved from an IDB database will be correctly refcounted if put back into the same database. (However, this is done at the level of the Blob itself, not sub-blobs in a composite blob created by new Blob([blob1, blob2, blob3]).)

Comment 7

[Tarek Ziadé (:tarek)]

Thanks for all the details Andrew!

Aristide, this sounds very promising -- could you isolate your change in a patch and we can iterate from that. Thanks

Comment 8

[BugBot [:suhaib / :marco]]

The severity field is not set for this bug.
:tarek, could you have a look please?

For more information, please visit BugBot documentation.

Comment 9

[Tarek Ziadé (:tarek)]

Fixed by https://bugzilla.mozilla.org/show_bug.cgi?id=1932407

Comment 10

[Mayank Bansal]

With the latest Nightly, this is what I get: https://share.firefox.dev/406ijaL
So the 600ms jank/high-intensity has been replaced by 20 seconds of low intentsity activity on the parent-process.

Comment 11

[Tarek Ziadé (:tarek)]

That seems "right" -- the files from the models get downloaded and streamed on disk, and we can see the memory spike occuring only inside the inference process when it's loaded back in memory.

I assume this is the first cold run and you don't get these 20 seconds in the next runs once the files are in IndexedDB?

Comment 12

[Mayank Bansal]

(In reply to Tarek Ziadé (:tarek) from comment #11)

That seems "right" -- the files from the models get downloaded and streamed on disk, and we can see the memory spike occuring only inside the inference process when it's loaded back in memory.

I assume this is the first cold run and you don't get these 20 seconds in the next runs once the files are in IndexedDB?

This is what i get on a subsequent run (after i have restarted Firefox): https://share.firefox.dev/42674Sr

Comment 13

[Tarek Ziadé (:tarek)]

Seem ok, I see a few requests that are probably head calls (because the model was main so it checks for a new version) , and then it calls the inference process, which loads directly from indexedb the files. You have used 1 single thread, so bumping it to 4 should speed the execution.

Let me know if I can close this bug

Comment 14

[Mayank Bansal]

Sounds good, thanks.

Comment 15

[Tarek Ziadé (:tarek)]

Thanks Mayank, we've appreciated your help on spotting the issue and your testing