Opened 4 months ago

Closed 3 months 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 4 months ago.
Bottom-Up performance

Download all attachments as: .zip

Change History (20)

comment:1 Changed 4 months ago by kzar

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

comment:2 Changed 4 months 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 4 months ago by kzar

  • Description modified (diff)

Changed 4 months ago by kzar

Bottom-Up performance

comment:4 Changed 4 months 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 4 months 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 4 months 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 4 months 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 4 months 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 4 months ago by trev

  • Owner set to trev

comment:10 Changed 4 months ago by trev

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

comment:11 Changed 4 months ago by trev

  • Description modified (diff)

comment:13 Changed 3 months 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 3 months 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 3 months 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 3 months 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 3 months ago by hfiguiere

  • Blocking 5438 added

comment:19 Changed 3 months 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.