Opened 3 years ago

Last modified 3 years ago

#1162 reviewing change

Cache notification URL matcher

Reported by: greiner Assignee: greiner
Priority: P2 Milestone:
Module: Core Keywords:
Cc: trev Blocked By:
Blocking: Platform: Unknown
Ready: yes Confidential: no
Tester: Verified working: no
Review URL(s):

http://codereview.adblockplus.org/5330039625220096/

Description

Background

We introduced notifications that can be triggered when matching a given set of filters for the anti-adblock notification. In the current implementation we are instantiating the matcher each time when we check for a notification based on the assumption that the performance impact would be insignificant.

However, since this is not the case due to this happening on each request we should only instantiate it once and reuse it after that.

What to change

In lib/notification.js:

  • Instantiate the matcher on notification creation and attach it to the notification as a property _matcher
  • Notifications should remain serializable (replace JSON.serialize() to drop all properties starting with "_" if necessary)
  • Remove the instantiation in getNextToShow() and instead use the matcher associated with the notification

Change History (3)

comment:1 Changed 3 years ago by greiner

  • Cc trev added

comment:2 Changed 3 years ago by greiner

  • Owner set to greiner

comment:3 Changed 3 years ago by greiner

  • Review URL(s) modified (diff)
  • Status changed from new to reviewing
Note: See TracTickets for help on using tickets.