-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Avoid fetching loop time on each request unless logging is enabled #10713
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
CodSpeed Performance ReportMerging #10713 will not alter performanceComparing Summary
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## master #10713 +/- ##
=======================================
Coverage 98.72% 98.72%
=======================================
Files 125 125
Lines 37733 37751 +18
Branches 2082 2081 -1
=======================================
+ Hits 37251 37271 +20
+ Misses 334 333 -1
+ Partials 148 147 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
This comment was marked as outdated.
This comment was marked as outdated.
I still need to write another test for this one. I keep getting pulled on to other tasks though |
I was looking for a missing partial shown on the previous commit, but it's gone after merging master, so all good. |
Backport to 3.11: 💔 cherry-picking failed — conflicts found❌ Failed to cleanly apply 8d74e26 on top of patchback/backports/3.11/8d74e26c70adeabda54c963d4744672c1874cb06/pr-10713 Backporting merged PR #10713 into master
🤖 @patchback |
Backport to 3.12: 💔 cherry-picking failed — conflicts found❌ Failed to cleanly apply 8d74e26 on top of patchback/backports/3.12/8d74e26c70adeabda54c963d4744672c1874cb06/pr-10713 Backporting merged PR #10713 into master
🤖 @patchback |
…equest unless logging is enabled (#10738) Co-authored-by: Sam Bull <git@sambull.org>
…equest unless logging is enabled (#10739) Co-authored-by: Sam Bull <git@sambull.org>
We would call
loop.time()
and than throw it away most of the time because logging was not enabled by default. This makes a syscall (or VDSO on more performant systems) and can be upwards of 8% of the request overhead on some ARM SBCs because of ARM errata that prevents VDSO from working (Cortex-A*3 and others)Note that this will not show a performance difference with codspeed since it excludes syscalls. It will need to be checked manually.
Looks to be ~2% speed up of request handling time on systems that support VDSO, and ~8% speed up on systems that don't


We still have one for the keep alive that we likely can't get rid of