[issue42862] Use functools.lru_cache iso. _sqlite.Cache in sqlite3 module

[Dong-hee Na]

Dong-hee Na  added the comment:


New changeset 34356a0a4bad0be124ae892cda6c30a38f5f1061 by Erlend Egeberg 
Aasland in branch 'main':
bpo-42862: Strip stale sqlite3 cache ignores from c-analyzer (GH-26876)
https://github.com/python/cpython/commit/34356a0a4bad0be124ae892cda6c30a38f5f1061


--
nosy: +corona10

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42862] Use functools.lru_cache iso. _sqlite.Cache in sqlite3 module

[Erlend E. Aasland]

Change by Erlend E. Aasland :


--
pull_requests: +25452
pull_request: https://github.com/python/cpython/pull/26876

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42862] Use functools.lru_cache iso. _sqlite.Cache in sqlite3 module

[Pablo Galindo Salgado]

Change by Pablo Galindo Salgado :


--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42862] Use functools.lru_cache iso. _sqlite.Cache in sqlite3 module

[Pablo Galindo Salgado]

Pablo Galindo Salgado  added the comment:


New changeset f461a7fc3f8740b9e79e8874175115a3474e5930 by Erlend Egeberg 
Aasland in branch 'main':
bpo-42862: Use functools.lru_cache iso. _sqlite.Cache in sqlite3 module 
(GH-24203)
https://github.com/python/cpython/commit/f461a7fc3f8740b9e79e8874175115a3474e5930


--
nosy: +pablogsal

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42862] Use functools.lru_cache iso. _sqlite.Cache in sqlite3 module

[Berker Peksag]

Berker Peksag  added the comment:

> Or is it ok to call gc.collect() in the test suite?

It is but in this case I'd say it's a bit weird that we need to call it. 
Unfortunately, I don't have much time to investigate it at the moment.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42862] Use functools.lru_cache iso. _sqlite.Cache in sqlite3 module

[Erlend E. Aasland]

Change by Erlend E. Aasland :


--
versions: +Python 3.11 -Python 3.10

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42862] Use functools.lru_cache iso. _sqlite.Cache in sqlite3 module

[Erlend Egeberg Aasland]

Erlend Egeberg Aasland  added the comment:

> Or is it ok to call gc.collect() in the test suite?

Seems like it's ok:

$ grep -r gc.collect Lib/test | wc -l
366

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42862] Use functools.lru_cache iso. _sqlite.Cache in sqlite3 module

[Erlend Egeberg Aasland]

Erlend Egeberg Aasland  added the comment:

This works:

1) fully implement GC in connection (bpo-42972)
2) also visit statement_cache
3) explicitly close connections _and_ call GC in problematic tests

The first point might not be needed for this particular fix.
The last point is a workaround, not a solution. Or is it ok to call 
gc.collect() in the test suite?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42862] Use functools.lru_cache iso. _sqlite.Cache in sqlite3 module

[Erlend Egeberg Aasland]

Erlend Egeberg Aasland  added the comment:

Tried applying bpo-42972 to sqlite and functools, but the error persists.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42862] Use functools.lru_cache iso. _sqlite.Cache in sqlite3 module

[Erlend Egeberg Aasland]

Erlend Egeberg Aasland  added the comment:

I need some help debugging the Windows issues. There are a handful of tests 
that fail because the sqlite3 is clinging on to objects or file descriptors. 
AFAICT, a GC bug. For example, test_open_uri fails with:

PermissionError: [WinError 32] The process cannot access the file because it is 
being used by another process: [...]

