Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34k
bpo-42862: Use functools.lru_cache iso. _sqlite.Cache in sqlite3 module#24203
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
Uh oh!
There was an error while loading. Please reload this page.
Conversation
erlend-aasland commented Jan 12, 2021 • edited by bedevere-bot
Loading Uh oh!
There was an error while loading. Please reload this page.
edited by bedevere-bot
Uh oh!
There was an error while loading. Please reload this page.
erlend-aasland commented Jan 12, 2021
9fa24b1 to 8f8694aCompareerlend-aasland commented Feb 9, 2021
@rhettinger Would you mind reviewing the use of |
2cceb7f to ec6485fCompare6cf36b2 to c79c3a9Compareerlend-aasland commented Mar 22, 2021
FYI @berkerpeksag, I've rebased onto |
c79c3a9 to 3d85a78Compareerlend-aasland commented May 2, 2021 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
Rebased onto master to resolve conflicts. Also added a tiny optimisation: increase the default cache size from 100 to 128. I'm guessing it is way to late to get this into 3.10 now :) |
6ceadcd to c561babCompareerlend-aasland commented May 18, 2021
@pablogsal, two questions:
|
Uh oh!
There was an error while loading. Please reload this page.
pablogsal commented May 19, 2021
Unless you are changing the module itself, is not very relevant, but you are always free to ask for his advice :)
That seems like some genuine problem. Maybe @vstinner has seen this before? |
Modules/_sqlite/connection.c Outdated
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.
Be careful with static globals as this is not subinterpreter-friendly. @vstinner or @ericsnowcurrently can explain further.
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.
Currently, there's no module state in sqlite3. Converting the sqlite3 state to heap types has been the first step on the path to sqlite3 multi-phase initialisation and module state. I can use this PR to establish a static global state, and put _lru_cache there.
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.
I added a global state struct and put _lru_cache there. I can add the rest of the static data to it in a separate PR. One step closer to multi-phase init and sub-interpreter support.
See bb186dca837c805cf5d78ae7675d8a43a0b217cd
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
pablogsal commented May 19, 2021
When you are done with the fixes, could you test with the buildbots using the buildbot label? |
erlend-aasland commented May 25, 2021
I messed up the previous merge so I had to rebase and force push. Sorry 'bout the mess. |
Uh oh!
There was an error while loading. Please reload this page.
bedevere-bot commented Jun 2, 2021
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
pablogsal commented Jun 3, 2021
@erlend-aasland is this still a draft (I assume not)? :) |
erlend-aasland commented Jun 3, 2021
Nope, I was waiting for the CI, just to be sure :) |
pablogsal commented Jun 3, 2021
Great job @erlend-aasland! 🎉 |
erlend-aasland commented Jun 3, 2021
Thanks, Pablo! 😃 |
https://bugs.python.org/issue42862