1143224 - Opening the netmonitor slows down requests on the page

Status: RESOLVED FIXED

(DevTools :: Netmonitor, defect, P1)

Description

[Andrei Petcu]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:36.0) Gecko/20100101 Firefox/36.0
Build ID: 20150305221847

Steps to reproduce:

1) install this js app from web
git clone [e-mail]:cegeka/library_app.git
cd library_app
git checkout 03cb865
cd spa
npm install -g grunt
bower install
npm install
grunt server

2) open app in Firefox and Chrome (fresh profile, addons disabled and safe mode) and point both browsers to http://localhost:8000/

3) go to network tab for both, clear the results and edit the file "library_app/spa/app/index.html" Each edit reloads all the files in both browsers. In FF it is much slower than in Chromium 2.27 seconds vs 268 miliseconds

Please check this video out http://youtu.be/qS7_IRrFBQA


Actual results:

Firefox loads the files much slower than Chrome. This is a normal web developer workflow. Each time I hit save in my source code this happens and sometimes it even goes to 5 seconds. I seen this happen even with fewer 404 requests (this example and this commit was specially crafted for this bug but this is common on a normal project).

Also Firefox loads the same file several times. There are more requests on FF than on Chromium.


Expected results:

Firefox should load files in a few houndred miliseconds

Comment 1

[Andrei Petcu]

