Closed
Bug 1290776
Opened 8 years ago
Closed 3 years ago
Long list of nodes with mousedown event poor performance
Categories
(Core :: Layout, defect, P3)
Core
Layout
Tracking
()
RESOLVED
WORKSFORME
Performance Impact | ? |
People
(Reporter: karlcow, Unassigned)
References
(Blocks 1 open bug, )
Details
(Keywords: perf)
Attachments
(1 file)
(deleted),
text/html
|
Details |
On gfxbench.com Web site, there is a page with a very long list of nodes (around 11,000 nodes).
URI: https://gfxbench.com/compare.jsp
The nodes are used for creating a list in a menu through JavaScript. The mousedown event on this list is noticeably longer than on WebKit or Blink. Partly distributed into Parse HTML and Minor GC.
More details are given https://webcompat.com/issues/2926
Reporter | ||
Updated•8 years ago
|
See Also: → https://webcompat.com/issues/2926
Comment 1•8 years ago
|
||
innerHTML part is quite small here. And even of that removing the old nodes takes time, and it takes time because of layout.
But this seems to be mostly about reflow and restyling.
Component: DOM → Layout
Comment 2•8 years ago
|
||
I do not see any long freeze on Firefox 50 on both Windows and OS X when clicking the dropdown. Time spent looks reasonable.
The only thing surprised me is that it triggers a flush for getting computed value of max-height. As far as I can see, max-height is not in the list of properties using used value as resolved value of CSSOM. [1] This is a web-compat issue we may want to fix.
But the document is only reflowed once anyway, so it may not the issue which leads to the slowness. (Probably doing flush while running script is something enough for making it feel slow?)
[1] https://drafts.csswg.org/cssom/#resolved-values
Comment 3•8 years ago
|
||
This is the testcase showing the difference about how engines deal with max-width/height in the declaration returned from getComputedStyle().
Gecko and Edge return the used value here, as pixels, however Blink and WebKit return the computed value, which leaves percentage and calc unresolved, and because of that, Blink and WebKit do not need a flush for querying computed max-width / height, but we need.
Blink and WebKit's behavior actually matches the spec, while our result is probably more useful. But in general, I don't think we want to put more things in the exception list, so I tend to make it return the computed value rather than the used value for max-width / height.
Updated•8 years ago
|
Comment 4•8 years ago
|
||
Hello,
Have you checked the movie at https://dl.dropboxusercontent.com/u/1332655/2016-07-28 20-02-41.flv ?
Something makes it take 10s on my side. I don't know how to further debug.
Comment 5•8 years ago
|
||
Sorry, space in URL. Correct URL: https://dl.dropboxusercontent.com/u/1332655/2016-07-28%2020-02-41.flv
Comment 6•8 years ago
|
||
(In reply to pedrogfrancisco from comment #5)
> Sorry, space in URL. Correct URL:
> https://dl.dropboxusercontent.com/u/1332655/2016-07-28%2020-02-41.flv
Could you use Gecko Profiler to collect some data to help us know more about the issue here?
Please see this guide for how to do that: https://developer.mozilla.org/en-US/docs/Mozilla/Performance/Reporting_a_Performance_Problem (Note: you don't need to use Nightly, the extension works on Release versions as well.)
Flags: needinfo?(pedrogfrancisco)
Comment 7•8 years ago
|
||
I'm having issues uploading it where I am now. Can you load it from here? https://dl.dropboxusercontent.com/u/1332655/7c4ab3ac-177a-4c70-8e5b-81d361701998
Comment 8•8 years ago
|
||
Yes, that helps a lot, thanks! (You can clear your needinfo request when you answered the question or provided the related information)
Blocks: a11yperf
Flags: needinfo?(pedrogfrancisco)
Comment 9•8 years ago
|
||
In the profiling report from comment 7, ~80% of time are spent on a11y functions including a11y::Accessible::HasGenericType (41.5%), a11y::EventTree::FindOrInsert (31.9%), and a11y::aria::GetRoleMapFromIndex (6.4%). ~5% are spent on the flush caused by querying max-height, which I noticed in comment 2.
So the main issue here is actually a11y. My rough guess is something regresses to O(n^2) unexpectedly again.
Comment 10•8 years ago
|
||
That makes sense.
I have a touchscreen. I am using browser.tabs.remote.force-enable to test e10s.
Is there anything else I can try to help?
Comment 11•8 years ago
|
||
If you know coding and you are interested in that, you can try fixing it ;)
Otherwise, the information you provided should be useful enough, and I'll try to reproduce it myself with a11y activated tomorrow.
Comment 12•8 years ago
|
||
Not enough disk space for build :(
Comment 13•8 years ago
|
||
Alex please see comment 9 etc. What can we do?
Flags: needinfo?(surkov.alexander)
Comment 14•8 years ago
|
||
(In reply to David Bolter [:davidb] from comment #13)
> Alex please see comment 9 etc. What can we do?
Michael and me are working on this
Flags: needinfo?(surkov.alexander)
Updated•7 years ago
|
Priority: -- → P3
Comment 15•7 years ago
|
||
Given, the event coalescence was changed from the bug report times, Marco, would you mind to give it a try to see how bad it is now?
Flags: needinfo?(mzehe)
Comment 16•7 years ago
|
||
It still is very bad. For one, Buffer load took 34.292 sec, 312479 chars
. But on top of that, before the buffer even starts loading, at least 5 seconds pass, and even when the browser becomes responsive again, over 40 seconds in total have passed. Moreover, the browser is still very sluggish until the tab is closed. This is with the test case given in comment #0. So it's definitely still worth investigating.
Flags: needinfo?(mzehe)
Updated•7 years ago
|
Whiteboard: [qf:needs-analysis]
Reporter | ||
Updated•3 years ago
|
Webcompat Priority: --- → ?
Updated•3 years ago
|
Performance Impact: --- → ?
Whiteboard: [qf:needs-analysis]
Reporter | ||
Comment 17•3 years ago
|
||
This is working fine now.
Maybe the issue can be closed.
Webcompat Priority: ? → ---
Comment 18•3 years ago
|
||
Closing as WFM based on comment 17. Let's re-open if this becomes an issue again.
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → WORKSFORME
You need to log in
before you can comment on or make changes to this bug.
Description
•