I've tried the following, with no success:
- Remove GC from the Connection type
- Remove the clear method, and modify traverse to only visit Py_TYPE(self) 
(https://bugs.python.org/issue42866#msg384675)
- Add heap type GC (visit type, GC tracking) to all pysqlite types

Maybe it's best to reopen GH-24135 and continue the work with multi-phase init? 
That is finalise bpo-40956, add heap type GC 
(https://bugs.python.org/issue42866#msg384675), establish module state, 
implement multi-phase init, and then revisit this issue.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42862] Use functools.lru_cache iso. _sqlite.Cache in sqlite3 module

[Erlend Egeberg Aasland]

Change by Erlend Egeberg Aasland :


--
pull_requests: +23028
pull_request: https://github.com/python/cpython/pull/24203

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42862] Use functools.lru_cache iso. _sqlite.Cache in sqlite3 module

[Erlend Egeberg Aasland]

Erlend Egeberg Aasland  added the comment:

> I do not like using _functools._lru_cache_wrapper. It is a deep 
> implementation detail, private function of private module. Use 
> functools.lru_cache.

All right, thanks.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42862] Use functools.lru_cache iso. _sqlite.Cache in sqlite3 module

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

I do not like using _functools._lru_cache_wrapper. It is a deep implementation 
detail, private function of private module. Use functools.lru_cache. If it is 
few nanoseconds slower, that cost is only added at connection creation time. It 
is insignificant in any real application in comparison with IO and any real 
work with data. Thousands of short-living in-memory DB instances are only 
created in tests.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42862] Use functools.lru_cache iso. _sqlite.Cache in sqlite3 module

[Erlend Egeberg Aasland]

Erlend Egeberg Aasland  added the comment:

True that :) I'll close GH-24135 for now and open a PR for this later today.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42862] Use functools.lru_cache iso. _sqlite.Cache in sqlite3 module

[Berker Peksag]

Berker Peksag  added the comment:

We can always reopen GH-24135 and merge it even if we revert this one for some 
reason :)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42862] Use functools.lru_cache iso. _sqlite.Cache in sqlite3 module

[Erlend Egeberg Aasland]

Erlend Egeberg Aasland  added the comment:

> I don't see any reason to merge GH-24135 if we are going to remove cache.[ch] 
> in this issue.

Yes, I've thought about that myself. A small argument pro merging GH-24135 
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).

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42862] Use functools.lru_cache iso. _sqlite.Cache in sqlite3 module

[Berker Peksag]

Berker Peksag  added the comment:

I don't see any reason to merge GH-24135 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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42862] Use functools.lru_cache iso. _sqlite.Cache in sqlite3 module

[Erlend Egeberg Aasland]

Erlend Egeberg Aasland  added the comment:

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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42862] Use functools.lru_cache iso. _sqlite.Cache in sqlite3 module

[Raymond Hettinger]

Raymond Hettinger  added the comment:

+1 This seems reasonable to me.

--
nosy: +rhettinger

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42862] Use functools.lru_cache iso. _sqlite.Cache in sqlite3 module

[Erlend Egeberg Aasland]

Change by Erlend Egeberg Aasland :


--
keywords: +patch
pull_requests: +23021
stage:  -> patch review
pull_request: https://github.com/python/cpython/pull/24135

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42862] Use functools.lru_cache iso. _sqlite.Cache in sqlite3 module

[Erlend Egeberg Aasland]

Erlend Egeberg Aasland  added the comment:

$ python3.10 -m timeit -s 'import sqlite3; con = sqlite3.connect(":memory:"); 
query="select * from sqlite_master"' 'con.execute(query); con.execute(query)'
10 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)'
10 loops, best of 5: 2.68 usec per loop

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42862] Use functools.lru_cache iso. _sqlite.Cache in sqlite3 module

[Erlend Egeberg Aasland]

Erlend Egeberg Aasland  added the comment:

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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42862] Use functools.lru_cache iso. _sqlite.Cache in sqlite3 module

[Erlend Egeberg Aasland]

New submission from Erlend Egeberg 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?

--
components: Library (Lib)
messages: 384625
nosy: berker.peksag, erlendaasland, serhiy.storchaka
priority: normal
severity: normal
status: open
title: Use functools.lru_cache iso. _sqlite.Cache in sqlite3 module
type: enhancement
versions: Python 3.10

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com