I managed to reproduce this with my work project that unfortunately I cannot share :( The difference between the example above and this one is the fact that it does not return lots of 404 requests, but lots of 304. Check out the video http://youtu.be/yhvLICI_8MI

Firefox loads the files in 2 seconds and Chromium in 478 miliseconds. This time Chromium has 2 more requests than Firefox and Firefox has no duplicate requests.

Comment 2

[Andrei Petcu]

Please check out commit e8fe568 from library_app I put duplicate JS files in order not to get lots of requests. It appears not to be related to the status code of the response.

Comment 3

[Andrei Petcu]

New insight!! :D If I open the developer tools the page loads slow but if I close the DevTools it is blazing fast just as Chromium :D Check out the video http://youtu.be/Dg_cSrUDbcs

Comment 4

[Andrei Petcu]

If I disable the Network Monitor tool the loading is very fast so the problem must be there.

Comment 5

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Are you using e10s in Firefox?  What version of Firefox?

Comment 6

[Andrei Petcu]

All of them. 36, 38 and 39 (I think 37 has it also but I don't run Beta :P )

Comment 7

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

What about e10s?  Is it enabled?

Comment 8

[Andrei Petcu]

With nightly and e10s enabled I get the exact results as with other versions.

Comment 9

[Andrei Petcu]

https://www.youtube.com/watch?v=Hif4LjfMDk0

Comment 10

[Andrei Petcu]

It seems that if I have "Flash" tab selected or "Media" it is a little faster than if I have "All" tab selected.

Comment 11

[Andrei Petcu]

Hmmm... I don't have e10s in the video I posted because I also selected "restart with addons disabled/start in safe mode". I just tried again with e10s and no safe mode and the problem persists. Please tell me if you have trouble reproducing it.

Comment 12

[Brian Grinstead [:bgrins]]

Attachment: recording-1143224.json

Recording taken with timeline tool in Browser Toolbox after following STR

Comment 13

[Brian Grinstead [:bgrins]]

Attachment: loading-soundcloud-netmonitor.png

Seems related: recording taken when loading soundcloud.com with the netmonitor enabled.  In this case the call to SideMenuWidget.insertItemAt dominates JS time during the jank.

Comment 14

[Brian Grinstead [:bgrins]]

It does appear that the netmonitor frontend is causing jank.  I notice that if I comment out the SideMenuGroup.insertSelfAt functionality the load time is noticeably improved: https://dxr.mozilla.org/mozilla-central/source/browser/devtools/shared/widgets/SideMenuWidget.jsm?from=sidemenuwidget.jsm#477-483.  It still needs further performance analysis, but the netmonitor frontend is pretty clearly affecting the load time of resources on the page.

Comment 15

[Victor Porof [:vporof][:vp]]

(In reply to Brian Grinstead [:bgrins] from comment #13)
> Created attachment 8580125 [details]
> loading-soundcloud-netmonitor.png
> 
> Seems related: recording taken when loading soundcloud.com with the
> netmonitor enabled.  In this case the call to SideMenuWidget.insertItemAt
> dominates JS time during the jank.

This graph is misleading, and we're working to fix that. Looks like the profiler's circular buffer filled up, and in turn skewed the first sample to cover the overflow.

To fix, just make shorter recordings for now :)

It's bad, but we're working on it. Bug 1145187

Comment 16

[Andrei Petcu]

I jus put a bounty on the bug you mentioned :D Hope this helps https://www.bountysource.com/issues/10463218-handle-when-profiler-exceeds-circular-buffer/backers

Comment 17

[Brian Grinstead [:bgrins]]

This can be reproduced easily on any page that has a lot of requests.  For example, https://soundcloud.com/explore

Comment 18

[Brian Grinstead [:bgrins]]

May depend on 1149634 if it requires frontend changes to netmonitor

Comment 20

[Panos Astithas (he/him) [:past] (please ni?)]

Bug 1156912 has simpler STR.

Comment 21

[Brian Grinstead [:bgrins]]

Attachment: debugging-netmonitor.patch

This is just a frontend change that tries to scroll and access height / scroll properties a bit less often

Comment 22

[Brian Grinstead [:bgrins]]

I made a demo page to make testing a bit easier: https://bgrins.github.io/devtools-demos/misc/requests.html

Comment 23

[Sam H]

Pretty intensive demo page. When the Network pane is open, FF totally locks up for me.

Obviously this is an issue of rendering data to the Network pane while loading.

Comment 24

[Brian Grinstead [:bgrins]]

(In reply to Samuel from comment #23)
> Pretty intensive demo page. When the Network pane is open, FF totally locks
> up for me.

Sorry, I will bump down the number of requests a bit, don't want to be causing any lock ups.  Adjusting from 1000 requests to 750.

Comment 25

[Sam H]

That works! Wow, about 54 seconds vs 12 seconds. You could probably even lower it to 500 and still see a measurable difference. But it works now.

Comment 26

[Brian Grinstead [:bgrins]]

Here is some output I'm seeing from local builds:

With devtools closed:

Progress: appending 750 out of 750
Progress: loaded 750 out of 750
All loads took 7502 ms

Without the patch applied and devtools opened:

Progress: appending 750 out of 750
Progress: loaded 750 out of 750
All loads took 30974 ms

With attachment 8596214 [details] [diff] [review] applied and devtools opened:

Progress: appending 750 out of 750
Progress: loaded 750 out of 750
All loads took 14273 ms

There's of course some variation between loads but the general theme is the same, at least for this test page:

no devtools -> 2x faster than netmonitor with this simple patch -> 2x faster than netmonitor currently

Comment 27

[Brian Grinstead [:bgrins]]

Attachment: netmonitor-profile.json.zip

Did some profiling on this with the patch applied with fitzgen.  Here's the results.  Found a couple of simple(ish) things that were taking a small amount of time(zebra striping for the UI, event emitter logging is checking a pref every time).  Have a couple of other ideas for the remaining frontend jank with ways to batch up requests that are sent from the controller to the view.

Comment 28

[Brian Grinstead [:bgrins]]

Attachment: netmonitor-scrolling.patch

A patch that updates the SideMenuWidget to use getBoundsWithoutFlushing and a DeferredTask to recalculate styles and scroll much less often, which fixes the bulk of the jank

Comment 29

[Brian Grinstead [:bgrins]]

Attachment: netmonitor-scrolling.patch

Batch up scrolls and do them all at once, and use getBoundsWithoutFlushing to determine whether the list is scrolled to bottom.  Changed the test file to use add_task and needed to make a few adjustments to that so it could accommodate the async scrolling.  Victor, I'm flagging you since I don't know who else can review this, but feel free to redirect.

Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=7026bb8120d6

Comment 30

[Victor Porof [:vporof][:vp]]

Ooh, TIL!

Comment 31

[Jeff Griffiths (:canuckistani) (:⚡︎)]

Tested this locally with the try build:

1. with netmonitor open: 18089, 14264, 16666
2. with netmonitor never opened, console open & net requests visible: 10222, 10760, 10702  
3. with no tools: 10489, 10380, 10391

Looks good! Ship it. I logged bug 1162770 because I observed that even when we're just as fast as chrome, we jank the page animations.

Comment 32

[Brian Grinstead [:bgrins]]

(In reply to Jeff Griffiths (:canuckistani) from comment #31)
> Tested this locally with the try build:
> 
> 1. with netmonitor open: 18089, 14264, 16666
> 2. with netmonitor never opened, console open & net requests visible: 10222,
> 10760, 10702  
> 3. with no tools: 10489, 10380, 10391
> 
> Looks good! Ship it. I logged bug 1162770 because I observed that even when
> we're just as fast as chrome, we jank the page animations.

There is still definitely room for improvement even with the patch applied.  We still slow down the requests when the netmonitor is opened.

From some testing, the next thing that looks promising would be to make _onNetworkEvent async, queue up events to send to the list, and dispatch them all at once.  AFAICT anything sync that is happening there (like NetMonitorView.RequestsMenu.addRequest) is janking the request time.

It's surprising *how* slow inserting items into this list is given the small number of requests (< 1000).  I think it's the flex layout on the netmonitor view that's contributing to that, because I had a test case that did the same kind of scrolling / inserting behavior that this does, but with a more simple DOM and it had no problem keeping up even with 10s of thousands of elements.

Comment 33

[Brian Grinstead [:bgrins]]

Attachment: async-add-queue.patch

An idea of how to process added requests in batches.  Seriously doubt that this will pass tests, but here's a try push with both patches for a performance comparison: https://treeherder.mozilla.org/#/jobs?repo=try&revision=8e8f7f762e00

Comment 34

[Sam H]

So, when will we see this fix? (in which version of Firefox) I wonder if this can somehow get pushed to the front instead of waiting till version 40 or 41.

Comment 35

[Victor Porof [:vporof][:vp]]

Comment on attachment 8603074 [details] [diff] [review]
async-add-queue.patch

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

Very cool

Comment 36

[Victor Porof [:vporof][:vp]]

(In reply to Victor Porof [:vporof][:vp] from comment #35)
> Comment on attachment 8603074 [details] [diff] [review]

Note that there are already some tiny mechanisms inside netmonitor-view.js that debounce rendering, but that stuff deals with a whole lot more (massaging packets, creating data sources, fetching additional information from the server), so I like seeing some more debouncing in this widget (especially since it's used in many other places, e.g. the debugger, shader editor).

Comment 37

[Brian Grinstead [:bgrins]]

(In reply to Jeff Griffiths (:canuckistani) from comment #31)
> Tested this locally with the try build:
> 
> 1. with netmonitor open: 18089, 14264, 16666
> 2. with netmonitor never opened, console open & net requests visible: 10222,
> 10760, 10702  
> 3. with no tools: 10489, 10380, 10391
> 
> Looks good! Ship it. I logged bug 1162770 because I observed that even when
> we're just as fast as chrome, we jank the page animations.

Jeff, can you grab a build from here: https://treeherder.mozilla.org/#/jobs?repo=try&revision=844c4b6bf387 and see how it compares with your measurements?

Comment 38

[Brian Grinstead [:bgrins]]

Attachment: async-add-queue.patch

Updated version of this patch that calls refreshSummary() and refreshZebra() less often and also fixes a couple of existing bugs where the summary is reported incorrectly during requests.

Comment 39

[Brian Grinstead [:bgrins]]

(In reply to Victor Porof [:vporof][:vp] from comment #36)
> (In reply to Victor Porof [:vporof][:vp] from comment #35)
> > Comment on attachment 8603074 [details] [diff] [review]
> 
> Note that there are already some tiny mechanisms inside netmonitor-view.js
> that debounce rendering, but that stuff deals with a whole lot more
> (massaging packets, creating data sources, fetching additional information
> from the server), so I like seeing some more debouncing in this widget
> (especially since it's used in many other places, e.g. the debugger, shader
> editor).

Was this meant to be a comment on the other patch (Attachment 8602832 [details] [diff])?  8603074 is the one that modifies netmonitor-view.js, not the side menu widget

Comment 40

[Brian Grinstead [:bgrins]]

(In reply to Brian Grinstead [:bgrins] from comment #38)
> and also fixes a couple of existing bugs where the summary is
> reported incorrectly during requests.

Thought about this more, and I should break those changes out into a different patch/bug.  They don't have anything to do with the performance

Comment 41

[Brian Grinstead [:bgrins]]

(In reply to Brian Grinstead [:bgrins] from comment #40)
> (In reply to Brian Grinstead [:bgrins] from comment #38)
> > and also fixes a couple of existing bugs where the summary is
> > reported incorrectly during requests.
> 
> Thought about this more, and I should break those changes out into a
> different patch/bug.  They don't have anything to do with the performance

Filed Bug 1163092 for this

Comment 42

[Jeff Griffiths (:canuckistani) (:⚡︎)]

(In reply to Brian Grinstead [:bgrins] from comment #37)
> Jeff, can you grab a build from here:
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=844c4b6bf387 and see
> how it compares with your measurements?

Subjectively it looks much much better, there is almost no jank in the text animation, and times are improved as well:

Without toolbox open: 7507ms, 7506ms, 7506ms

With netmonitor open: 10289ms, 10764ms, 11019ms

Interactivity with the toolbox and content is locked briefly up as the toolbox finishes rendering all the results - that's the last less-than-ideal thing I notice.

Comment 43

[Brian Grinstead [:bgrins]]

(In reply to Jeff Griffiths (:canuckistani) from comment #42)
> (In reply to Brian Grinstead [:bgrins] from comment #37)
> > Jeff, can you grab a build from here:
> > https://treeherder.mozilla.org/#/jobs?repo=try&revision=844c4b6bf387 and see
> > how it compares with your measurements?
> 
> Subjectively it looks much much better, there is almost no jank in the text
> animation, and times are improved as well:
> 
> Without toolbox open: 7507ms, 7506ms, 7506ms
> 
> With netmonitor open: 10289ms, 10764ms, 11019ms
> 
> Interactivity with the toolbox and content is locked briefly up as the
> toolbox finishes rendering all the results - that's the last less-than-ideal
> thing I notice.

OK great, I still need to do some work to get tests passing with the async-add-queue patch but it sounds like Victor liked the general idea so that's promising.  The scrolling changes could land ASAP (pending review), as those are passing tests

Comment 44

[Brian Grinstead [:bgrins]]

(In reply to Samuel from comment #34)
> So, when will we see this fix? (in which version of Firefox) I wonder if
> this can somehow get pushed to the front instead of waiting till version 40
> or 41.

We will ship these in 40 for sure (whether they land in 41 or not we will request uplift).  I think the first patch (which makes a very big improvement) may be a good candidate for uplifting to beta - we'll know better once we get it landed.

Comment 45

[Jeff Griffiths (:canuckistani) (:⚡︎)]

(In reply to Samuel from comment #34)
> So, when will we see this fix? (in which version of Firefox) I wonder if
> this can somehow get pushed to the front instead of waiting till version 40
> or 41.

These changes will debut in Dev Edition 40 - if we uplift after merge next week you'll see that by continuing to follow this bug. We will not be uplifting to beta 39 or release 38. You're of course encouraged to use Dev Edition for early access to bug fixes such as this one - there are a LOT of improvements going into 40.

Comment 46

[Victor Porof [:vporof][:vp]]

Comment on attachment 8602832 [details] [diff] [review]
netmonitor-scrolling.patch

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

Nice!

Comment 47

[Brian Grinstead [:bgrins]]

remote:   https://hg.mozilla.org/integration/fx-team/rev/ae102895f6e9

Comment 48

[Phil Ringnalda (:philor)]

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

Comment 49

[Brian Grinstead [:bgrins]]

Attachment: netmonitor-async-add-pt1.patch

This is part 1 - a mostly mechanical change to move the addRequest logic into flushRequests.  Luckily the view already had async rendering in place (for updates).

There is a second patch I've split out that does various optimizations and test fixes.  They are meant to be folded together for landing

Comment 50

[Jordan Santell [:jsantell] [@jsantell]]

Comment on attachment 8604359 [details] [diff] [review]
netmonitor-async-add-pt1.patch

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

::: browser/devtools/netmonitor/netmonitor-view.js
@@ +517,3 @@
>      }
> +
> +    this._flushRequestsTask.arm();

Maybe we should disarm this in destruction so we don't have any lingering events going on

Comment 51

[Brian Grinstead [:bgrins]]

(In reply to Jordan Santell [:jsantell] [@jsantell] from comment #50)
> Comment on attachment 8604359 [details] [diff] [review]
> netmonitor-async-add-pt1.patch
> 
> Review of attachment 8604359 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: browser/devtools/netmonitor/netmonitor-view.js
> @@ +517,3 @@
> >      }
> > +
> > +    this._flushRequestsTask.arm();
> 
> Maybe we should disarm this in destruction so we don't have any lingering
> events going on

Good idea, I've added that

Comment 52

[Brian Grinstead [:bgrins]]

Attachment: netmonitor-async-add-pt1-r=jsantell.patch

Disarm the task on destroy

Comment 53

[Brian Grinstead [:bgrins]]

Attachment: netmonitor-async-add-pt2.patch

The changes here:

1) Fixing the waitForNetworkEvents helper function for tests.  Right now this fires early sometimes, since it doesn't take into account dead requests for RECEIVED events.  In order to make that work, I'm waiting to fire these events until after the update queue has been processed (which is when dead requests are detected).
2) Adding an event that fires when a request is added and using that in a couple tests as necessary (if they weren't using waitForNetworkEvents)
3) A few optimizations (getting rid of unnecessary calls to refresh zerbra striping and summary stats for every new request since that can fire once at the end of flushRequests)

Still awaiting try results (https://treeherder.mozilla.org/#/jobs?repo=try&revision=1824d4cad377), but everything is passing locally.

As we discussed on IRC, I'm not sure offhand how much this will interfere with the refactoring in Bug 1149634, but I'm hoping we can at least land (1) ASAP since I think it may fix Bug 1163775 and maybe some other intermittent issues.  I can break that part separately if that makes things easier.

Comment 54

[Jordan Santell [:jsantell] [@jsantell]]

Comment on attachment 8604376 [details] [diff] [review]
netmonitor-async-add-pt2.patch

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

Looks good -- starting to get worried about one by one, retroactively adding these to the refactor patch, and wonder if there's a solution we can keep up to date as more patches for netmonitor land

::: browser/devtools/netmonitor/netmonitor-controller.js
@@ +569,5 @@
>            transferredSize: networkInfo.response.transferredSize,
>            mimeType: networkInfo.response.content.mimeType
>          });
>          this.webConsoleClient.getResponseContent(actor, this._onResponseContent);
> +        window.emit(EVENTS.UPDATING_RESPONSE_CONTENT, actor);

Do none of these events use the URL?

@@ +588,5 @@
>     * @param object aResponse
>     *        The message received from the server.
>     */
>    _onRequestHeaders: function(aResponse) {
>      NetMonitorView.RequestsMenu.updateRequest(aResponse.from, {

Good catch with these. I'd say let's have these be promises, but due to the queue, maybe it makes more sense for these to be CBs.

::: browser/devtools/netmonitor/test/browser_net_details-no-duplicated-content.js
@@ +94,5 @@
>  
>      info("Waiting for NETWORK_EVENT");
>      yield onNetworkEvent;
>  
> +    if (!RequestsMenu.getItemAtIndex(0)) {

space between function name and parens

Comment 55

[Brian Grinstead [:bgrins]]

(In reply to Jordan Santell [:jsantell] [@jsantell] from comment #54)
> Comment on attachment 8604376 [details] [diff] [review]
> netmonitor-async-add-pt2.patch
> 
> Review of attachment 8604376 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Looks good -- starting to get worried about one by one, retroactively adding
> these to the refactor patch, and wonder if there's a solution we can keep up
> to date as more patches for netmonitor land

We definitely need to just knock that patch out.  In this case, I can rebase it on top of these patches to keep it updated.

> ::: browser/devtools/netmonitor/netmonitor-controller.js
> @@ +569,5 @@
> >            transferredSize: networkInfo.response.transferredSize,
> >            mimeType: networkInfo.response.content.mimeType
> >          });
> >          this.webConsoleClient.getResponseContent(actor, this._onResponseContent);
> > +        window.emit(EVENTS.UPDATING_RESPONSE_CONTENT, actor);
> 
> Do none of these events use the URL?

I started passing this in just today for Bug 1162961, but realized it'd be easier to fetch the URL directly from the webConsoleClient in waitForNetworkEvents (the events are just used for tests).  There's actually a logging bug that got introduced there where progress isn't going to be correctly reported in the test logs for the RECEIVED events since they aren't passing in [actor,url] right now.  This was before I fully understood how these events were working.  Fetching from the client seems generally better.

Comment 56

[Brian Grinstead [:bgrins]]

Attachment: netmonitor-async-add-pt2-r=jsantell.patch

Rebased and made setting lazyUpdate automatically flush requests to fix issues in tests where even after setting lazyUpdate = false they would still need to `yield monitor.panelWin.once(monitor.panelWin.EVENTS.REQUEST_ADDED);`.

Here's a try push with a bunch of retriggers: https://treeherder.mozilla.org/#/jobs?repo=try&revision=85ac14073f82.  Assuming this turns up green, I'll fold these patches and check in.

Comment 57

[Brian Grinstead [:bgrins]]

Attachment: netmonitor-async-add.patch

Folded, try push looks pretty good besides some intermittents: https://treeherder.mozilla.org/#/jobs?repo=try&revision=129380672ee5.

Comment 58

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/92cf636281bb

Comment 59

[Brian Grinstead [:bgrins]]

remote:   https://hg.mozilla.org/integration/fx-team/rev/92cf636281bb

Comment 60

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/92cf636281bb

Comment 61

[Andrei Petcu]

Is this in Nightly? I got a new Nightly 41.0a1 (2015-06-14) and it seems to be slow just like in the initial bug report. Am I doing something wrong (any config flags, with e10s, without e10s)? I started a new vanilla profile with no addons. Thank you!

Comment 62

[Brian Grinstead [:bgrins]]

(In reply to andrei from comment #61)
> Is this in Nightly? I got a new Nightly 41.0a1 (2015-06-14) and it seems to
> be slow just like in the initial bug report. Am I doing something wrong (any
> config flags, with e10s, without e10s)? I started a new vanilla profile with
> no addons. Thank you!

Was it better before the new nightly that you just got?  Meaning, was it all OK in between the initial landing on 5-10 and yesterday?  I guess it's possible that part 2 (or bug 1164728) regressed this, although in my local testing I didn't see any problems.

Comment 63

[Andrei Petcu]

I think it was the same between the new nightly and the initial stable Firefox when I reported the bug.

I did not test other Nightly versions before yesterday :( If it is that useful for you I can get an older build and compare.

Did you use my test case with the nodejs app for testing?

Comment 64

[Brian Grinstead [:bgrins]]

(In reply to andrei from comment #63)
> I think it was the same between the new nightly and the initial stable
> Firefox when I reported the bug.
> 
> I did not test other Nightly versions before yesterday :( If it is that
> useful for you I can get an older build and compare.

If you could, that would be really helpful.  Can you grab a build from June 11 or 12 and do a quick test?  Here is a build directory https://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2015-06-11-03-02-08-mozilla-central/.  Assuming you are on linux, you would grab this file I believe: https://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2015-06-11-03-02-08-mozilla-central/firefox-41.0a1.en-US.linux-x86_64.tar.bz2.

> Did you use my test case with the nodejs app for testing?

Yes, originally I ran through with that, and I thought the first fix helped.  Although I switched over to soundcloud.com, cnn.com, and bgrins.github.io/devtools-demos/misc/requests.html for local testing so I haven't confirmed in a while.

Comment 65

[Andrei Petcu]

Hmmmm... this is really strange. I tested the build you provided on my work computer (Ubuntu 15.04) and it was really fast (400ms while Chromium was around 300ms). I decided to update my Nightly to version 41.0a1 (2015-06-15) and tried again and it was fast again.

The first tests I ran, the one with 41.0a1 (2015-06-14) was on my home computer (Fedora 22 + Xorg). I really don't know why that was slow and I will retest when I get home with the latest build.

This is an awesome performance change! This is soo cool. I'm quite sure a lot of web devs dropped Firefox because of this bug but did not bother to report it.

Comment 66

[Brian Grinstead [:bgrins]]

(In reply to andrei from comment #65)
> Hmmmm... this is really strange. I tested the build you provided on my work
> computer (Ubuntu 15.04) and it was really fast (400ms while Chromium was
> around 300ms). I decided to update my Nightly to version 41.0a1 (2015-06-15)
> and tried again and it was fast again.
> 
> The first tests I ran, the one with 41.0a1 (2015-06-14) was on my home
> computer (Fedora 22 + Xorg). I really don't know why that was slow and I
> will retest when I get home with the latest build.

Hm, yeah let me know if you still see it on the other machine.  It's possible I guess that if the web server is running slower then requests won't be batched up as well.  If that's the case, I'd like to know if the June 11 build is faster than the latest Nightly.

It's also possible that you had e10s on with the builds you just tested and not on with the other computer, which could explain this type of behavior (with multiprocess, any jank in our frontend shouldn't affect the page).  This can be toggled in about:preferences.

> This is an awesome performance change! This is soo cool. I'm quite sure a
> lot of web devs dropped Firefox because of this bug but did not bother to
> report it.

Yes, thank you for making the report and the test case!

Comment 67

[Andrei Petcu]

(In reply to Brian Grinstead [:bgrins] from comment #66)
> If that's the case, I'd like to know if the June 11 build is faster than the latest Nightly.
I got a fresh Nightly and the June 11 build and ran the tests again on my Fedora 22. While I could see some performance differences, I could not say exactly which one is faster. I got 3 consecutive runs where the June 11 was faster, then 4 consecutive where the latest was faster and then all over again. The difference was small between the two and I consider them equally fast.

> It's also possible that you had e10s on with the builds

I had e10s on both OSs. The Fedora 22 Nightly I tested with the first time might have been "dirty" :P It was actually a Firefox GTK3 build upgraded to the latest Nightly and I had several updates on top of that. The latest Nightly works fine so I don't think we should focus on this anymore.

> Yes, thank you for making the report and the test case!

I consider this bug fixed but I might create several performance bug reports in the future :) I'm surprised of how performance issues can be fixed if provided a good test case.

Regarding the bounty.... I have no clue how bountysource works, but I will check out how I can send the actual bounty money. I think the reward should be payed when this fix lands in Aurora, a lot of stuff changes in Nightly.

Thank you again!

Comment 68

[Andrei Petcu]

https://www.bountysource.com/issues/10463206-opening-the-netmonitor-slows-down-requests-on-the-page

Comment 69

[Brian Grinstead [:bgrins]]

(In reply to andrei from comment #67)
> Regarding the bounty.... I have no clue how bountysource works, but I will
> check out how I can send the actual bounty money. I think the reward should
> be payed when this fix lands in Aurora, a lot of stuff changes in Nightly.
> 
> Thank you again!

Thanks for the bounty!  I'm donating the funds to the EB Research Partnership.

Comment 70

[Andrei Petcu]

As I said, I want to make more real-life test scenarios where Firefox is slow. I made a similar bug to this one. I tested with the latest Nightly also. Since Nightly is fast with this bug's scenario and slow with the other scenario I decided to open a new bug. https://bugzilla.mozilla.org/show_bug.cgi?id=1176050 This bug's app resembles more to my day-to-day workflow and the performance issues I have with Firefox.

Comment 71

[Jeff Griffiths (:canuckistani) (:⚡︎)]

(In reply to Brian Grinstead [:bgrins] from comment #69)
> (In reply to andrei from comment #67)
> > Regarding the bounty.... I have no clue how bountysource works, but I will
> > check out how I can send the actual bounty money. I think the reward should
> > be payed when this fix lands in Aurora, a lot of stuff changes in Nightly.
> > 
> > Thank you again!
> 
> Thanks for the bounty!  I'm donating the funds to the EB Research
> Partnership.

Nice one!