-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Cache construction of middleware handlers #9158
Conversation
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 #9158 +/- ##
=======================================
Coverage 98.31% 98.31%
=======================================
Files 107 107
Lines 34461 34469 +8
Branches 4089 4092 +3
=======================================
+ Hits 33879 33887 +8
Misses 411 411
Partials 171 171
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Thanks for the quick look. Leaving now so I'll try to finish up later this week before I get back from holiday. |
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.
Doesnt the cache need to be invalidated if an additional middleware is added at some late stage? (Know too little about internals to know if this can happen)
They're frozen, they can't be modified once the app is started. |
Need to look at the lru stats once I have stable internet |
|
Backport to 3.10: 💔 cherry-picking failed — conflicts found❌ Failed to cleanly apply bf022b3 on top of patchback/backports/3.10/bf022b390ee97172d20c0f554b87868d0ef4d938/pr-9158 Backporting merged PR #9158 into master
🤖 @patchback |
Backport to 3.11: 💔 cherry-picking failed — conflicts found❌ Failed to cleanly apply bf022b3 on top of patchback/backports/3.11/bf022b390ee97172d20c0f554b87868d0ef4d938/pr-9158 Backporting merged PR #9158 into master
🤖 @patchback |
(cherry picked from commit bf022b3)
(cherry picked from commit bf022b3)
In post merge self re-review of #9158, I noticed I should have moved the reversing into the cache
In post merge self re-review of #9158, I noticed I should have moved the reversing into the cache
What do these changes do?
Cache construction of middleware. Calling
update_wrapper
many times for each request with middleware is more expensive than running all the middlewaresThe
update_wrapper
call was introduced in #4195 which is what made middleware much slower.Are there changes in behavior for the user?
No
Is it a substantial burden for the maintainers to support this?
no
Before
After