-
Notifications
You must be signed in to change notification settings - Fork 63
feat: dont reorganize __cache if unbounded #696
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
base: master
Are you sure you want to change the base?
feat: dont reorganize __cache if unbounded #696
Conversation
|
Since it's an internal-only attribute, can we use a builtins.dict instead of the OrderedDict in cases where maxsize is None? I believe that was the only blocker for implementing #144 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #696 +/- ##
==========================================
- Coverage 97.22% 96.47% -0.76%
==========================================
Files 12 12
Lines 793 794 +1
Branches 41 42 +1
==========================================
- Hits 771 766 -5
Misses 20 20
- Partials 2 8 +6
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
CodSpeed Performance ReportMerging #696 will not alter performanceComparing Summary
|
|
The benchmarks on this one look like there's a possibility this actually reduces performance slightly (I tried rerunning the benchmarks to check if it's random, but codspeed doesn't seem to update with the latest upload). The other PRs also seem to show no discernible change in performance. |
|
You can force codspeed to rerun and update the dashboard with an empty commit, they use the commit hash as the key so if you run it twice on the same commit nothing happens iirc
I did notice this as well, though when I was looking at the flamegraph data I noticed that there's substantially more time taken by asyncio than by async-lru. I'd like to leave these open for now and think about how to best reduce the amount of asyncio stuff in the benchmark probably by prebuilding futures and waiting until after the benchmark to deallocate them, among other potentials |
What do these changes do?
If a cache wrapper does not have a maxsize, there isn't any reason to reorganize the OrderedDict.
Let's wait until the codspeed stuff is merged before we look at this one.
Are there changes in behavior for the user?
None.
Related issue number
N/A
Checklist