Open Bug 1679917 Opened 10 months ago Updated 9 months ago

Game selection page on shutupandsitdown.com causes 6.4s event processing delay on fenix

Categories

(Core :: CSS Parsing and Computation, defect)

defect

Tracking

()

People

(Reporter: mcomella, Unassigned, NeedInfo)

References

Details

Basic information

Steps to Reproduce:

  • On fenix, open shutupandsitdown.com/games-page/ (a board game website)
  • (wait a very long time for the content to load – is this the same problem as later?)
  • Click one of the filtering categories such as "cooperative games"

Expected Results:
Filtering occurs quickly. On Chrome, it takes maybe 3 seconds (which is also longer than desired)

Actual Results:
Filtering is slow. On Fenix, it's twice as slow as Chrome, maybe 6 seconds. Since it reproduces in Chrome, it indicates a poor page operation but given that we're subjectively twice as slow as chrome, there may be areas for us to improve


More information

Screenshot: (if relevant, please attach a screenshot or screencast to this bug report)

Profile URL: https://share.firefox.dev/39pcBZ0

Basic systems configuration:

Device: Pixel 2
OS version: Android 11
Number of cores:
Amount of memory (RAM):

Thanks so much for your help.

ni myself to profile this.

Flags: needinfo?(acreskey)

My profiling showed the same issue that mcomella found, multisecond calls to Servo_SelectorList_QueryAll, e.g. https://share.firefox.dev/3np4rUr

Using Chrome on the same device, this is a few times faster (noticeable).
Imported Chrome profile here:
https://share.firefox.dev/2WicLtp

Maybe there's something that can be done here?
QuerySelector cache being invalidated unnecessarily?

Component: Performance → CSS Parsing and Computation
Flags: needinfo?(acreskey)

Also, do I have the right component/repo, if the area of improvement could be in servo/stylo?

Yes, this is the right component.

That page is dumb btw, they have a lot of useless stuff going on... But anyhow, test-case:

  • Load https://www.shutupandsitdown.com/games-page/
  • Open devtools.
  • Paste: var start = performance.now(); for (let i = 0; i < 10000; ++i) document.querySelectorAll(".esg-fgc-27.esg-filter-wrapper .hiddensearchfield"); console.log(performance.now() - start);

In Firefox I get around 1700ms. In Chrome I get ~1000ms, which matches the reported speed difference basically.

On page load this is part of the problem, but also that they're calling getComputedStyle computing transforms on a bunch of undisplayed elements. That's bug 1381071.

The worse thing is that that element doesn't even exist, so they're basically scanning the DOM over and over and over.

I suspect this is going to either be:

  • DOM iteration order overhead, or...
  • has_class overhead.

https://treeherder.mozilla.org/jobs?repo=try&revision=bfadb467e9ad9c23b35d35b3b6972fc42b3a7d8a is a try run in case this is the former.

Other than this I need to jump to perf profiling or something.

Flags: needinfo?(emilio)

(In reply to Emilio Cobos Álvarez (:emilio) from comment #7)

https://treeherder.mozilla.org/jobs?repo=try&revision=bfadb467e9ad9c23b35d35b3b6972fc42b3a7d8a is a try run in case this is the former.

Other than this I need to jump to perf profiling or something.

Thank you for jumping into this.

I've added android build jobs and I'll compare your commit against the parent commit.
Lower-end android may show a bigger difference.

Flags: needinfo?(acreskey)

Nah, that didn't seem to cause any meaningful difference.

Using the steps from comment 4, I get these results on a Pixel 3.

Make next_in_preorder generate slightly better code.

17898
17822
18047
18296
Mean = 18016

Baseline: (the m-c commit before the above)

19418
18872
18656
18582
Mean = 18882

I'm not claiming that this is a statistically significant improvement, but it does look a bit better.

Chrome, on that device, is much faster:

3734.300000127405
3782.999999821186
3805.9999998658895
3746.3999995961785
Flags: needinfo?(acreskey)

Curious, does dom.arena_allocator.enabled=false or such make any difference? I'm landing that patch in bug 1683295 anyways.

Flags: needinfo?(acreskey)

(In reply to Emilio Cobos Álvarez (:emilio) from comment #11)

Curious, does dom.arena_allocator.enabled=false or such make any difference? I'm landing that patch in bug 1683295 anyways.

It looks like the arena allocator helps the performance slightly in this test case:

dom.arena_allocator.enabled:true
19364		
18903		
18720		
18494		
18558
Median: 18720
Mean:   18807.8
dom.arena_allocator.enabled:false
19913
19567
19459
19230
19303
Median: 19459
Mean:   19494.4

Measurements from Fenix Beta (fenix.beta.2020.12.18), aarch64, Pixel 3

Flags: needinfo?(acreskey)
Severity: -- → S3

Lots of elements on that page have very few classes, and from what I can see with perf we're paying a lot for the indirection:

[...document.querySelectorAll("[class]")].map(e => e.classList.length).reduce((out, value) => { out[value] = (out[value] || 0) + 1; return out}, {}):

1: 4940
2: 1417
3: 1407
4: 1405
5: 9
8: 3
10: 4
11: 51
12: 163
13: 145
14: 125
15: 92
16: 59
17: 30
18: 12
19: 5
20: 1
32: 1
Depends on: 1684673
You need to log in before you can comment on or make changes to this bug.