1346585 - Horrible performance on Tesco webshop

Status: RESOLVED FIXED

(Core :: Panning and Zooming, defect)

Description

[avada]

This has been going on for a while, months at least. But since it didn't improve I'm filing this bug.
It happens with a brand new profile also.

STR is simple. Just visit the url and basically do anything. Everything is laggy the CPU usage is constantly high, pages take ages to load. For example there can be several second between clicking the search field and it becoming usable. (Keystrokes are buffered in the meanwhile.)
Even scrolling can murder performance. There's a huge delay for pulling the scrollbar and the actual scrolling to happen.
Etc...

Comment 1

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

avada, does that issue also happen if Firefox run in its safe mode? See https://support.mozilla.org/t5/Procedures-to-diagnose-and-fix/Troubleshoot-Firefox-issues-using-Safe-Mode/ta-p/1687

Comment 2

[avada]

(In reply to Sebastian Hengst [:aryx][:archaeopteryx] (needinfo on intermittent or backout) from comment #1)
> avada, does that issue also happen if Firefox run in its safe mode? See
> https://support.mozilla.org/t5/Procedures-to-diagnose-and-fix/Troubleshoot-
> Firefox-issues-using-Safe-Mode/ta-p/1687

Well I tried it with a brand new profile (forgot to mention), which is much the same but better even.
Anyway I tried safe mode too. No difference.

Comment 3

[Vlad Bacia-Mociran [:VladB]]

User Agent: Mozilla/5.0 (Windows NT 6.3; Win64; x64; rv:55.0) Gecko/20100101 Firefox/55.0

I have tested your issue on latest Firefox release v52.0, latest Nightly (Buid ID: 20170314030215) and managed to reproduce it. I have opened https://bevasarlas.tesco.hu/groceries/en-GB and started browsing through the website. On page load, when viewing different products the CPU usage rises up to 30%-35%. Other than that, the website has huge delay when opening webpages.

I did a performance profile using Gecko Profiler: https://perfht.ml/2mMPHSE .
@Mike, please, can you help me with this? I need to find a home for this issue. Thank you.

Comment 4

[Mike Conley (:mconley) (:⚙️)]

Hm. That profile seems to show us blocking the main thread in the content process due to lots of JS going on on the Tesco page. Lots of "mounting components", which makes me think this page is powered by React or something. In any case, lots of busy JS in there, greatly exceeding the 16ms time limit per frame that's required for 60fps.

I'm not sure there's much else to find in there. Hey jrmuizel, do you see anything else in there worth calling out?

Comment 5

[Jeff Muizelaar [:jrmuizel]]

It looks to me like the problem here is SendSetTargetAPZCNotification. It looks expensive and is happening on every mouse event.

Comment 6

[Jeff Muizelaar [:jrmuizel]]

Attachment: Only call describe when we're actually going to do the logging

Comment 7

[Jeff Muizelaar [:jrmuizel]]

It looks like we're spending 238ms in dom::Element::Describe(). This patch should make things much better.

Comment 8

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8847722 [details] [diff] [review]
Only call describe when we're actually going to do the logging

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

r+ but in the future i'm going to start minusing patches without commit messages so beware!

Also, while this appears to be 12% of the profile, and will definitely help, I'm not convinced it will fix the problem entirely. There's a lot of JS running no matter how you slice it.

::: gfx/layers/apz/util/APZCCallbackHelper.cpp
@@ +28,5 @@
>  #include "nsString.h"
>  #include "nsView.h"
>  #include "Layers.h"
>  
> +#ifdef APZCCH_LOGGING

nit: above this, add this:

// #define APZCCH_LOGGING 1

so that enabling the logging is just a matter of uncommenting the line.

Comment 9

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8847722 [details] [diff] [review]
Only call describe when we're actually going to do the logging

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

Yep. I'd already added that line locally. You beat me to uploading a new patch.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e88badcab9a4
Only call describe when we're actually going to do the logging r=kats

Comment 11

[Jeff Muizelaar [:jrmuizel]]

Can you try out this build https://queue.taskcluster.net/v1/task/Lh0MvlePRBaFBbe5Oh6z1w/runs/0/artifacts/public/build/install/sea/firefox-55.0a1.en-US.win64.installer.exe to see if the performance is improved?

Comment 12

[Carsten Book [:Tomcat]]

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

Comment 13

[avada]

(In reply to Jeff Muizelaar [:jrmuizel] from comment #11)
> Can you try out this build
> https://queue.taskcluster.net/v1/task/Lh0MvlePRBaFBbe5Oh6z1w/runs/0/
> artifacts/public/build/install/sea/firefox-55.0a1.en-US.win64.installer.exe
> to see if the performance is improved?

Lots better. There's only a bit of lag when the page loads.

Comment 14

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8847722 [details] [diff] [review]
Only call describe when we're actually going to do the logging

Approval Request Comment
[Feature/Bug causing the regression]: Yes been broken for a while.
[User impact if declined]: Slow performance on some websites to the point of being unusable.
[Is this code covered by automated tests?]: Doesn't need to be.
[Has the fix been verified in Nightly?]: Yes.
[Needs manual test from QE? If yes, steps to reproduce]: Nope
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: No.
[Why is the change risky/not risky?]: This changes is not risky. It only removes some debugging code and gives very noticeable performance impact on some sites.

Comment 15

[Gerry Chang [:gchang]]

Comment on attachment 8847722 [details] [diff] [review]
Only call describe when we're actually going to do the logging

Improve a performance issue. Beta53+ & Aurora54+.

Comment 16

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/3b34b60914cc

Comment 17

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/fa875453ebf5

Comment 18

[Andrei Vaida [:avaida]]

Setting qe-verify- based on Jeff's assessment on manual testing needs (see Comment 14).