Use functools.lru_cache iso. _sqlite.Cache in sqlite3 module

[erlend-aasland]

BPO	42862
Nosy	@rhettinger, @berkerpeksag, @serhiy-storchaka, @corona10, @pablogsal, @erlend-aasland
PRs	bpo-40956: Convert _sqlite3.Cache to Argument Clinic #24135 bpo-42862: Use functools.lru_cache iso. _sqlite.Cache in sqlite3 module #24203 bpo-42862: Strip stale sqlite3 cache ignores from c-analyzer #26876

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2021-06-03.20:01:40.172>
created_at = <Date 2021-01-08.00:39:03.919>
labels = ['type-feature', 'library', '3.11']
title = 'Use functools.lru_cache iso. _sqlite.Cache in sqlite3 module'
updated_at = <Date 2021-06-23.14:04:34.134>
user = 'https://github.com/erlend-aasland'

bugs.python.org fields:

activity = <Date 2021-06-23.14:04:34.134>
actor = 'corona10'
assignee = 'none'
closed = True
closed_date = <Date 2021-06-03.20:01:40.172>
closer = 'pablogsal'
components = ['Library (Lib)']
creation = <Date 2021-01-08.00:39:03.919>
creator = 'erlendaasland'
dependencies = []
files = []
hgrepos = []
issue_num = 42862
keywords = ['patch']
message_count = 18.0
messages = ['384625', '384627', '384691', '384892', '384923', '384926', '384927', '384928', '384929', '384935', '384976', '385193', '385376', '385444', '385445', '393932', '395039', '396422']
nosy_count = 6.0
nosy_names = ['rhettinger', 'berker.peksag', 'serhiy.storchaka', 'corona10', 'pablogsal', 'erlendaasland']
pr_nums = ['24135', '24203', '26876']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue42862'
versions = ['Python 3.11']

[erlend-aasland]

Pro: less code, less complexity, improved maintainability
Con: minor performance hit

PoC here: https://github.com/erlend-aasland/cpython/commits/sqlite-cache

$ ./python.exe
>>> import sqlite3
>>> con = sqlite3.connect(":memory:")
>>> con.execute("select * from sqlite_master")
>>> con.execute("select * from sqlite_master")
>>> c = con.cache()
>>> c.cache_info()
CacheInfo(hits=1, misses=1, maxsize=128, currsize=1)

The test suite runs approx. 10-20 ms slower with this change. Using _functools._lru_cache_wrapper iso. functools.lru_cache almost removes this performance regression.

Berker, is it worth pursuing?

[erlend-aasland]

Short diffstat: 8 files changed, 85 insertions(+), 406 deletions(-)

[erlend-aasland]

$ python3.10 -m timeit -s 'import sqlite3; con = sqlite3.connect(":memory:"); query="select * from sqlite_master"' 'con.execute(query); con.execute(query)'
100000 loops, best of 5: 2.95 usec per loop
$ ./python.exe -m timeit -s 'import sqlite3; con = sqlite3.connect(":memory:"); query="select * from sqlite_master"' 'con.execute(query); con.execute(query)'
100000 loops, best of 5: 2.68 usec per loop

[rhettinger]

+1 This seems reasonable to me.

[erlend-aasland]

I can throw up the PoC branch as a draft PR after #68323 is merged. We can just close the PR if this is uninteresting or something we want to postpone.

[berkerpeksag]

I don't see any reason to merge #68323 if we are going to remove cache.[ch] in this issue.

I was -0 before but since Raymond gave his +1, you can count me as +1 too.

[erlend-aasland]

I don't see any reason to merge #68323 if we are going to remove cache.[ch] in this issue.

Yes, I've thought about that myself. A small argument pro merging #68323 would be that if we for some reason decide to revert this change, then cache.[ch] would still be prepared for module state (which is needed for multi-phase init).

[berkerpeksag]

We can always reopen #68323 and merge it even if we revert this one for some reason :)

[erlend-aasland]

True that :) I'll close #68323 for now and open a PR for this later today.