AdBlocker: request engine - #3245
Conversation
a458be7 to
07af4c3
Compare
686b310 to
931240e
Compare
|
It seems |
|
|
931240e to
42abfa4
Compare
| }; | ||
| var it: Tokens = .{ .text = url }; | ||
| while (it.next()) |token| { | ||
| if (request.tokens_len == request.tokens_buf.len) break; |
There was a problem hiding this comment.
Right now, AdBlocker.match does:
if (self.trie.matches(self.suppressed, hostname) != null) return .none;
if (self.trie.matches(self.blocked_important, hostname) != null) return .blocked;
if (self.blocking_important.match(request) != null) return .blocked;
if (self.exceptions.match(request) != null) return .allowed;
if (self.trie.matches(self.allowed, hostname) != null) return .allowed;
if (self.trie.matches(self.blocked, hostname) != null) return .blocked;
if (self.blocking.match(request) != null) return .blocked;There are cases where you won't even need the tokens that you've already split / hashed. AND, you can rewrite that to be even more efficient:
if (self.trie.matches(self.suppressed, hostname) != null) return .none;
if (self.trie.matches(self.blocked_important, hostname) != null) return .blocked;
if (self.blocking_important.match(request) != null) return .blocked;
if (self.trie.matches(self.blocked, hostname) != null or self.blocking.match(request) != null) {
if (self.trie.matches(self.allowed, hostname) != null) return .allowed;
if (self.exceptions.match(request) != null) return .allowed;
return .blocked;
}
return .none;There's no need to check exceptions unless something is blocked (.allowed vs .none is just a metrics difference) AND there are fewer cases where you need to tokenize/hash anything.
With that in mind, don't pre-tokenize. Tokenize on demand. Whether or not it's worth caching in the tokenizer, I don't know. But it isn't worth limiting it to 128 tokens. Since the tokenizer becomes streaming, you can cache the first 128 values, and then stream until the end of the URL.
There was a problem hiding this comment.
Since the tokenizer becomes streaming, you can cache the first 128 values, and then stream until the end of the URL.
I actually thought the same, Claude also recommended, but unsure how many cases would benefit from it. Currently overflow results to loss of tokens; comparing it with stream-the-rest approach.
`Parser`: don't drop cosmetic lines introduce url pattern matcher yet-another-url-parser
…ilter list in `HttpClient`
* Engine.Request.fromHttp(req, source_url, buffers) now builds the adblock request straight from HttpClient.Request. * The URL is tokenized once per request (hashed into the Request, shared by all engines); capped at 128 tokens (same as adblock-rust). * Document hostname longer than 253 bytes now skips adblocking.
* `@@…$important` -> `error.InvalidOption` * `||host`, `||host^`, `||host|` (and bare `host|`/`|host` lines) all read as "hostname or subdomain" and land in the trie when option-free. * Wildcard trimming now follows uBO's pointless-wildcard rules.
* remove unnecessary `pub` marking * metrics tracking cosmetic filters separately
Eager tokenization is ~7% of a ~2.1 µs match. A lazy fill saved nothing because every request reaches the first engine. Streaming from scratch per engine cost +10% capped and +16% uncapped, and stayed +10% even with exception gating. The 128 cap changed 1 verdict in 242,908. So we went with hybrid approach: no token is lost, and 97% of URLs still pay one tokenization.
e993082 to
2c5603a
Compare
Solely to pass failing metrics test.
On-going effort; here's the recent benchmark made against EasyList + EasyPrivacy on this branch:
Adblock engine: EasyList + EasyPrivacy corpus report
Lists downloaded fresh from easylist.to the same day (135,908 lines total). Measured with
the per-request engine (tries + token-bucket indexes) after the adversarial-review fixes.
Coverage: what loads
The 209 network rules we don't apply, by reason
/regex/filters (24 EL + 7 EP) — the only rules uBO enforces asblocks that we genuinely can't: there's still no regex engine. ~0.03% of the
corpus.
$generichide/$elemhide/$specifichide) — they modify cosmetic filtering, which we don't have yet,so there's nothing for them to except.
$removeparam-class) — uBO applies these as URLrewrites, never blocks; dropping them is the conservative, correct choice.
@@…$redirect-rulegoogle-analytics line —correctly treated as excepting nothing, so
google-analytics.comstaysblocked).
The ~10,884 "cosmetic" drops are element-hiding rules (
##,#@#— including944 with spaces in their CSS selectors that take the hosts-file parsing path);
no network engine applies those, so they're out of scope rather than failures.
Notably, zero hostnames land in the
suppressedtrie across the wholecorpus — every exception either evaluates per-request or provably unblocks
nothing.
Correctness on the loaded rules
review fixes in: gpt.js blocked except on the listed carve-out sites, the
/ssai/exception honored, wrong-type/wrong-site/wrong-path variants allcorrect, and the apex
doubleclick.netundecided rather than suppressed.lightpanda fetchon a page loading the real ad stack blocksgpt.js,td./googleads./fls.doubleclick.netand Google Analyticsbefore they reach the network.
Bottom line
99.8% of the network rules in EasyList+EasyPrivacy load and enforce; the
only real gap is 31 regex filters.