[perf] use StackVector to save memory & CPU #553
+142
−48
Conversation
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
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Rust Benchmark
| Benchmark suite | Current: d3df508 | Previous: 3feffc9 | Ratio |
|---|---|---|---|
rule-match-browserlike/brave-list |
2074493135 ns/iter (± 17176018) |
2089868473 ns/iter (± 20161585) |
0.99 |
rule-match-first-request/brave-list |
1105573 ns/iter (± 21061) |
1119548 ns/iter (± 13496) |
0.99 |
blocker_new/brave-list |
138914815 ns/iter (± 1184608) |
155371876 ns/iter (± 816509) |
0.89 |
blocker_new/brave-list-deserialize |
28715496 ns/iter (± 1164468) |
29471071 ns/iter (± 1288134) |
0.97 |
memory-usage/brave-list-initial |
10213344 ns/iter (± 3) |
10213344 ns/iter (± 3) |
1 |
memory-usage/brave-list-initial/max |
60612235 ns/iter (± 3) |
60612235 ns/iter (± 3) |
1 |
memory-usage/brave-list-initial/alloc-count |
996170 ns/iter (± 3) |
1231711 ns/iter (± 3) |
0.81 |
memory-usage/brave-list-1000-requests |
2692712 ns/iter (± 3) |
2692696 ns/iter (± 3) |
1.00 |
memory-usage/brave-list-1000-requests/alloc-count |
69464 ns/iter (± 3) |
71591 ns/iter (± 3) |
0.97 |
url_cosmetic_resources/brave-list |
192596 ns/iter (± 977) |
192025 ns/iter (± 3720) |
1.00 |
cosmetic-class-id-match/brave-list |
3407715 ns/iter (± 928969) |
3479706 ns/iter (± 936426) |
0.98 |
This comment was automatically generated by workflow using github-action-benchmark.
atuchin-m
force-pushed
the
add-stack-vector
branch
from
d1f4e55 to
0558c35
Compare
atuchin-m
force-pushed
the
add-stack-vector
branch
from
0558c35 to
5c1e3bf
Compare
atuchin-m
force-pushed
the
add-stack-vector
branch
from
96e3dfd to
d6a346b
Compare
boocmp
reviewed
Nov 12, 2025
| let msg = "The number of blocked/passed requests has changed. ".to_string() | ||
| + "If this is expected, update the expected values in the test."; | ||
| assert_eq!((blocked, passes), (106860, 136085), "{msg}"); | ||
| assert_eq!((blocked, passes), (106861, 136084), "{msg}"); |
Collaborator
Author
There was a problem hiding this comment.
I mentioned in the PR description: we hits the token limits for one rule.
If you take master and increase the limits, you will get the same behavior.
Collaborator
There was a problem hiding this comment.
nvm, the reason is in the descr
src/flatbuffers/unsafe_tools.rs
Outdated
| } | ||
|
|
||
| impl<T: Default, const MAX_SIZE: usize> StackVector<T, MAX_SIZE> { | ||
| pub fn push(&mut self, value: T) -> bool { |
Collaborator
There was a problem hiding this comment.
Do you mind to panic instead of returning bool ?
Collaborator
Author
There was a problem hiding this comment.
I believe we shouldn't panic by default.
If a caller need, it can use if (!push(..)) { panic!(...); }
boocmp
approved these changes
Nov 12, 2025
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
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.
The PR reduces the amount of allocation by introducing a new struct
StackVectorthat allocates memory outside on a stack instead of a heap.-19%, memory-usage/brave-list-initial/alloc-count);-11-15%, memory-usage/brave-list-initial);Also, I tried to use
MaybeUinit+unsafeto avoid zeroing the memory, but it turns out it doesn't give any profit now, so let's stash that idea for future usage.