0.11.x merge to master #530
Conversation
The PR moves from per-NetworkList flatbuffers to a one (per-Engine). It doesn't affect the performance metrics, but opens a possibility to put cosmetic filters to the same flatbuffer. It also simplifies the serialization/deserialization code.
* Unsafe tools moved to flatbuffers. * Added FlatMultiMapView, FlatFilterMap replaced. * Added FlatSetView. * Removed unused utils. * Temporary marked FlatSet as dead_code. * Added test for FlatSetView. * Added FlatMultiMapView tests. * Clean up. * Ignore live test. * Clippy fixes. * Review issues are addressed. * Optional iterator for multimap (improve perf). * Removed unused file.
The PR introduces new structures to be used in cosmetic_filters: FlatSerialize trait for things that can be serialized to flatbuffer; Builder structure to help construct a serializable representation + unit tests; Migrate network filter to the new API; Increase MIN_ALIGNMENT to 8 to support using u64 as a key.
The PR introduces new structures to store cosmetic filters in flatbuffer. * The algorithms to sort and apply rules shouldn't be touched, only storage level is changed. * CosmeticFilterCache is now a view for a flatbuffer data. * Old storage layer (via serde) is removed, the version is now stored in the flatbuffer, * Another container 'FlatMap' is introduced + tests * Most host-specific rules are stored in a single FlatMap (domain_hash => HostnameSpecificRules). Although, the most common rule kinds are stored as a dedicated multi-maps to save memory * A code to build flatbuffer structure is moved to dedicated files.
unfortunately this requires duplicating some definitions to support additional `Send + Sync` trait bounds, since Rust does not natively support conditional supertraits.
|
[puLL-Merge] - brave/adblock-rust@530 DescriptionThis PR significantly refactors the internal storage and serialization architecture of the adblock-rust crate. The main changes include:
Possible Issues
Security Hotspots
ChangesChanges
sequenceDiagram
participant User
participant Engine
participant FilterDataContext
participant FlatBufferMemory
participant CosmeticCache
participant ResourceStorage
User->>Engine: from_rules(rules)
Engine->>FilterDataContext: new(flatbuffer_memory)
FilterDataContext->>FlatBufferMemory: create verified memory
Engine->>CosmeticCache: from_context(filter_data_context)
Engine->>ResourceStorage: default (in-memory backend)
User->>Engine: check_network_request(request)
Engine->>FilterDataContext: access network filters
FilterDataContext->>FlatBufferMemory: query flatbuffer data
FlatBufferMemory-->>Engine: filter results
User->>Engine: serialize()
Engine->>FilterDataContext: get flatbuffer data
FilterDataContext-->>Engine: raw flatbuffer bytes
Engine-->>User: serialized .dat file with integrity hash
User->>Engine: deserialize(data)
Engine->>FlatBufferMemory: verify integrity hash
FlatBufferMemory->>FilterDataContext: create new context
Engine->>CosmeticCache: rebuild from new context
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| flatbuffers::Vector<'a, flatbuffers::ForwardsUOffset<&'a str>>, | ||
| >>(StringVector::VT_DATA, None) | ||
| .unwrap() | ||
| } |
There was a problem hiding this comment.
reported by reviewdog 🐶
[semgrep] Detected 'unsafe' usage, please audit for secure usage
Source: https://semgrep.dev/r/rust.lang.security.unsafe-usage.unsafe-usage
Cc @thypon @kdenhartog
| flatbuffers::Vector<'a, flatbuffers::ForwardsUOffset<&'a str>>, | ||
| >>(CosmeticFilters::VT_SIMPLE_CLASS_RULES, None) | ||
| .unwrap() | ||
| } |
There was a problem hiding this comment.
reported by reviewdog 🐶
[semgrep] Detected 'unsafe' usage, please audit for secure usage
Source: https://semgrep.dev/r/rust.lang.security.unsafe-usage.unsafe-usage
Cc @thypon @kdenhartog
| flatbuffers::Vector<'a, flatbuffers::ForwardsUOffset<&'a str>>, | ||
| >>(CosmeticFilters::VT_SIMPLE_ID_RULES, None) | ||
| .unwrap() | ||
| } |
There was a problem hiding this comment.
reported by reviewdog 🐶
[semgrep] Detected 'unsafe' usage, please audit for secure usage
Source: https://semgrep.dev/r/rust.lang.security.unsafe-usage.unsafe-usage
Cc @thypon @kdenhartog
| flatbuffers::Vector<'a, flatbuffers::ForwardsUOffset<&'a str>>, | ||
| >>(CosmeticFilters::VT_MISC_GENERIC_SELECTORS, None) | ||
| .unwrap() | ||
| } |
There was a problem hiding this comment.
reported by reviewdog 🐶
[semgrep] Detected 'unsafe' usage, please audit for secure usage
Source: https://semgrep.dev/r/rust.lang.security.unsafe-usage.unsafe-usage
Cc @thypon @kdenhartog
| flatbuffers::Vector<'a, flatbuffers::ForwardsUOffset<&'a str>>, | ||
| >>(CosmeticFilters::VT_COMPLEX_CLASS_RULES_INDEX, None) | ||
| .unwrap() | ||
| } |
There was a problem hiding this comment.
reported by reviewdog 🐶
[semgrep] Detected 'unsafe' usage, please audit for secure usage
Source: https://semgrep.dev/r/rust.lang.security.unsafe-usage.unsafe-usage
Cc @thypon @kdenhartog
| self._tab.get::<flatbuffers::ForwardsUOffset< | ||
| flatbuffers::Vector<'a, flatbuffers::ForwardsUOffset<&'a str>>, | ||
| >>(HostnameSpecificRules::VT_UNHIDE, None) | ||
| } |
There was a problem hiding this comment.
reported by reviewdog 🐶
[semgrep] Detected 'unsafe' usage, please audit for secure usage
Source: https://semgrep.dev/r/rust.lang.security.unsafe-usage.unsafe-usage
Cc @thypon @kdenhartog
| self._tab.get::<flatbuffers::ForwardsUOffset< | ||
| flatbuffers::Vector<'a, flatbuffers::ForwardsUOffset<&'a str>>, | ||
| >>(HostnameSpecificRules::VT_UNINJECT_SCRIPT, None) | ||
| } |
There was a problem hiding this comment.
reported by reviewdog 🐶
[semgrep] Detected 'unsafe' usage, please audit for secure usage
Source: https://semgrep.dev/r/rust.lang.security.unsafe-usage.unsafe-usage
Cc @thypon @kdenhartog
| self._tab.get::<flatbuffers::ForwardsUOffset< | ||
| flatbuffers::Vector<'a, flatbuffers::ForwardsUOffset<&'a str>>, | ||
| >>(HostnameSpecificRules::VT_PROCEDURAL_ACTION, None) | ||
| } |
There was a problem hiding this comment.
reported by reviewdog 🐶
[semgrep] Detected 'unsafe' usage, please audit for secure usage
Source: https://semgrep.dev/r/rust.lang.security.unsafe-usage.unsafe-usage
Cc @thypon @kdenhartog
| >>( | ||
| HostnameSpecificRules::VT_PROCEDURAL_ACTION_EXCEPTION, None | ||
| ) | ||
| } |
There was a problem hiding this comment.
reported by reviewdog 🐶
[semgrep] Detected 'unsafe' usage, please audit for secure usage
Source: https://semgrep.dev/r/rust.lang.security.unsafe-usage.unsafe-usage
Cc @thypon @kdenhartog
| pub(crate) fn filter_list(&self) -> fb::NetworkFilterList<'_> { | ||
| unsafe { fb::root_as_network_filter_list_unchecked(self.data()) } | ||
| pub(crate) fn root(&self) -> fb::Engine<'_> { | ||
| unsafe { fb::root_as_engine_unchecked(self.data()) } |
There was a problem hiding this comment.
reported by reviewdog 🐶
[semgrep] Detected 'unsafe' usage, please audit for secure usage
Source: https://semgrep.dev/r/rust.lang.security.unsafe-usage.unsafe-usage
Cc @thypon @kdenhartog
There was a problem hiding this comment.
Rust Benchmark
| Benchmark suite | Current: dad4934 | Previous: 1bfadd6 | Ratio |
|---|---|---|---|
rule-match-browserlike/brave-list |
2298565192 ns/iter (± 13731034) |
2157327098 ns/iter (± 15833464) |
1.07 |
rule-match-first-request/brave-list |
1142959 ns/iter (± 9598) |
1067208 ns/iter (± 8765) |
1.07 |
blocker_new/brave-list |
157723755 ns/iter (± 1127633) |
175128732 ns/iter (± 2762769) |
0.90 |
blocker_new/brave-list-deserialize |
24367977 ns/iter (± 501562) |
73769476 ns/iter (± 854420) |
0.33 |
memory-usage/brave-list-initial |
11287711 ns/iter (± 3) |
18344503 ns/iter (± 3) |
0.62 |
memory-usage/brave-list-initial/max |
66961277 ns/iter (± 3) |
66961277 ns/iter (± 3) |
1 |
memory-usage/brave-list-initial/alloc-count |
1635258 ns/iter (± 3) |
1616082 ns/iter (± 3) |
1.01 |
memory-usage/brave-list-1000-requests |
2562777 ns/iter (± 3) |
2551890 ns/iter (± 3) |
1.00 |
memory-usage/brave-list-1000-requests/alloc-count |
69244 ns/iter (± 3) |
68816 ns/iter (± 3) |
1.01 |
url_cosmetic_resources/brave-list |
197905 ns/iter (± 578) |
189708 ns/iter (± 2713) |
1.04 |
cosmetic-class-id-match/brave-list |
3340829 ns/iter (± 968415) |
5166097 ns/iter (± 1523809) |
0.65 |
This comment was automatically generated by workflow using github-action-benchmark.
atuchin-m
force-pushed
the
0.11.x-merge
branch
from
f669e19 to
dad4934
Compare
The commits were rebased in a new branch
0.11.x-merge(to preserve the original commits).