Closed Bug 1333377 Opened 3 years ago Closed 3 years ago

Netmonitor freezes when selecting big json response

Categories

(DevTools :: Netmonitor, defect, P1)

53 Branch
defect

Tracking

(firefox53 fixed, firefox54 fixed)

RESOLVED FIXED
Firefox 54
Tracking Status
firefox53 --- fixed
firefox54 --- fixed

People

(Reporter: jdescottes, Assigned: jdescottes)

References

()

Details

Attachments

(3 files)

Attached image netmonitor-freezing.gif
Reproducible with Nightly (54.0a1 (2017-01-23)), but I guess this also applies to 53 which should be on aurora soon

STRs:

- open devtools
- open netmonitor
- navigate to http://juliandescottes.github.io/devtools-demos/netmonitor/big-json.html
- click on the data.json request

AR: Then devtools freeze for a while (around 6 seconds on a very decent laptop). 
Most of the time is spent rendering the response tab.

The properties-view component automatically expands up to 7 levels of nesting [1], so if the response contains any big array, we're very likely to hit this kind of issues.

Also note the json used here is not particularly big (20k gzipped).

[1] http://searchfox.org/mozilla-central/rev/ed04f7a44925dace34f2d2e15ef9c0f2809d70b0/devtools/client/netmonitor/shared/components/properties-view.js#28
Here's a workaround proposal which limits the number of expanded nodes to a hardcoded value (50 here).

Honza, do you think we should go for something like this or do you have anything else in mind?
Flags: needinfo?(odvarko)
Version: 52 Branch → 53 Branch
Comment on attachment 8829819 [details] [diff] [review]
bug1333377.wip.patch

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

Yes, this limitation looks good to me. It also fixes the issue reported in comment #0

It should also be uplifted to 53

Thanks for catching this!
Honza
Attachment #8829819 - Flags: review+
Flags: needinfo?(odvarko)
Comment on attachment 8829819 [details] [diff] [review]
bug1333377.wip.patch

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

Yes, this limitation looks good to me. It also fixes the issue reported in comment #0

It should also be uplifted to 53

Thanks for catching this!
Honza

Update: keeping review? since it's marked as WIP
Attachment #8829819 - Flags: review+ → review?(odvarko)
Thanks for the feedback Honza! Sent it to try : https://treeherder.mozilla.org/#/jobs?repo=try&revision=f4d99999b28f8044e95c91ca8099242be642e0ef

I'll reflag for review if I have to make test changes, otherwise I'll forward your earlier r+ and will land/uplift.
Has STR: --- → yes
Comment on attachment 8830818 [details]
Bug 1333377 - cap the number of expanded nodes in netmonitor prop view;

https://reviewboard.mozilla.org/r/107532/#review108876

Looks reasonable to me.

Honza
Attachment #8830818 - Flags: review?(odvarko) → review+
Assignee: nobody → jdescottes
Status: NEW → ASSIGNED
Pushed by jdescottes@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/316ba97fc61a
cap the number of expanded nodes in netmonitor prop view;r=Honza
https://hg.mozilla.org/mozilla-central/rev/316ba97fc61a
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 54
I have reproduced this bug with Nightly 54.0a1 (2017-01-24) (64-bit) on Windows 7 , 64 Bit !

This bug's fix is verified with latest Nightly !

Build    ID : 20170201030207
User Agent  : Mozilla/5.0(Windows NT 6.1; Win64; x64; rv:54.0) Gecko/20100101 Firefox/54.0

[bugday-20170201]
Comment on attachment 8830818 [details]
Bug 1333377 - cap the number of expanded nodes in netmonitor prop view;

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1307743 (didn't check precisely which bug of the refactor introduced the regression)
[User impact if declined]: in devtools netmonitor selecting a reponse containing a big JSON object can hang the browser
[Is this code covered by automated tests?]: No, the performance issue itself is not tested. 
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: Yes
- open devtools
- open netmonitor
- navigate to http://juliandescottes.github.io/devtools-demos/netmonitor/big-json.html
- click on the data.json request

[List of other uplifts needed for the feature/fix]: N/A
[Is the change risky?]: No.
[Why is the change risky/not risky?]: devtools ui fix only. Simply capping the number of nodes we render in a panel. 
[String changes made/needed]: N/A
Attachment #8830818 - Flags: approval-mozilla-aurora?
Comment on attachment 8830818 [details]
Bug 1333377 - cap the number of expanded nodes in netmonitor prop view;

Polish rendering time for big json in response tab. Aurora53+.
Attachment #8830818 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
I have reproduced this bug with Nightly 54.0a1 (2017-01-24) on Windows 10, 64 bit!

The bug's fix is now verified on Latest Beta 53.0b9

Build ID 	20170403072723
User Agent 	Mozilla/5.0 (Windows NT 10.0; WOW64; rv:53.0) Gecko/20100101 Firefox/53.0

[bugday-20170419]
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.