Open Bug 1393086 Opened 2 years ago Updated 6 months ago

Try to optimize toolbox startup by running more code in parallel between parent and content process

Categories

(DevTools :: General, enhancement, P3)

enhancement

Tracking

(Not tracked)

People

(Reporter: ochameau, Unassigned)

References

(Depends on 1 open bug, Blocks 2 open bugs)

Details

Attachments

(1 file)

bug 1378734 comment 1 highlights that toolbox initialization codepath is very sequential between parent and content process. In most case, we often wait for the other process to compute something before proceeding with the next initialization step.

On profile from attachment 8900307 [details], we should try to run steps 12,13,18-20,22,23 happening in the content process while some others from the parent process are still running.

Here is some details on these steps:
12: loadFrameScript(child.js)
13: receive debug:connect message and RootActor.getTab() > load ExtensionContent.jsm

18: require(actors/styles)
19: require(actors/highlighters)
20: getHighlighter

22: getCSSDatabase
23: getPageStyle

1) We may try to load child.js earlier (move step 12 to earlier).
It is being done during Group C, from here:
http://searchfox.org/mozilla-central/source/devtools/server/main.js#1030

2) We may try to load inspector actor earlier (execute steps 18 to 20 earlier).
Or, try to avoid blocking on inspector front readyness from inspector client code.

3) Steps 22 and 23 may already be running while things are computing in the parent, we we should verify that.
Comment on attachment 8900313 [details]
Bug 1393086 - Parallelize toolbox and child process server initializations.

This patch is quite stressful as it may introduce various unexpected race in client code, but DAMP says it speeds up tools loading by 10%
https://treeherder.mozilla.org/perf.html#/comparesubtest?originalProject=try&originalRevision=46cd72b4e7694034efd17b011bec92a1ba0ad831&newProject=try&newRevision=caa114d3e8127f9910b338c5e166d0520a7d0bfa&originalSignature=edaec66500db21d37602c99daa61ac983f21a6ac&newSignature=edaec66500db21d37602c99daa61ac983f21a6ac&showOnlyImportant=1&framework=1

And there isn't that many tests failing:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=caa114d3e8127f9910b338c5e166d0520a7d0bfa

If anyone want to test this patch and see if there is a substancial win.
I tried to run a custom test locally, and also just open toolboxes manually,
and can see for sure a win.
Thanks for taking a look Alex. I am very invested in continuing your work and will continue to play around with it.
Blocks: 1378734
With a local test focusing on first load of the inspector I see only a 0.7% win.
If anyone has some time to look at performances of this patch, I would like to confirm it brings value before proceeding.
May be this patch improve all but first load?

DAMP is expected to ignore first-load-only improvements as it toggles the toolbox many times for each run.
But even when toggling it once (https://hg.mozilla.org/try/rev/1ec7988042a98e7e8410b4fc50cdee7481592152),
it still reports significant wins:
https://treeherder.mozilla.org/perf.html#/comparesubtest?originalProject=try&originalRevision=7e70aa85947eedfac3dd2223f7efc3225c0a0696&newProject=try&newRevision=68e081b0b5b46f43fac7994977fe217caf0517e6&originalSignature=edaec66500db21d37602c99daa61ac983f21a6ac&newSignature=edaec66500db21d37602c99daa61ac983f21a6ac&filter=open&framework=1

Also, now DAMP is special as it loads devtools-browser.js itself:
http://searchfox.org/mozilla-central/source/testing/talos/talos/tests/devtools/addon/content/damp.js#1
It mess up with lazy load of devtools, it basically cancels the efforts from bug 1359855.
It doesn't reflect what users have, the first time they open the tools, we will have to load all framework modules first,
whereas on DAMP, they will already be loaded before running the test, by damp.js.

We can't decently work on performance and only follows DAMP results if DAMP is wrong like that.
On bug 1392602 I get the opposite, DAMP reports no win, whereas I see a 3-4% win locally.
Depends on: 1394804
(In reply to Alexandre Poirot [:ochameau] from comment #4)
> With a local test focusing on first load of the inspector I see only a 0.7%
> win.
> If anyone has some time to look at performances of this patch, I would like
> to confirm it brings value before proceeding.
> May be this patch improve all but first load?
> 
> DAMP is expected to ignore first-load-only improvements as it toggles the
> toolbox many times for each run.
> But even when toggling it once
> (https://hg.mozilla.org/try/rev/1ec7988042a98e7e8410b4fc50cdee7481592152),
> it still reports significant wins:
> https://treeherder.mozilla.org/perf.html#/
> comparesubtest?originalProject=try&originalRevision=7e70aa85947eedfac3dd2223f
> 7efc3225c0a0696&newProject=try&newRevision=68e081b0b5b46f43fac7994977fe217caf
> 0517e6&originalSignature=edaec66500db21d37602c99daa61ac983f21a6ac&newSignatur
> e=edaec66500db21d37602c99daa61ac983f21a6ac&filter=open&framework=1

I believe config.repeat is 1 in either case - do you not see similar gains if you remove those commented lines from the base revision?

> Also, now DAMP is special as it loads devtools-browser.js itself:
> http://searchfox.org/mozilla-central/source/testing/talos/talos/tests/
> devtools/addon/content/damp.js#1
> It mess up with lazy load of devtools, it basically cancels the efforts from
> bug 1359855.
> It doesn't reflect what users have, the first time they open the tools, we
> will have to load all framework modules first,
> whereas on DAMP, they will already be loaded before running the test, by
> damp.js.
> 
> We can't decently work on performance and only follows DAMP results if DAMP
> is wrong like that.
> On bug 1392602 I get the opposite, DAMP reports no win, whereas I see a 3-4%
> win locally.

As you mention here and as discussed in Bug 1394804 it's difficult for DAMP to capture the initial startup time so we may need to come up with another way to measure this if we want to track it.
(In reply to Brian Grinstead [:bgrins] from comment #5)
> (In reply to Alexandre Poirot [:ochameau] from comment #4)
> > We can't decently work on performance and only follows DAMP results if DAMP
> > is wrong like that.
> > On bug 1392602 I get the opposite, DAMP reports no win, whereas I see a 3-4%
> > win locally.

FWIW I think the 'warm startup' case is still more important since people are oftern opening the tools more than one time per process.  And I'd also expect the performance to track closely with cold startup (minus a constant for loading the modules). What are the timings you see locally for warm vs cold startup?
Priority: -- → P3
Depends on: 1397341
Severity: normal → enhancement
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.