From a2a8ef7e851471b3902ff1606f9266bfd93669ed Mon Sep 17 00:00:00 2001
From: Raymond Hill <rhill@raymondhill.net>
Date: Mon, 16 Aug 2021 10:58:04 -0400
Subject: [PATCH] Avoid matching the block-important realm unconditionally

When matching a network request in the static network filtering
engine ("snfe"), these are the possible outcomes, from most
to least likely:

- No block
- Block
- Unblock ("exception" filter overriding the block)
- Block-important ("important" filter override the unblock)

Hence why the matching in the snfe always check for a match in
the "block" realm, and the "unblock" realm would be checked
if and only if there was a match in the "block" realm.

However the "block-important" realm was always matched against
first, and when a match in that realm was found, there would
be no need to check in other realms since nothing can override
the "important" option. The problem with this approach though
is that matches in the "block-important" realm are most
unlikely, which means pointless work being done for vast
majority of network requests.

This commit makes it so that the "block-important" realm is
matched against ONLY when there is a matched "unblock" filter.
The result is a measurable improvement in the snfe-related
benchmarks (though given the numbers involved, end users won't
perceive a difference).

Somewhat related discussion which was the motivation to look
more into this:

https://github.com/cliqz-oss/adblocker/discussions/2170#discussioncomment-1168125
---
 src/js/background.js           |  2 +-
 src/js/static-net-filtering.js | 67 +++++++++++++++++++++++++++++-----
 2 files changed, 59 insertions(+), 10 deletions(-)

diff --git a/src/js/background.js b/src/js/background.js
index 86e7a6351..ea3a28896 100644
--- a/src/js/background.js
+++ b/src/js/background.js
@@ -154,7 +154,7 @@ const µBlock = {  // jshint ignore:line
 
     // Read-only
     systemSettings: {
-        compiledMagic: 37,  // Increase when compiled format changes
+        compiledMagic: 38,  // Increase when compiled format changes
         selfieMagic: 38,    // Increase when selfie format changes
     },
 
diff --git a/src/js/static-net-filtering.js b/src/js/static-net-filtering.js
index aba753f8b..a408ead76 100644
--- a/src/js/static-net-filtering.js
+++ b/src/js/static-net-filtering.js
@@ -297,9 +297,6 @@ class LogData {
             isRegex: false,
         };
         filterUnits[iunit].logData(logData);
-        if ( (categoryBits & Important) !== 0 ) {
-            logData.options.unshift('important');
-        }
         if ( (categoryBits & ThirdParty) !== 0 ) {
             logData.options.unshift('3p');
         } else if ( (categoryBits & FirstParty) !== 0 ) {
@@ -599,6 +596,42 @@ registerFilterClass(FilterTrue);
 
 /******************************************************************************/
 
+// The only purpose of this class is so that the `important` filter
+// option is added to the logged raw filter.
+
+const FilterImportant = class {
+    match() {
+        return true;
+    }
+
+    logData(details) {
+        details.options.unshift('important');
+    }
+
+    toSelfie() {
+        return FilterImportant.compile();
+    }
+
+    static compile() {
+        return [ FilterImportant.fid ];
+    }
+
+    static fromCompiled() {
+        return new FilterImportant();
+    }
+
+    static fromSelfie() {
+        return new FilterImportant();
+    }
+
+    static keyFromArgs() {
+    }
+};
+
+registerFilterClass(FilterImportant);
+
+/******************************************************************************/
+
 const FilterPatternPlain = class {
     constructor(i, n) {
         this.i = i | 0;
@@ -3398,6 +3431,11 @@ class FilterCompiler {
             units.push(FilterAnchorRight.compile());
         }
 
+        // Important
+        if ( (this.action & Important) !== 0 ) {
+            units.push(FilterImportant.compile());
+        }
+
         // Strict partiness
         if ( this.strictParty !== 0 ) {
             units.push(FilterStrictParty.compile(this));
@@ -3434,7 +3472,19 @@ class FilterCompiler {
         const fdata = units.length === 1
             ? units[0]
             : FilterCompositeAll.compile(units);
+
         this.compileToAtomicFilter(fdata, writer);
+
+        // Add block-important filters to the block realm, so as to avoid
+        // to unconditionally match against the block-important realm for
+        // every network request. Block-important filters are quite rare so
+        // the block-important realm should be checked when and only when
+        // there is a matched exception filter, which important filters are
+        // meant to override.
+        if ( (this.action & BlockImportant) !== 0 ) {
+            this.action &= ~Important;
+            this.compileToAtomicFilter(fdata, writer);
+        }
     }
 
     compileToAtomicFilter(fdata, writer) {
@@ -4202,15 +4252,14 @@ FilterContainer.prototype.matchRequest = function(fctxt, modifiers = 0) {
     $docEntity.reset();
     $requestHostname = fctxt.getHostname();
 
-    // Important block realm.
-    if ( this.realmMatchString(BlockImportant, typeValue, partyBits) ) {
-        return 1;
-    }
-
-    // Evaluate block realm before allow realm.
+    // Evaluate block realm before allow realm, and allow realm before
+    // block-important realm, i.e. by order of likelihood of a match.
     const r = this.realmMatchString(BlockAction, typeValue, partyBits);
     if ( r || (modifiers & 0b0010) !== 0 ) {
         if ( this.realmMatchString(AllowAction, typeValue, partyBits) ) {
+            if ( this.realmMatchString(BlockImportant, typeValue, partyBits) ) {
+                return 1;
+            }
             return 2;
         }
         if ( r ) { return 1; }