Opened 19 months ago

Closed 19 months ago

Last modified 19 months ago

#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):

https://codereview.adblockplus.org/29341238/

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

  1. Start recording a CPU profile using the Chrome profiling tool.
  2. Open http://projects.fivethirtyeight.com/election-2016/primary-forecast/new-york-democratic/
  3. 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)

addElemHideSelectors-profile.png (128.0 KB) - added by kzar 19 months ago.
Screenshot of addElemHideSelectors profiling in Chrome 52

Download all attachments as: .zip

Change History (8)

Changed 19 months ago by kzar

Screenshot of addElemHideSelectors profiling in Chrome 52

comment:1 Changed 19 months ago by kzar

  • Description modified (diff)

comment:2 Changed 19 months ago by kzar

  • Owner set to kzar

comment:3 Changed 19 months ago by kzar

  • Review URL(s) modified (diff)
  • Status changed from new to reviewing

comment:4 Changed 19 months ago by sebastian

  • Priority changed from Unknown to P2
  • Ready set

comment:5 Changed 19 months ago by abpbot

A commit referencing this issue has landed:
https://hg.adblockplus.org/adblockpluschrome/rev/9f451f809d40

comment:6 Changed 19 months ago 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 19 months ago by kzar

  • Cc Ross scheer added
Note: See TracTickets for help on using tickets.