[Draft] Cosmetic filter fb v3: full migration#497
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.
There was a problem hiding this comment.
Rust Benchmark
Details
| Benchmark suite | Current: f087a7b | Previous: 4738d3f | Ratio |
|---|---|---|---|
rule-match-browserlike/brave-list |
2859316369 ns/iter (± 26370025) |
2252126465 ns/iter (± 13369021) |
1.27 |
rule-match-first-request/brave-list |
1093627 ns/iter (± 7231) |
1000284 ns/iter (± 8364) |
1.09 |
blocker_new/brave-list |
153748871 ns/iter (± 1079694) |
150674514 ns/iter (± 1975624) |
1.02 |
blocker_new/brave-list-deserialize |
63334161 ns/iter (± 907905) |
62360204 ns/iter (± 1865060) |
1.02 |
memory-usage/brave-list-initial |
8803460 ns/iter (± 3) |
16225933 ns/iter (± 3) |
0.54 |
memory-usage/brave-list-initial/max |
64817626 ns/iter (± 3) |
64817658 ns/iter (± 3) |
1.00 |
memory-usage/brave-list-initial/alloc-count |
1547531 ns/iter (± 3) |
1514650 ns/iter (± 3) |
1.02 |
memory-usage/brave-list-1000-requests |
2516439 ns/iter (± 3) |
2505592 ns/iter (± 3) |
1.00 |
memory-usage/brave-list-1000-requests/alloc-count |
66508 ns/iter (± 3) |
66070 ns/iter (± 3) |
1.01 |
This comment was automatically generated by workflow using github-action-benchmark.
| 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
| flatbuffers::Vector<'a, flatbuffers::ForwardsUOffset<&'a str>>, | ||
| >>(Engine::VT_COMPLEX_ID_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
| flatbuffers::Vector<'a, flatbuffers::ForwardsUOffset<&'a str>>, | ||
| >>(Engine::VT_COMPLEX_ID_RULES_VALUES, 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
| 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>>, | ||
| >>(Engine::VT_HOSTNAME_HIDE_VALUES, 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>>, | ||
| >>(Engine::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>>, | ||
| >>(Engine::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>>, | ||
| >>(Engine::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>>, | ||
| >>(Engine::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
| flatbuffers::Vector<'a, flatbuffers::ForwardsUOffset<&'a str>>, | ||
| >>(Engine::VT_COMPLEX_CLASS_RULES_VALUES, 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
There was a problem hiding this comment.
⚠️ Performance Alert ⚠️
Possible performance regression was detected for benchmark 'Rust Benchmark'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.10.
| Benchmark suite | Current: f087a7b | Previous: 4738d3f | Ratio |
|---|---|---|---|
rule-match-browserlike/brave-list |
2859316369 ns/iter (± 26370025) |
2252126465 ns/iter (± 13369021) |
1.27 |
This comment was automatically generated by workflow using github-action-benchmark.
11e5634 to
62d4b1f
Compare
| flatbuffers::Vector<'a, flatbuffers::ForwardsUOffset<NetworkFilterList>>, | ||
| >>(Engine::VT_NETWORK_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
| 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
| 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
62d4b1f to
bede9e2
Compare
bede9e2 to
f087a7b
Compare
DO NOT MERGE