1516197 - Debugger loads slowly

Status: RESOLVED FIXED

(DevTools :: Debugger, defect, P2)

Description

[Jason Laster [:jlast]]

It looks like we made a change lately that slows the debugger down on initial open.

STR:

1. go to a complicated page (e.g localhost:8000 for the launchpad)
2. cmd+option+i w/ debugger as the default


slow: https://perfht.ml/2RgxPzx
fast: https://perfht.ml/2GzMjq4

When I measured it in perf.html it looked like ThreadActor.onSources was pretty slow... The last change too it removed a Promise.all, which might have made the fetch non-blocking... I'm not too sure

Reverting that change seems to speed it up

> diff --git a/devtools/server/actors/thread.js b/devtools/server/actors/thread.js
> index c9c87265863e..105a70bdbe09 100644
> --- a/devtools/server/actors/thread.js
> +++ b/devtools/server/actors/thread.js
> @@ -1235,10 +1235,10 @@ const ThreadActor = ActorClassWithSpec(threadSpec, {
>      return res ? res : {};
>    },
> 
> -  onSources: function(request) {
> -    for (const source of this.dbg.findSources()) {
> +  onSources: async function(request) {
> +    await Promise.all(this.dbg.findSources().map(source => {
>        this.sources.createSourceActor(source);
> -    }
> +    }));
> 
>      // No need to flush the new source packets here, as we are sending the
>      // list of sources out immediately and we don't need to invoke the
> (END)

Comment 1

[Jason Laster [:jlast]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=0b31c70c44074c4e81b513155f615ca17d16aa04

Comment 2

[Jason Laster [:jlast]]

Attachment: fix-onsource-1.patch

Comment 3

[Jason Laster [:jlast]]

talos run to measure the performance impact

https://treeherder.mozilla.org/perf.html#/comparechooser?newProject=try&newRevision=49130805484f42e121f564780678fc4ae2624bba

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e2ba0eeb80c7
Improve Thread.onSources performance. r=bhackett

Comment 5

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/e2ba0eeb80c7

Comment 6

[Jason Laster [:jlast]]

It looks like I might have jumped the gun here... 

here is a profile of Firefox Nightly with the change -- https://perfht.ml/2Rgk5oB

It looks like `this.emit("newSource", actor)` might be the problem, perhaps this patch https://searchfox.org/mozilla-central/diff/b03057f1e8526bba8979955ddd39aee0e649b463/devtools/server/actors/utils/TabSources.js#142 should not have dropped this condition `if (!actor.source) {`?

Comment 7

[Jason Laster [:jlast]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=7c5b690d794d10b99b11e20c1b539d81e86b5db9

Comment 8

[Logan Smyth [:loganfsmyth]]

The main change from my patch is that `this.emit("newSource", actor);` used to run in the next microtask, whereas now it is running synchronously from `onSources`. That seems like the most likely issue?

Comment 9

[Logan Smyth [:loganfsmyth]]

Attachment: Bug 1516197 - Delay newSource packet processing to avoid blocking onSources. r=jlast

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1deebf9d09a8
Delay newSource packet processing to avoid blocking onSources. r=jlast

Comment 11

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/1deebf9d09a8