Opened 6 weeks ago

Closed 12 days ago

#5395 closed defect (fixed)

Scrolling through Facebook feed is very laggy

Reported by: kzar Assignee: trev
Priority: P1 Milestone:
Module: Core Keywords:
Cc: philll, arthur, sebastian, trev, mapx, hfiguiere, fhd Blocked By:
Blocking: #5438 Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Ross Verified working: yes
Review URL(s):

https://codereview.adblockplus.org/29485567/

Description (last modified by trev)

Environment

  • Adblock Plus 1.13.3 release candidate
  • Chrome 59
  • No subscriptions, the following filters:
facebook.com#?#.ego_column :-abp-has(.adsCategoryTitleLink)
facebook.com#?#div[id^="hyperfeed_story_id_"] :-abp-has(a[href*="?hc_ref=ADS"])

How to reproduce

  1. Browse to https://www.facebook.com/ and login.
  2. Scroll up and down through your timeline, you'll need to scroll far enough down to trigger the loading of more items.

Observed behaviour

Items in the feed render very slowly, "Page Unresponsive" messages sometimes displayed.

Expected behaviour

Page does not load very slowly, like with Adblock Plus 1.13.2

Background

Facebook is constantly loading new stylesheets as new feed items are being added. This currently makes us reapply all element hiding emulation rules, which takes increasingly long as the document grows. This behavior should be considered a bug because the rules in question don't use :-abp-properties() and so don't depend on stylesheets.

What to change

  • When stylesheets change, only reprocess rules that actually depend on styles (:-abp-properties(), also as an inner selector of :-abp-has(... )).
  • Don't process stylesheet changes more often than every three seconds - if multiple stylesheets load in quick succession, process them as a batch.

Attachments (1)

slow-elemhide.png (121.6 KB) - added by kzar 6 weeks ago.
Bottom-Up performance

Download all attachments as: .zip

Change History (20)

comment:1 Changed 6 weeks ago by kzar

Did I get the details right? I'm going to attempt to reproduce this now.

comment:2 Changed 6 weeks ago by kzar

  • Cc hfiguiere added
  • Description modified (diff)
  • Milestone set to Adblock-Plus-1.13.3-for-Chrome-Opera
  • Ready set

comment:3 Changed 6 weeks ago by kzar

  • Description modified (diff)

Changed 6 weeks ago by kzar

Bottom-Up performance

comment:4 Changed 6 weeks ago by mapx

I see only 2 rules in easylist with the new feature (both on FB)

facebook.com#?#.ego_column :-abp-has(.adsCategoryTitleLink)
facebook.com#?#div[id^="hyperfeed_story_id_"] :-abp-has(a[href*="?hc_ref=ADS"])

comment:5 Changed 6 weeks ago by kzar

  • Description modified (diff)

I see only 2 rules in easylist with the new feature (both on FB)

Yep you're right, I can reproduce the problem with only those two filters. I've updated the description.

comment:6 Changed 6 weeks ago by trev

So far the issue seems to be:

  • Facebook is constantly loading new stylesheets as new feed items are being added. This currently makes us reapply all element hiding emulation rules, which takes increasingly longer as the document grows.
  • This behavior should be considered a bug because the rules in question don't use :-abp-properties() and so don't depend on stylesheets.
  • Without this bug these rules wouldn't have any noteworthy effect on Facebook given that in the current iteration our :-abp-has() implementation doesn't consider dynamic changes yet.

comment:7 Changed 6 weeks ago by kzar

  • Cc fhd added
  • Owner kzar deleted

Unassigning myself for now since other than reverting these changes I'm not sure how to proceed.

comment:8 Changed 6 weeks ago by trev

  • Component changed from Platform to Core
  • Milestone Adblock-Plus-1.13.3-for-Chrome-Opera deleted

Moving to Core, the Platform part is going to be a simple dependency update.

comment:9 Changed 6 weeks ago by trev

  • Owner set to trev

comment:10 Changed 6 weeks ago by trev

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

comment:11 Changed 6 weeks ago by trev

  • Description modified (diff)

comment:13 Changed 6 weeks ago by trev

Leaving open because this change only landed on a branch for now, it needs to be merged to master as well.

comment:14 Changed 6 weeks ago by Ross

I've tried applying these filters and scrolling through my feed and they don't seem to apply at all/match anything at all. I'm trying to check via the devtools. Am I missing something?

ABP 1.13.2.1786
Chrome 59 / Windows 10

comment:15 Changed 6 weeks ago by trev

No, in build 1786 they aren't supposed to apply any more - the fact that they did before was a bug.

comment:16 Changed 6 weeks ago by Ross

  • Tester changed from Unknown to Ross
  • Verified working set

Thanks. This is fixed. Loading more items works fine without anymore than usual lag and the page is never made unresponsive.

ABP 1.13.2.1787
Chrome 51+ / Windows 7
Opera 38+ / Windows 7

comment:17 Changed 4 weeks ago by hfiguiere

  • Blocking 5438 added

comment:19 Changed 12 days ago by trev

  • Resolution set to fixed
  • Status changed from reviewing to closed

Landed on master now, only context changed slightly here.

Note: See TracTickets for help on using tickets.