-
Notifications
You must be signed in to change notification settings - Fork 177
Pre release 0.11.x fixes #531
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rust Benchmark
| Benchmark suite | Current: 2c4a433 | Previous: 71a1a48 | Ratio |
|---|---|---|---|
rule-match-browserlike/brave-list |
2271643196 ns/iter (± 14735781) |
2238088933 ns/iter (± 14469145) |
1.01 |
rule-match-first-request/brave-list |
1121225 ns/iter (± 8517) |
1115051 ns/iter (± 24675) |
1.01 |
blocker_new/brave-list |
152141429 ns/iter (± 1011080) |
153552333 ns/iter (± 606844) |
0.99 |
blocker_new/brave-list-deserialize |
23077399 ns/iter (± 52090) |
23594962 ns/iter (± 54120) |
0.98 |
memory-usage/brave-list-initial |
11315143 ns/iter (± 3) |
11287711 ns/iter (± 3) |
1.00 |
memory-usage/brave-list-initial/max |
66961309 ns/iter (± 3) |
66961277 ns/iter (± 3) |
1.00 |
memory-usage/brave-list-initial/alloc-count |
1635306 ns/iter (± 3) |
1635258 ns/iter (± 3) |
1.00 |
memory-usage/brave-list-1000-requests |
2562825 ns/iter (± 3) |
2562777 ns/iter (± 3) |
1.00 |
memory-usage/brave-list-1000-requests/alloc-count |
69292 ns/iter (± 3) |
69244 ns/iter (± 3) |
1.00 |
url_cosmetic_resources/brave-list |
196947 ns/iter (± 825) |
195856 ns/iter (± 597) |
1.01 |
cosmetic-class-id-match/brave-list |
3396292 ns/iter (± 943684) |
3422837 ns/iter (± 951785) |
0.99 |
This comment was automatically generated by workflow using github-action-benchmark.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
⚠️ 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: e045807 | Previous: 71a1a48 | Ratio |
|---|---|---|---|
blocker_new/brave-list-deserialize |
28229342 ns/iter (± 778186) |
23594962 ns/iter (± 54120) |
1.20 |
This comment was automatically generated by workflow using github-action-benchmark.
f011755 to
e045807
Compare
| /// Decodes permission bits from the last byte of a script string | ||
| /// Decodes permission bits from the last char of a script string | ||
| /// Returns (permission, script) tuple | ||
| pub(crate) fn decode_script_with_permission(encoded_script: &str) -> (PermissionMask, &str) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good catch. Can we change encoded script + permissions to be &[u8] in all instances? Should make it harder to misuse, especially considering that Rust APIs widely assume a &str is valid Unicode data.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately, that's problematic: it hits the flatbuffer format limitations. It doesn't support arrays of arrays, so we have to wrap it in a new table. That will consume extra memory and makes the code less clear.
The PR:
decode_script_with_permissionissues with somePermissionMask;ResourceImplandEngineDebugInfo.P.S. the perf issue with
blocker_new/brave-list-deserializeisn't relevant (checked locally). It's likely a fast runner was used for reference.