perf: More Context benchmarks #2707
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2707 +/- ##
=====================================
Coverage 79.3% 79.3%
=====================================
Files 123 123
Lines 22660 22660
=====================================
Hits 17977 17977
Misses 4683 4683 ☔ View full report in Codecov by Sentry. |
bantonsson
force-pushed
the
ban/context-benchmarks
branch
from
1980b5a to
db82c76
Compare
| }; | ||
|
|
||
| // Run this benchmark with: | ||
| // cargo bench --bench current_context |
There was a problem hiding this comment.
| // cargo bench --bench current_context | |
| // cargo bench --bench context_attach |
There was a problem hiding this comment.
🤦🏼♂️ Thanks. I'll change it.
| context_type: &str, | ||
| context: &Context, | ||
| ) { | ||
| let _restore = Context::current().attach(); |
There was a problem hiding this comment.
| let _restore = Context::current().attach(); | |
| let _context_guard = Context::current().attach(); |
nits!
| fn single_cx(cx: Context) { | ||
| let cx = black_box(cx.attach()); | ||
| let _ = black_box(dummy_work()); | ||
| drop(cx); |
There was a problem hiding this comment.
is it intentional to do explicit drop of cx? Any reason why it is done so?
There was a problem hiding this comment.
The drop will happen anyway, so I can remove it.
| } | ||
|
|
||
| #[inline(never)] | ||
| fn overlapping_cx(cx1: Context, cx2: Context) { |
There was a problem hiding this comment.
nit: Rename to "out_of_order_cx_drop"
cijothomas
left a comment
cijothomas
left a comment
There was a problem hiding this comment.
Looks good!
Left some nits, not a blocker. Eager to see improvements here in upcoming PRs!
|
LGTM too. Once this and #2706 is in you can merge down from main into your context fix branch and we should get the perf change announcements. I'll make sure the other one is ready to go this morning. |
3 participants
Changes
This breaks out the context benchmark work from #2378 so the performance changes can be seen in the PR.
Merge requirement checklist
CHANGELOG.mdfiles updated for non-trivial, user-facing changes