Fetch environments on demand

RESOLVED FIXED in Firefox 55

Status

()

Firefox
Developer Tools: Debugger
RESOLVED FIXED
7 months ago
7 months ago

People

(Reporter: jlast, Assigned: jlast)

Tracking

unspecified
Firefox 55
Points:
---

Firefox Tracking Flags

(firefox55 fixed)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

7 months ago
It's really common for modern web apps (react, angular, etc) to have large call stacks. This will be especially true when we include async call stacks. 

The large call stacks require significant time to serialize the forms. https://perfht.ml/2r2twqk

It would be nice if frame forms deferred serializing their environment's form until the client requested it. This can have a large performance improvement: when i tested the patch on a typical react app stepping times went from 800ms to 100ms.
(Assignee)

Comment 1

7 months ago
Created attachment 8875412 [details] [diff] [review]
skip-frames-envs-1.patch
Attachment #8875412 - Flags: review?(jimb)
(Assignee)

Comment 2

7 months ago
Created attachment 8875415 [details] [diff] [review]
skip-frames-envs-2.patch

oops, small typo
Attachment #8875412 - Attachment is obsolete: true
Attachment #8875412 - Flags: review?(jimb)
Attachment #8875415 - Flags: review?(jimb)

Comment 3

7 months ago
Comment on attachment 8875415 [details] [diff] [review]
skip-frames-envs-2.patch

Review of attachment 8875415 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, one comment.

::: devtools/server/actors/frame.js
@@ +77,5 @@
> +    // NOTE: ignoreFrameEnvironment lets the client explicitly avoid
> +    // populating form environments on pause.
> +    if (
> +      this.frame.environment &&
> +      !this.threadActor._options.ignoreFrameEnvironment

Is `this.frame` a Debugger.Frame object? If so, I would reverse the order of these two terms. Simply testing this.frame.environment causes it to build the Debugger.Environment object, so if we can avoid that when we won't use it, that would be best.
Attachment #8875415 - Flags: review?(jimb) → review+
(Assignee)

Comment 4

7 months ago
Created attachment 8875473 [details] [diff] [review]
skip-frames-envs-3.patch
Attachment #8875415 - Attachment is obsolete: true
(Assignee)

Updated

7 months ago
Keywords: checkin-needed
(Assignee)

Comment 6

7 months ago
added checkin-needed, but feel free to wait until things cool down.
Assignee: nobody → jlaster

Comment 7

7 months ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f7d5030aee45
Fetch environments on demand. r=jimb
Keywords: checkin-needed

Comment 8

7 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/f7d5030aee45
Status: NEW → RESOLVED
Last Resolved: 7 months ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
You need to log in before you can comment on or make changes to this bug.