Opened 6 years ago

Closed 6 years ago

Last modified 5 years ago

#117 closed defect (fixed)

Allow other events to be processed while patterns.ini is being read

Reported by: trev Assignee: trev
Priority: P2 Milestone: Adblock-Plus-2.6-for-Firefox
Module: Adblock-Plus-for-Firefox Keywords:
Cc: Blocked By: #152
Blocking: #189 Platform:
Ready: no Confidential: no
Tester: Verified working: no
Review URL(s):

http://codereview.adblockplus.org/5731880649359360/

Description

The processing of patterns.ini happens on the main thread and can potentially delay Firefox startup, particularly critical on mobile. To prevent this we can occasionally spin the event loop and allow other events to be processed. However, we have to be careful about potential reentrance then.

Change History (15)

comment:1 Changed 6 years ago by trev

  • Status changed from new to reviewing

comment:2 Changed 6 years ago by trev

I measured the impact of this change using the about:startup add-on, looking at the sessionRestored time (same session every time):

Without Adblock Plus: 830-980 ms
Unchanged Adblock Plus: 1900-2000 ms
Adblock Plus with this change: ~860 ms

The delay added by Adblock Plus with this change is low enough on my laptop that it is obscured by regular variations in startup performance.

comment:3 Changed 6 years ago by trev

  • Milestone set to Adblock-Plus-for-Firefox-next
  • Resolution set to fixed
  • Status changed from reviewing to closed
Version 0, edited 6 years ago by trev (next)

comment:4 Changed 6 years ago by trev

  • Blocked By 152 added

comment:5 Changed 6 years ago by trev

  • in_progress set to 0
  • Ready unset
  • Review URL(s) modified (diff)

comment:6 Changed 6 years ago by Ede_123

  • Resolution fixed deleted
  • Status changed from closed to reopened

I'm afraid this is not yet fixed...

Running Firefox 28.0 on Windows 7 x64 I measured the following startup times ("firstPaint") with a clean profile:
1428 ms mean (over 7 startups) with Adblock Plus disabled
2544 ms mean (over 9 startups) with Adblock Plus enabled (2.5.1.3781)
4780 ms mean (over 6 startups) with Adblock Plus enabled (2.5.1 stable)
This slowdown of 78 % would be OK (although not nearly as fast as trev measured above) in principle, but I noticed that an additional lag is introduced directly after startup (ABP icon takes about 3-4 seconds to appear; Firefox UI is sluggish during this time).

When using my productive Firefox profile the improvement is even gone:
2488 ms mean (over 7 startups) with Adblock Plus disabled
5703 ms mean (over 6 startups) with Adblock Plus enabled (2.5.1.3781)
The latter value is about what I got with Adblock 2.5.1 stable. However I can't notice any lag here, so I assume "firstPaint" is delayed by some other Add-On I'm using until ABP has finished it's "post startup work".

For the tests above I subscribed to the following filter lists:
EasyList, EasyList Germany, EasyPrivacy, Fanboy's Social Blocking List, Adblock Warning Removal List

Details can also be found in the relevant forum thread (https://adblockplus.org/forum/viewtopic.php?f=11&t=19901)

comment:7 Changed 6 years ago by trev

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

This issue as filed has been resolved, processing patterns.ini now allows other events to be processed. Please create a follow-up issue for the problem you are seeing.

comment:8 Changed 6 years ago by Ede_123

If this is the way you prefer... New bug is #177!

comment:9 Changed 6 years ago by trev

  • Blocked By 177 added
  • Summary changed from Don't delay Firefox startup to Allow other events to be processed while patterns.ini is being read

comment:10 Changed 6 years ago by philll

  • Resolution fixed deleted
  • Status changed from closed to reopened

If this issue here is blocked by another one, it cannot be closed by nature.

comment:11 Changed 6 years ago by trev

  • Blocked By 177 removed
  • Resolution set to fixed
  • Status changed from reopened to closed

As discussed on IRC, the "blocked by" flag here is used to track potential issues with the change. At this point it isn't really clear whether #177 indicates an issue with the approach here or whether it points out a different though related problem. If we leave issues open in scenarios like this one then we'll often leave them open forever.

comment:12 Changed 6 years ago by trev

  • Blocked By 177 added

comment:13 Changed 6 years ago by trev

  • Blocked By 177 removed

comment:14 Changed 6 years ago by trev

  • Blocking 189 added

comment:15 Changed 5 years ago by hajj_3

you have broken adblock plus on firefox now! I have about 40 tabs that restore when i launch firefox, now adblock plus doesn't load straight away and there are ads on lots of the webpages now. The whole purpose of adblock plus is to block ads now i have to refresh each page to get rid of them just so that firefox launches slightly faster. Please undo this change!

Note: See TracTickets for help on using tickets.