Closed Bug 1449961 Opened 2 years ago Closed 2 years ago

Debugger release v19.4 for FF60 Beta

Categories

(DevTools :: Debugger, defect, P3)

defect

Tracking

(firefox60 fixed, firefox61 unaffected)

RESOLVED FIXED
Tracking Status
firefox60 --- fixed
firefox61 --- unaffected

People

(Reporter: jlast, Assigned: jlast)

References

Details

Attachments

(2 files, 3 obsolete files)

https://github.com/devtools-html/debugger.html/compare/release-19-3...release-19.4

Inspired by the huge performance impact here:
- https://github.com/devtools-html/debugger.html/pull/5645
- https://github.com/devtools-html/debugger.html/pull/5648

15 seconds -> 3 seconds for loading pages built with webpack eval-mode and angular development mode
Assignee: nobody → jlaster
Priority: -- → P3
Attached patch rel-19.4-align.patch (obsolete) — Splinter Review
[Feature/Bug causing the regression]: https://bugzilla.mozilla.org/show_bug.cgi?id=1440550
[User impact if declined]: debugging react & angular apps will have 15+second load times in the debugger
[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]:
[Is the change risky?]: no
[Why is the change risky/not risky?]:only affects the debugger
[String changes made/needed]:
Attachment #8963643 - Flags: review?(jdescottes)
Attachment #8963643 - Flags: approval-mozilla-beta?
Attached patch rel-19.4.patch (obsolete) — Splinter Review
same as above
Attachment #8963644 - Flags: review?(jdescottes)
Attachment #8963644 - Flags: approval-mozilla-beta?
Unfortunately we need to land 19.4-align in order to align the modules inside of the bundle for the 19.4 bundle change. Fortunately, 19.4-align is just sorting the order of the modules so it is not a risky code change.

19.4 is applying two patches that previously landed in nightly in v23 (https://bugzilla.mozilla.org/show_bug.cgi?id=1446234) two weeks ago. It is also well tested by our mochitests.

Here is a new profile with the improved performance (~4 seconds)
https://perfht.ml/2E4opg1
Comment on attachment 8963643 [details] [diff] [review]
rel-19.4-align.patch

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

LGTM, only moving bundles around.
Attachment #8963643 - Flags: review?(jdescottes) → review+
Attached patch rel-19.4-align-2.patch (obsolete) — Splinter Review
Attachment #8963643 - Attachment is obsolete: true
Attachment #8963644 - Attachment is obsolete: true
Attachment #8963643 - Flags: approval-mozilla-beta?
Attachment #8963644 - Flags: review?(jdescottes)
Attachment #8963644 - Flags: approval-mozilla-beta?
Attachment #8963743 - Flags: review?(jdescottes)
Attachment #8963743 - Flags: approval-mozilla-beta?
Attached patch rel-19.4-2.patchSplinter Review
Attachment #8963744 - Flags: review?(jdescottes)
Attachment #8963744 - Flags: approval-mozilla-beta?
Attachment #8963743 - Attachment is obsolete: true
Attachment #8963743 - Flags: review?(jdescottes)
Attachment #8963743 - Flags: approval-mozilla-beta?
Attachment #8963751 - Flags: review?(jdescottes)
Attachment #8963751 - Flags: approval-mozilla-beta?
Attachment #8963744 - Flags: review?(jdescottes) → review+
Attachment #8963751 - Flags: review?(jdescottes) → review+
Comment on attachment 8963744 [details] [diff] [review]
rel-19.4-2.patch

Glad to hear that the scary-big patch is just a giant no-op and this is the only functional change. Approved for 60.0b9.
Attachment #8963744 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #8963751 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
(In reply to Jason Laster [:jlast] from comment #2)
> [Is this code covered by automated tests?]: yes
> [Has the fix been verified in Nightly?]: yes
> [Needs manual test from QE? If yes, steps to reproduce]: no

Setting qe-verify- based on Jason's assessment on manual testing needs and the fact that this fix has automated tests.
Flags: qe-verify-
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.