Opened on 05/11/2016 at 02:29:54 PM
Closed on 05/11/2016 at 04:21:28 PM
Last modified on 05/11/2016 at 04:23:59 PM
#4036 closed defect (fixed)
Slow addElemHideSelectors performance with Chrome >= 51
| Reported by: | kzar | Assignee: | kzar |
|---|---|---|---|
| Priority: | P2 | Milestone: | Adblock-Plus-1.12-for-Chrome-Opera-Safari |
| Module: | Platform | Keywords: | |
| Cc: | sebastian, mapx, trev, Ross, scheer | Blocked By: | |
| Blocking: | Platform: | Chrome | |
| Ready: | yes | Confidential: | no |
| Tester: | Unknown | Verified working: | no |
| Review URL(s): | |||
Description (last modified by kzar)
Environment
- Chrome both Version 51.0.2704.36 beta (64-bit) and Version 52.0.2729.3 dev (64-bit)
- EasyList subscription with AA enabled
- Debian Linux Stretch 64bit
- Thinkpad T450
How to reproduce
- Start recording a CPU profile using the Chrome profiling tool.
- Open http://projects.fivethirtyeight.com/election-2016/primary-forecast/new-york-democratic/
- Once loaded stop recording the profile and take a look.
Observed behaviour
More than 6000ms is spent in the addElemHideSelectors function in the include.preload.js content script.
Expected behaviour
With earlier Chrome versions around 300ms is spent in that function.
Notes
- I couldn't reproduce the issue with either Version 49.0.2623.75 (64-bit) or Version 50.0.2661.94 (64-bit).
- This seems completely unrelated to issue #235, performance of ElemHide.getSelectorsByDomain() seems to be just as fast with newer Chrome versions.
- I don't think this is down to a deopt with new Chrome versions, or at least the profiler doesn't show any deoptimisation warnings. (See the attached screenshot.)
- See related discussion in the forum (Note: The title seems incorrect, it seems that the problem started with version 51 not 50.)
- The problem seems to be caused by the while loop inside addElemHideSelectors somehow. When that is commented out the problem goes away. (The problem does not go away when only the style.sheet.addRule line is commented so it looks like the problem could be with the line above.)
Attachments (1)
Change History (8)
Changed on 05/11/2016 at 02:30:35 PM by kzar
comment:2 Changed on 05/11/2016 at 03:07:09 PM by kzar
- Owner set to kzar
comment:3 Changed on 05/11/2016 at 03:34:31 PM by kzar
- Review URL(s) modified (diff)
- Status changed from new to reviewing
comment:4 Changed on 05/11/2016 at 03:46:36 PM by sebastian
- Priority changed from Unknown to P2
- Ready set
comment:5 Changed on 05/11/2016 at 04:20:22 PM by abpbot
A commit referencing this issue has landed:
https://hg.adblockplus.org/adblockpluschrome/rev/9f451f809d40
comment:6 Changed on 05/11/2016 at 04:21:28 PM by kzar
- Milestone set to Adblock-Plus-for-Chrome-Opera-Safari-next
- Resolution set to fixed
- Status changed from reviewing to closed
comment:7 Changed on 05/11/2016 at 04:23:59 PM by kzar
- Cc Ross scheer added
Note: See
TracTickets for help on using
tickets.
Screenshot of addElemHideSelectors profiling in Chrome 52