Opened 10 months ago

Closed 3 weeks ago

#7129 closed change (rejected)

Don't check every hour for something to download

Reported by: erikvold Assignee:
Priority: Unknown Milestone:
Module: Core Keywords: closed-in-favor-of-gitlab
Cc: kzar, mjethani, sebastian, greiner Blocked By:
Blocking: Platform: Unknown / Cross platform
Ready: no Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

Description

At the moment we have a Downloader class with a _doCheck method which is called every minute in order to check for things to download.

Instead we should just set timers for when things should download, and then we can avoid many calls to _doCheck.

Change History (9)

comment:1 Changed 10 months ago by sebastian

  • Cc mjethani sebastian added

comment:2 follow-up: Changed 10 months ago by mjethani

I'm not familiar with the Downloader code, but are you sure that this is the case? In lib/synchronizer.js we instantiate the Downloader object with an initial delay of 1 second and a check interval of 1 hour. My initial hunch is that the interval is already 1 hour.

comment:3 in reply to: ↑ 2 Changed 10 months ago by erikvold

  • Summary changed from Don't check every minute for something to download to Don't check every hour for something to download

Replying to mjethani:

I'm not familiar with the Downloader code, but are you sure that this is the case? In lib/synchronizer.js we instantiate the Downloader object with an initial delay of 1 second and a check interval of 1 hour. My initial hunch is that the interval is already 1 hour.

Oh yes looks like you are correct, it changes the delay after the first call, initial delay is 1 min, then the interval is 1 hr.

https://gitlab.com/eyeo/adblockplus/adblockpluscore/blob/89a0ce4eaf68ada0f0992b63a147a0fca812faa0/lib/downloader.js#L108

comment:4 Changed 10 months ago by mjethani

What do we mean now by "don't check every hour?"

comment:5 Changed 10 months ago by erikvold

I mean timers should be set for when they should go off, not checking every hour if they should go off. If we have one list that updates every day then that's 23 unnecessary checks.

comment:6 Changed 10 months ago by mjethani

I see, thanks.

comment:7 Changed 10 months ago by erikvold

I think the reason this _doCheck is run every hour now is because it actually does more than just "check" things, it is mutating the expiration values for various reasons. These reasons should be events, which then reset the timers.

Last edited 10 months ago by erikvold (previous) (diff)

comment:8 Changed 10 months ago by greiner

  • Cc greiner added

comment:9 Changed 3 weeks ago by sebastian

  • Keywords closed-in-favor-of-gitlab added
  • Resolution set to rejected
  • Status changed from new to closed

Sorry, but we switched to GitLab. If this issue is still relevant, please file it again in the new issue tracker.

Note: See TracTickets for help on using tickets.