[perf] Route FilterSet network rules incrementally during load#675
Open
Tilzen wants to merge 7 commits into
Open
[perf] Route FilterSet network rules incrementally during load#675Tilzen wants to merge 7 commits into
Tilzen wants to merge 7 commits into
Conversation
Avoid holding a monolithic network_filters vec plus a second NetworkRulesBuilder split at Engine creation when debug=false. Rules route into eight lists during add_filter_list/add_filters, NetworkOnly skips cosmetic reserve, and make_flatbuffer_from_compilation serializes the incremental builder directly. Fixes brave#541
Heap flamegraphs from dhat profiles and a CPU phase chart; workflow to capture CPU flamegraphs on CI where perf is available.
Collaborator
|
I suppose this is already resolved in |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #541
Why
Loading
brave-main-list.txtduring Shields startup kept network rules twice in memory: a monolithicVec<NetworkFilter>insideFilterSetCompilation, then a full rebuild intoNetworkRulesBuilderviafrom_rulesatEngine::from_filter_set. Maintainers reported ~67 MB peak and high CPU (brave-browser#50361).What Changed
FilterSetCompilation(debug=false): routes network rules incrementally intoNetworkRulesBuilderduringadd_filter_list/add_filtersinstead of a monolithic vec.make_flatbuffer_from_compilation: serializes the incremental builder directly (no secondfrom_rulespass).add_filter_list: pre-scans$badfilterids, usesread_list_metadatafor header comments, stream-parses lines.RuleTypes::NetworkOnly: skips cosmetic parsing and cosmeticreserve()on the Shields network-only path.Enginehelpers:from_filter_list,from_filter_list_network,from_rules_network.bench_memorynetwork-only max;bench_rulesfilter-set path.Serialized engine hash for brave-list is unchanged (
7678096075090154250).Benchmarks
Environment: Linux 6.1, release builds,
data/brave/brave-main-list.txt(~204k lines).Peak memory
/proc/self/statm, 3 runs)bench_memorymax (memory-usage-max/brave-list-initial/max)bench_memorymax (memory-usage-max/brave-list-network-only/max)Startup CPU (
profile_brave_list_cpu, release, no dhat)add_filter_listfrom_filter_setserializeNetworkOnly total: ~113 ms (~20% faster than master All).
Criterion (bencher output,
--sample-size 10)Trade-off: allocation count rises (~853k → ~2.26M) because rules live in eight routed vecs during load; peak RSS still drops because the duplicate monolithic vec +
from_rulessplit at compile time is eliminated.Profiling (flamegraphs)
Images captured from
data/brave/brave-main-list.txtload path. Heap profiles viadhat+dhat-to-flamegraph(-m heap-max). CPU profiles viacargo flamegraphon GitHub Actions ubuntu (examples/profile_brave_list_cpu).Heap at peak
Before (master)
After (this PR)
CPU flamegraph (Shields path:
add_filter_list→Engine::from_filter_set)Before (master)
After (this PR)
On master,
from_filter_setstill pays forNetworkRulesBuilder::from_rules(re-split of the monolithic vec). On this PR that work moves into incremental routing during parse;from_filter_setis dominated by flatbuffer serialize/optimize instead.Startup CPU phases (ms)
Test Plan
cargo test --all-features --tests(300 passed)RUSTFLAGS='--deny warnings' cargo test --all-features --testscargo fmt --checkdeserialization_brave_listhash unchangedbrave_streaming_matches_vec_path— incremental path matches vec/from_rulesReferences
Notes
optimizeis only known at engine creation,FilterSet::clone(JS bindings) must preserve all rules, and retroactive$badfiltercomplicates already-serialized entries.blocker_new/brave-list(+~5%) and alloc-count; peak memory benchmarks should improve.docs/perf/issue-541/images/on the PR branch; CPU flamegraphs were captured via.github/workflows/capture-flamegraphs.ymlon the fork (ubuntu + perf).