1710884 - Add IPC glue code for self-hosted XDR content [Android]

Status: RESOLVED FIXED

(Core :: IPC, task)

Description

[Nicolas B. Pierron [:nbp] {away}]

Fork of the second part of Bug 1698045.

Comment 1

[Nicolas B. Pierron [:nbp] {away}]

Attachment: WIP: Bug 1710884 - Use shared memory for JS initialization on Android.

Comment 2

[Nicolas B. Pierron [:nbp] {away}]

Note to self, the self-hosted shared-memory is optional and can be toggle by a preference in about:config. Due to the not-yet-implemented-highly-needed-cleanup required by Android, we might have to pass a fake fd (-1) even when the content is not present, to satisfy the fact that Android matches fd to their purpose by looking up at specific indexes in the fd-array.

Comment 3

[Chris Peterson [:cpeterson]]

Ted Campbell says we should fix this bug for Android Fission because it will reduce memory usage, probably about 400 kB per process. See bug 1710987 comment 5 for the memory improvements seen on desktop.

Comment 4

[Markus Stange [:mstange]]

Randell, I saw you updated the patch recently - did you push it to try and see if the failures / timeouts are still happening?

Comment 5

[Nicolas B. Pierron [:nbp] {away}]

I have no intent to push these changes forward, if anybody more knowledgeable about GeckoView development is willing to have a look, feel free to take over the patch.

This change is expected to bring a ~40ms improvement (Bug 1618391) to the start of the content process(-es), by using what the parent process already computed.

Comment 6

[Dave Hunt [:davehunt] [he/him] ⌚BST]

The Performance Impact Calculator has determined this bug's performance impact to be low. If you'd like to request re-triage, you can reset the Performance Impact flag to "?" or needinfo the triage sheriff.

Platforms: Android
Impact on browser: Causes noticeable startup delay

Comment 7

[Randell Jesup [:jesup] (needinfo me)]

I have a copy of this patch locally, but it's low on my personal queue

Comment 8

[[:owlish] 🦉 PST]

Hey Dave, you mentioned that the performance calculator suggests the performance impact of this bug will be low. How does that calculator work? Has it proved to be reliable?

Comment 9

[Dave Hunt [:davehunt] [he/him] ⌚BST]

(In reply to [:owlish] 🦉 PST from comment #8)

Hey Dave, you mentioned that the performance calculator suggests the performance impact of this bug will be low. How does that calculator work? Has it proved to be reliable?

Hey. You can find details on the calculator at https://wiki.mozilla.org/Performance/Triage#Triage_calculator. Since this bug was triaged we have updated the calculator so that Android has the same weight as Windows. This upgrades the impact from low to medium.

The Performance Impact Calculator has determined this bug's performance impact to be medium. If you'd like to request re-triage, you can reset the Performance Impact flag to "?" or needinfo the triage sheriff.

Platforms: Android
Impact on browser: Causes noticeable startup delay

Comment 10

[Dave Hunt [:davehunt] [he/him] ⌚BST]

I should add that the performance triage and impact calculator are more suited to defects than tasks.

Comment 11

[Matthew Gaudet (he/him) [:mgaudet]]

Attachment: Bug 1710884 - Add JS_LOG and timing to JS engine startup to get insight into XDR decode. r?nbp

Comment 12

[Matthew Gaudet (he/him) [:mgaudet]]

Attachment: Bug 1710884 - Enable Android to use the self-hosted shared-memory cache for process startup r?nika

I have manually verified that this works on geckoview-example on a local Pixel 8 via logging

This was inspired by D114126, but it appears that almost all the machinery in that patch
was obviated by Bug 1440207.

I am unable to measure via our performance testing framework a performance win here, however
using local timing on a Pixel 8 I seem to be seeing at least a 100ms improvement in content
process self-hosted initialization time, at a cost of 3ms of encode time.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/573319cc3d32
Add JS_LOG and timing to JS engine startup to get insight into XDR decode. r=nbp
https://hg.mozilla.org/integration/autoland/rev/756ee35c4f48
Enable Android to use the self-hosted shared-memory cache for process startup r=nika
https://hg.mozilla.org/integration/autoland/rev/0038d2ef62be
apply code formatting via Lando

Comment 14

[Serban Stanca [:SerbanS]]

Backed out for causing build bustages in SelfHosting.cpp.

• Backout link
• Push with failures
• Failure Log
• Failure line: /builds/worker/checkouts/gecko/js/src/vm/SelfHosting.cpp:2652:23: error: use of undeclared identifier 'Info'

Comment 15

[Matthew Gaudet (he/him) [:mgaudet]]

Raced with Bug 1943485.

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/14e8eb30d5b1
Add JS_LOG and timing to JS engine startup to get insight into XDR decode. r=nbp
https://hg.mozilla.org/integration/autoland/rev/00c0f7f77f54
Enable Android to use the self-hosted shared-memory cache for process startup r=nika
https://hg.mozilla.org/integration/autoland/rev/3780ceaa56c5
apply code formatting via Lando

Comment 17

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/14e8eb30d5b1
https://hg.mozilla.org/mozilla-central/rev/00c0f7f77f54
https://hg.mozilla.org/mozilla-central/rev/3780ceaa56c5