From 9c24ebc190e9516bb3a2514e70a010c78b3cda43 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Sat, 26 Dec 2020 20:58:24 +0100 Subject: [PATCH 01/16] Use functools.lru_cache iso. _sqlite.Cache/Node --- Modules/_sqlite/cache.c | 349 -------------------------- Modules/_sqlite/cache.h | 68 ----- Modules/_sqlite/clinic/connection.c.h | 20 +- Modules/_sqlite/connection.c | 66 ++++- Modules/_sqlite/connection.h | 4 +- Modules/_sqlite/cursor.c | 23 +- Modules/_sqlite/module.c | 6 +- PCbuild/_sqlite3.vcxproj | 2 - PCbuild/_sqlite3.vcxproj.filters | 6 - setup.py | 2 +- 10 files changed, 91 insertions(+), 455 deletions(-) delete mode 100644 Modules/_sqlite/cache.c delete mode 100644 Modules/_sqlite/cache.h diff --git a/Modules/_sqlite/cache.c b/Modules/_sqlite/cache.c deleted file mode 100644 index e0a1707348c912..00000000000000 --- a/Modules/_sqlite/cache.c +++ /dev/null @@ -1,349 +0,0 @@ -/* cache .c - a LRU cache - * - * Copyright (C) 2004-2010 Gerhard Häring - * - * This file is part of pysqlite. - * - * This software is provided 'as-is', without any express or implied - * warranty. In no event will the authors be held liable for any damages - * arising from the use of this software. - * - * Permission is granted to anyone to use this software for any purpose, - * including commercial applications, and to alter it and redistribute it - * freely, subject to the following restrictions: - * - * 1. The origin of this software must not be misrepresented; you must not - * claim that you wrote the original software. If you use this software - * in a product, an acknowledgment in the product documentation would be - * appreciated but is not required. - * 2. Altered source versions must be plainly marked as such, and must not be - * misrepresented as being the original software. - * 3. This notice may not be removed or altered from any source distribution. - */ - -#include "cache.h" -#include - -/* only used internally */ -static pysqlite_Node * -pysqlite_new_node(PyObject *key, PyObject *data) -{ - pysqlite_Node* node; - - node = (pysqlite_Node*) (pysqlite_NodeType->tp_alloc(pysqlite_NodeType, 0)); - if (!node) { - return NULL; - } - - node->key = Py_NewRef(key); - node->data = Py_NewRef(data); - - node->prev = NULL; - node->next = NULL; - - return node; -} - -static int -node_traverse(pysqlite_Node *self, visitproc visit, void *arg) -{ - Py_VISIT(self->key); - Py_VISIT(self->data); - Py_VISIT(Py_TYPE(self)); - return 0; -} - -static int -node_clear(pysqlite_Node *self) -{ - Py_CLEAR(self->key); - Py_CLEAR(self->data); - return 0; -} - -static void -pysqlite_node_dealloc(pysqlite_Node *self) -{ - PyTypeObject *tp = Py_TYPE(self); - PyObject_GC_UnTrack(self); - tp->tp_clear((PyObject *)self); - tp->tp_free(self); - Py_DECREF(tp); -} - -static int -pysqlite_cache_init(pysqlite_Cache *self, PyObject *args, PyObject *kwargs) -{ - PyObject* factory; - int size = 10; - - self->factory = NULL; - - if (!PyArg_ParseTuple(args, "O|i", &factory, &size)) { - return -1; - } - - /* minimum cache size is 5 entries */ - if (size < 5) { - size = 5; - } - self->size = size; - self->first = NULL; - self->last = NULL; - - self->mapping = PyDict_New(); - if (!self->mapping) { - return -1; - } - - self->factory = Py_NewRef(factory); - - self->decref_factory = 1; - - return 0; -} - -static int -cache_traverse(pysqlite_Cache *self, visitproc visit, void *arg) -{ - pysqlite_Node *node = self->first; - while (node) { - Py_VISIT(node); - node = node->next; - } - Py_VISIT(self->mapping); - if (self->decref_factory) { - Py_VISIT(self->factory); - } - Py_VISIT(Py_TYPE(self)); - return 0; -} - -static int -cache_clear(pysqlite_Cache *self) -{ - /* iterate over all nodes and deallocate them */ - pysqlite_Node *node = self->first; - self->first = NULL; - while (node) { - pysqlite_Node *delete_node = node; - node = node->next; - Py_CLEAR(delete_node); - } - if (self->decref_factory) { - Py_CLEAR(self->factory); - } - Py_CLEAR(self->mapping); - return 0; -} - -static void -pysqlite_cache_dealloc(pysqlite_Cache *self) -{ - if (!self->factory) { - /* constructor failed, just get out of here */ - return; - } - - PyObject_GC_UnTrack(self); - PyTypeObject *tp = Py_TYPE(self); - tp->tp_clear((PyObject *)self); - tp->tp_free(self); - Py_DECREF(tp); -} - -PyObject* pysqlite_cache_get(pysqlite_Cache* self, PyObject* key) -{ - pysqlite_Node* node; - pysqlite_Node* ptr; - PyObject* data; - - node = (pysqlite_Node*)PyDict_GetItemWithError(self->mapping, key); - if (node) { - /* an entry for this key already exists in the cache */ - - /* increase usage counter of the node found */ - if (node->count < LONG_MAX) { - node->count++; - } - - /* if necessary, reorder entries in the cache by swapping positions */ - if (node->prev && node->count > node->prev->count) { - ptr = node->prev; - - while (ptr->prev && node->count > ptr->prev->count) { - ptr = ptr->prev; - } - - if (node->next) { - node->next->prev = node->prev; - } else { - self->last = node->prev; - } - if (node->prev) { - node->prev->next = node->next; - } - if (ptr->prev) { - ptr->prev->next = node; - } else { - self->first = node; - } - - node->next = ptr; - node->prev = ptr->prev; - if (!node->prev) { - self->first = node; - } - ptr->prev = node; - } - } - else if (PyErr_Occurred()) { - return NULL; - } - else { - /* There is no entry for this key in the cache, yet. We'll insert a new - * entry in the cache, and make space if necessary by throwing the - * least used item out of the cache. */ - - if (PyDict_GET_SIZE(self->mapping) == self->size) { - if (self->last) { - node = self->last; - - if (PyDict_DelItem(self->mapping, self->last->key) != 0) { - return NULL; - } - - if (node->prev) { - node->prev->next = NULL; - } - self->last = node->prev; - node->prev = NULL; - - Py_DECREF(node); - } - } - - /* We cannot replace this by PyObject_CallOneArg() since - * PyObject_CallFunction() has a special case when using a - * single tuple as argument. */ - data = PyObject_CallFunction(self->factory, "O", key); - - if (!data) { - return NULL; - } - - node = pysqlite_new_node(key, data); - if (!node) { - return NULL; - } - node->prev = self->last; - - Py_DECREF(data); - - if (PyDict_SetItem(self->mapping, key, (PyObject*)node) != 0) { - Py_DECREF(node); - return NULL; - } - - if (self->last) { - self->last->next = node; - } else { - self->first = node; - } - self->last = node; - } - - return Py_NewRef(node->data); -} - -static PyObject * -pysqlite_cache_display(pysqlite_Cache *self, PyObject *args) -{ - pysqlite_Node* ptr; - PyObject* prevkey; - PyObject* nextkey; - PyObject* display_str; - - ptr = self->first; - - while (ptr) { - if (ptr->prev) { - prevkey = ptr->prev->key; - } else { - prevkey = Py_None; - } - - if (ptr->next) { - nextkey = ptr->next->key; - } else { - nextkey = Py_None; - } - - display_str = PyUnicode_FromFormat("%S <- %S -> %S\n", - prevkey, ptr->key, nextkey); - if (!display_str) { - return NULL; - } - PyObject_Print(display_str, stdout, Py_PRINT_RAW); - Py_DECREF(display_str); - - ptr = ptr->next; - } - - Py_RETURN_NONE; -} - -static PyType_Slot node_slots[] = { - {Py_tp_dealloc, pysqlite_node_dealloc}, - {Py_tp_traverse, node_traverse}, - {Py_tp_clear, node_clear}, - {0, NULL}, -}; - -static PyType_Spec node_spec = { - .name = MODULE_NAME ".Node", - .basicsize = sizeof(pysqlite_Node), - .flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE | Py_TPFLAGS_HAVE_GC, - .slots = node_slots, -}; -PyTypeObject *pysqlite_NodeType = NULL; - -static PyMethodDef cache_methods[] = { - {"get", (PyCFunction)pysqlite_cache_get, METH_O, - PyDoc_STR("Gets an entry from the cache or calls the factory function to produce one.")}, - {"display", (PyCFunction)pysqlite_cache_display, METH_NOARGS, - PyDoc_STR("For debugging only.")}, - {NULL, NULL} -}; - -static PyType_Slot cache_slots[] = { - {Py_tp_dealloc, pysqlite_cache_dealloc}, - {Py_tp_methods, cache_methods}, - {Py_tp_init, pysqlite_cache_init}, - {Py_tp_traverse, cache_traverse}, - {Py_tp_clear, cache_clear}, - {0, NULL}, -}; - -static PyType_Spec cache_spec = { - .name = MODULE_NAME ".Cache", - .basicsize = sizeof(pysqlite_Cache), - .flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE | Py_TPFLAGS_HAVE_GC, - .slots = cache_slots, -}; -PyTypeObject *pysqlite_CacheType = NULL; - -int -pysqlite_cache_setup_types(PyObject *mod) -{ - pysqlite_NodeType = (PyTypeObject *)PyType_FromModuleAndSpec(mod, &node_spec, NULL); - if (pysqlite_NodeType == NULL) { - return -1; - } - - pysqlite_CacheType = (PyTypeObject *)PyType_FromModuleAndSpec(mod, &cache_spec, NULL); - if (pysqlite_CacheType == NULL) { - return -1; - } - return 0; -} diff --git a/Modules/_sqlite/cache.h b/Modules/_sqlite/cache.h deleted file mode 100644 index 083356f93f9e4c..00000000000000 --- a/Modules/_sqlite/cache.h +++ /dev/null @@ -1,68 +0,0 @@ -/* cache.h - definitions for the LRU cache - * - * Copyright (C) 2004-2010 Gerhard Häring - * - * This file is part of pysqlite. - * - * This software is provided 'as-is', without any express or implied - * warranty. In no event will the authors be held liable for any damages - * arising from the use of this software. - * - * Permission is granted to anyone to use this software for any purpose, - * including commercial applications, and to alter it and redistribute it - * freely, subject to the following restrictions: - * - * 1. The origin of this software must not be misrepresented; you must not - * claim that you wrote the original software. If you use this software - * in a product, an acknowledgment in the product documentation would be - * appreciated but is not required. - * 2. Altered source versions must be plainly marked as such, and must not be - * misrepresented as being the original software. - * 3. This notice may not be removed or altered from any source distribution. - */ - -#ifndef PYSQLITE_CACHE_H -#define PYSQLITE_CACHE_H -#include "module.h" - -/* The LRU cache is implemented as a combination of a doubly-linked with a - * dictionary. The list items are of type 'Node' and the dictionary has the - * nodes as values. */ - -typedef struct _pysqlite_Node -{ - PyObject_HEAD - PyObject* key; - PyObject* data; - long count; - struct _pysqlite_Node* prev; - struct _pysqlite_Node* next; -} pysqlite_Node; - -typedef struct -{ - PyObject_HEAD - int size; - - /* a dictionary mapping keys to Node entries */ - PyObject* mapping; - - /* the factory callable */ - PyObject* factory; - - pysqlite_Node* first; - pysqlite_Node* last; - - /* if set, decrement the factory function when the Cache is deallocated. - * this is almost always desirable, but not in the pysqlite context */ - int decref_factory; -} pysqlite_Cache; - -extern PyTypeObject *pysqlite_NodeType; -extern PyTypeObject *pysqlite_CacheType; - -PyObject* pysqlite_cache_get(pysqlite_Cache* self, PyObject* args); - -int pysqlite_cache_setup_types(PyObject *module); - -#endif diff --git a/Modules/_sqlite/clinic/connection.c.h b/Modules/_sqlite/clinic/connection.c.h index f231ecc2ae78be..c42285c44a85c1 100644 --- a/Modules/_sqlite/clinic/connection.c.h +++ b/Modules/_sqlite/clinic/connection.c.h @@ -2,6 +2,24 @@ preserve [clinic start generated code]*/ +PyDoc_STRVAR(pysqlite_connection_statement_cache__doc__, +"statement_cache($self, /)\n" +"--\n" +"\n" +"Return the connection statement cache."); + +#define PYSQLITE_CONNECTION_STATEMENT_CACHE_METHODDEF \ + {"statement_cache", (PyCFunction)pysqlite_connection_statement_cache, METH_NOARGS, pysqlite_connection_statement_cache__doc__}, + +static PyObject * +pysqlite_connection_statement_cache_impl(pysqlite_Connection *self); + +static PyObject * +pysqlite_connection_statement_cache(pysqlite_Connection *self, PyObject *Py_UNUSED(ignored)) +{ + return pysqlite_connection_statement_cache_impl(self); +} + PyDoc_STRVAR(pysqlite_connection_cursor__doc__, "cursor($self, /, factory=)\n" "--\n" @@ -710,4 +728,4 @@ pysqlite_connection_exit(pysqlite_Connection *self, PyObject *const *args, Py_ss #ifndef PYSQLITE_CONNECTION_LOAD_EXTENSION_METHODDEF #define PYSQLITE_CONNECTION_LOAD_EXTENSION_METHODDEF #endif /* !defined(PYSQLITE_CONNECTION_LOAD_EXTENSION_METHODDEF) */ -/*[clinic end generated code: output=c1bf09db3bcd0105 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=9620be094a4fea95 input=a9049054013a1b77]*/ diff --git a/Modules/_sqlite/connection.c b/Modules/_sqlite/connection.c index 47d97d17a91b98..b0963d4a80641b 100644 --- a/Modules/_sqlite/connection.c +++ b/Modules/_sqlite/connection.c @@ -21,7 +21,6 @@ * 3. This notice may not be removed or altered from any source distribution. */ -#include "cache.h" #include "module.h" #include "structmember.h" // PyMemberDef #include "connection.h" @@ -57,6 +56,39 @@ static const char * const begin_statements[] = { static int pysqlite_connection_set_isolation_level(pysqlite_Connection* self, PyObject* isolation_level, void *Py_UNUSED(ignored)); static void _pysqlite_drop_unused_cursor_references(pysqlite_Connection* self); +static PyObject *_lru_cache = NULL; + +int +load_functools_lru_cache(PyObject *Py_UNUSED(module)) +{ + PyObject *functools = PyImport_ImportModule("functools"); + if (functools == NULL) { + return -1; + } + _lru_cache = PyObject_GetAttrString(functools, "lru_cache"); + Py_DECREF(functools); + if (_lru_cache == NULL) { + return -1; + } + return 0; +} + +static PyObject * +new_statement_cache(pysqlite_Connection *self, int maxsize) +{ + PyObject *args[] = { PyLong_FromLong(maxsize), }; + PyObject *inner = PyObject_Vectorcall(_lru_cache, args, 1, NULL); + Py_XDECREF(args[0]); + if (inner == NULL) { + return NULL; + } + + args[0] = (PyObject *)self; // Borrowed ref. + PyObject *res = PyObject_Vectorcall(inner, args, 1, NULL); + Py_DECREF(inner); + return res; +} + static int pysqlite_connection_init(pysqlite_Connection *self, PyObject *args, PyObject *kwargs) @@ -134,7 +166,10 @@ pysqlite_connection_init(pysqlite_Connection *self, PyObject *args, } Py_DECREF(isolation_level); - self->statement_cache = (pysqlite_Cache*)PyObject_CallFunction((PyObject*)pysqlite_CacheType, "Oi", self, cached_statements); + self->statement_cache = new_statement_cache(self, cached_statements); + if (self->statement_cache == NULL) { + return -1; + } if (PyErr_Occurred()) { return -1; } @@ -149,14 +184,6 @@ pysqlite_connection_init(pysqlite_Connection *self, PyObject *args, return -1; } - /* By default, the Cache class INCREFs the factory in its initializer, and - * decrefs it in its deallocator method. Since this would create a circular - * reference here, we're breaking it by decrementing self, and telling the - * cache class to not decref the factory (self) in its deallocator. - */ - self->statement_cache->decref_factory = 0; - Py_DECREF(self); - self->detect_types = detect_types; self->timeout = timeout; (void)sqlite3_busy_timeout(self->db, (int)(timeout*1000)); @@ -261,9 +288,10 @@ connection_clear(pysqlite_Connection *self) static void connection_dealloc(pysqlite_Connection *self) { + PyObject_GC_UnTrack(self); PyTypeObject *tp = Py_TYPE(self); PyObject_GC_UnTrack(self); - tp->tp_clear((PyObject *)self); + (void)connection_clear(self); /* Clean up if user has not called .close() explicitly. */ if (self->db) { @@ -300,6 +328,21 @@ int pysqlite_connection_register_cursor(pysqlite_Connection* connection, PyObjec return 0; } + +/*[clinic input] +_sqlite3.Connection.statement_cache as pysqlite_connection_statement_cache + +Return the connection statement cache. +[clinic start generated code]*/ + +static PyObject * +pysqlite_connection_statement_cache_impl(pysqlite_Connection *self) +/*[clinic end generated code: output=a88b63d0c46650ba input=32465d5ec271ca8c]*/ +{ + return Py_NewRef(self->statement_cache); +} + + /*[clinic input] _sqlite3.Connection.cursor as pysqlite_connection_cursor @@ -1911,6 +1954,7 @@ static PyMethodDef connection_methods[] = { PYSQLITE_CONNECTION_SET_AUTHORIZER_METHODDEF PYSQLITE_CONNECTION_SET_PROGRESS_HANDLER_METHODDEF PYSQLITE_CONNECTION_SET_TRACE_CALLBACK_METHODDEF + PYSQLITE_CONNECTION_STATEMENT_CACHE_METHODDEF {NULL, NULL} }; diff --git a/Modules/_sqlite/connection.h b/Modules/_sqlite/connection.h index 82f6baf6eef3d7..7fa0bca9460237 100644 --- a/Modules/_sqlite/connection.h +++ b/Modules/_sqlite/connection.h @@ -28,7 +28,6 @@ #include "pythread.h" #include "structmember.h" -#include "cache.h" #include "module.h" #include "sqlite3.h" @@ -64,7 +63,7 @@ typedef struct /* thread identification of the thread the connection was created in */ unsigned long thread_ident; - pysqlite_Cache* statement_cache; + PyObject *statement_cache; /* Lists of weak references to statements and cursors used within this connection */ PyObject* statements; @@ -115,5 +114,6 @@ int pysqlite_check_thread(pysqlite_Connection* self); int pysqlite_check_connection(pysqlite_Connection* con); int pysqlite_connection_setup_types(PyObject *module); +int load_functools_lru_cache(PyObject *module); #endif diff --git a/Modules/_sqlite/cursor.c b/Modules/_sqlite/cursor.c index b3e1ce2c04784b..446dbdcecca8b6 100644 --- a/Modules/_sqlite/cursor.c +++ b/Modules/_sqlite/cursor.c @@ -414,6 +414,14 @@ static int check_cursor(pysqlite_Cursor* cur) return pysqlite_check_thread(cur->connection) && pysqlite_check_connection(cur->connection); } +static PyObject * +get_statement(pysqlite_Cursor *self, PyObject *operation) +{ + PyObject *args[] = { operation, }; + PyObject *cache = self->connection->statement_cache; + return PyObject_Vectorcall(cache, args, 1, NULL); +} + static PyObject * _pysqlite_query_execute(pysqlite_Cursor* self, int multiple, PyObject* operation, PyObject* second_argument) { @@ -422,7 +430,6 @@ _pysqlite_query_execute(pysqlite_Cursor* self, int multiple, PyObject* operation PyObject* parameters = NULL; int i; int rc; - PyObject* func_args; PyObject* result; int numcols; PyObject* column_name; @@ -484,22 +491,12 @@ _pysqlite_query_execute(pysqlite_Cursor* self, int multiple, PyObject* operation Py_SETREF(self->description, Py_None); self->rowcount = 0L; - func_args = PyTuple_New(1); - if (!func_args) { - goto error; - } - if (PyTuple_SetItem(func_args, 0, Py_NewRef(operation)) != 0) { - goto error; - } - if (self->statement) { (void)pysqlite_statement_reset(self->statement); } - Py_XSETREF(self->statement, - (pysqlite_Statement *)pysqlite_cache_get(self->connection->statement_cache, func_args)); - Py_DECREF(func_args); - + PyObject *stmt = get_statement(self, operation); + Py_XSETREF(self->statement, (pysqlite_Statement *)stmt); if (!self->statement) { goto error; } diff --git a/Modules/_sqlite/module.c b/Modules/_sqlite/module.c index a5e5525481f756..258a5b6485ae68 100644 --- a/Modules/_sqlite/module.c +++ b/Modules/_sqlite/module.c @@ -24,7 +24,6 @@ #include "connection.h" #include "statement.h" #include "cursor.h" -#include "cache.h" #include "prepare_protocol.h" #include "microprotocols.h" #include "row.h" @@ -373,7 +372,6 @@ PyMODINIT_FUNC PyInit__sqlite3(void) (pysqlite_row_setup_types(module) < 0) || (pysqlite_cursor_setup_types(module) < 0) || (pysqlite_connection_setup_types(module) < 0) || - (pysqlite_cache_setup_types(module) < 0) || (pysqlite_statement_setup_types(module) < 0) || (pysqlite_prepare_protocol_setup_types(module) < 0) ) { @@ -424,6 +422,10 @@ PyMODINIT_FUNC PyInit__sqlite3(void) goto error; } + if (load_functools_lru_cache(module) < 0) { + goto error; + } + return module; error: diff --git a/PCbuild/_sqlite3.vcxproj b/PCbuild/_sqlite3.vcxproj index 5eb8559d2925ec..e268c473f4c985 100644 --- a/PCbuild/_sqlite3.vcxproj +++ b/PCbuild/_sqlite3.vcxproj @@ -97,7 +97,6 @@ - @@ -108,7 +107,6 @@ - diff --git a/PCbuild/_sqlite3.vcxproj.filters b/PCbuild/_sqlite3.vcxproj.filters index 51830f6a4451a4..79fc17b53fb508 100644 --- a/PCbuild/_sqlite3.vcxproj.filters +++ b/PCbuild/_sqlite3.vcxproj.filters @@ -12,9 +12,6 @@ - - Header Files - Header Files @@ -41,9 +38,6 @@ - - Source Files - Source Files diff --git a/setup.py b/setup.py index 3857e6887a929f..ce71a966a81941 100644 --- a/setup.py +++ b/setup.py @@ -1578,7 +1578,7 @@ def detect_sqlite(self): sqlite_libdir = [os.path.abspath(os.path.dirname(sqlite_libfile))] if sqlite_incdir and sqlite_libdir: - sqlite_srcs = ['_sqlite/cache.c', + sqlite_srcs = [ '_sqlite/connection.c', '_sqlite/cursor.c', '_sqlite/microprotocols.c', From 0a8133af11870addf0246839e7c847430ac70585 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Wed, 13 Jan 2021 00:14:43 +0100 Subject: [PATCH 02/16] Update documentation --- Doc/library/sqlite3.rst | 10 ++++++++++ Doc/whatsnew/3.10.rst | 6 ++++++ 2 files changed, 16 insertions(+) diff --git a/Doc/library/sqlite3.rst b/Doc/library/sqlite3.rst index f9e4c8a269027b..eef4cb6afd20b1 100644 --- a/Doc/library/sqlite3.rst +++ b/Doc/library/sqlite3.rst @@ -515,6 +515,16 @@ Connection Objects .. XXX what's a db_row-based solution? + .. method:: statement_cache() + + Returns the LRU statement cache for this connection. The statement + cache is implemented using :meth:`functools.lru_cache`, so it's + possible to inspect its state by calling :func:`~functools.cache_info` + or :func:`~functools.cache_parameters` on the returned object. The + cache can be cleared by calling :func:`~functools.cache_clear` on + the returned object. + + .. versionadded:: 3.10 .. attribute:: text_factory diff --git a/Doc/whatsnew/3.10.rst b/Doc/whatsnew/3.10.rst index 233ee8b201f0e6..ce65500b4564ab 100644 --- a/Doc/whatsnew/3.10.rst +++ b/Doc/whatsnew/3.10.rst @@ -1237,6 +1237,12 @@ Add audit events for :func:`~sqlite3.connect/handle`, :meth:`~sqlite3.Connection.load_extension`. (Contributed by Erlend E. Aasland in :issue:`43762`.) + +:mod:`sqlite3` now utilizes :meth:`functools.lru_cache` to implement the +connection statement cache. The statement cache can be accessed via the new +method :meth:`sqlite3.Connection.statement_cache`. +(Contributed by Erlend E. Aasland in :issue:`42862`.) + sys --- From 7830f3b680cab406ed8c2f1c6082aa031a3d8753 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Tue, 12 Jan 2021 23:45:11 +0100 Subject: [PATCH 03/16] Add tests --- Lib/sqlite3/test/dbapi.py | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/Lib/sqlite3/test/dbapi.py b/Lib/sqlite3/test/dbapi.py index 39c9bf5b61143d..252d49a477800c 100644 --- a/Lib/sqlite3/test/dbapi.py +++ b/Lib/sqlite3/test/dbapi.py @@ -192,6 +192,33 @@ def test_open_uri(self): cx.execute('insert into test(id) values(1)') +class StatementCacheTests(unittest.TestCase): + def test_statement_cache(self): + query = 'select * from sqlite_master' + cx = sqlite.connect(':memory:') + cx.execute(query) # cache miss + cx.execute(query) # cache hit + cache = cx.statement_cache() + info = cache.cache_info() + self.assertEqual(info.hits, 1) + self.assertEqual(info.misses, 1) + self.assertEqual(info.maxsize, 100) + self.assertEqual(info.currsize, 1) + + def test_statement_cache_maxsize(self): + maxsize = 5 + testsize = maxsize + 1 + cx = sqlite.connect(':memory:', cached_statements=maxsize) + for i in range(testsize): + cx.execute(f'select {i}') + cache = cx.statement_cache() + info = cache.cache_info() + self.assertEqual(info.hits, 0) + self.assertEqual(info.misses, testsize) + self.assertEqual(info.maxsize, maxsize) + self.assertEqual(info.currsize, maxsize) + + class CursorTests(unittest.TestCase): def setUp(self): self.cx = sqlite.connect(":memory:") @@ -943,6 +970,7 @@ def suite(): ExtensionTests, ModuleTests, SqliteOnConflictTests, + StatementCacheTests, ThreadTests, ] return unittest.TestSuite( From 5c8e05caaa04aae5e16b942930097b1e9617e3b9 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Wed, 13 Jan 2021 00:02:54 +0100 Subject: [PATCH 04/16] Add NEWS --- .../next/Library/2021-01-13-00-02-44.bpo-42862.Z6ACLN.rst | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 Misc/NEWS.d/next/Library/2021-01-13-00-02-44.bpo-42862.Z6ACLN.rst diff --git a/Misc/NEWS.d/next/Library/2021-01-13-00-02-44.bpo-42862.Z6ACLN.rst b/Misc/NEWS.d/next/Library/2021-01-13-00-02-44.bpo-42862.Z6ACLN.rst new file mode 100644 index 00000000000000..cacd9e5ba573ce --- /dev/null +++ b/Misc/NEWS.d/next/Library/2021-01-13-00-02-44.bpo-42862.Z6ACLN.rst @@ -0,0 +1,4 @@ +:mod:`sqlite3` now utilizes :meth:`functools.lru_cache` to implement the +connection statement cache. The statement cache can be accessed via the new +method :meth:`sqlite3.Connection.statement_cache`. Patch by Erlend E. +Aasland. From d2078bced6179882fe911be091b467e506a0db56 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Thu, 21 Jan 2021 21:19:36 +0100 Subject: [PATCH 05/16] Fix tests on Windows --- Lib/sqlite3/test/dbapi.py | 5 +++++ Lib/sqlite3/test/hooks.py | 4 ++++ 2 files changed, 9 insertions(+) diff --git a/Lib/sqlite3/test/dbapi.py b/Lib/sqlite3/test/dbapi.py index 252d49a477800c..be071c28aa36a5 100644 --- a/Lib/sqlite3/test/dbapi.py +++ b/Lib/sqlite3/test/dbapi.py @@ -25,6 +25,7 @@ import sqlite3 as sqlite import sys +from test.support import gc_collect from test.support.os_helper import TESTFN, unlink @@ -180,6 +181,8 @@ def __fspath__(self): path = Path() with sqlite.connect(path) as cx: cx.execute('create table test(id integer)') + cx.close() + gc_collect() def test_open_uri(self): self.addCleanup(unlink, TESTFN) @@ -190,6 +193,8 @@ def test_open_uri(self): with sqlite.connect('file:' + TESTFN + '?mode=ro', uri=True) as cx: with self.assertRaises(sqlite.OperationalError): cx.execute('insert into test(id) values(1)') + cx.close() + gc_collect() class StatementCacheTests(unittest.TestCase): diff --git a/Lib/sqlite3/test/hooks.py b/Lib/sqlite3/test/hooks.py index 8c60bdcf5d70aa..b8ab3b4c24cfa7 100644 --- a/Lib/sqlite3/test/hooks.py +++ b/Lib/sqlite3/test/hooks.py @@ -23,6 +23,7 @@ import unittest import sqlite3 as sqlite +from test.support import gc_collect from test.support.os_helper import TESTFN, unlink @@ -260,6 +261,9 @@ def trace(statement): con2.execute("create table bar(x)") cur.execute(queries[1]) self.assertEqual(traced_statements, queries) + con1.close() + con2.close() + gc_collect() def suite(): From 6c6a4228c9d05a7237ed5d799ef33c0459910c97 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Sun, 2 May 2021 22:08:57 +0200 Subject: [PATCH 06/16] Tiny optimisation: increase the cache size from 100 to 128 --- Doc/library/sqlite3.rst | 2 +- Doc/whatsnew/3.10.rst | 4 ++-- Lib/sqlite3/test/dbapi.py | 4 ++-- .../next/Library/2021-01-13-00-02-44.bpo-42862.Z6ACLN.rst | 5 +++-- Modules/_sqlite/connection.c | 2 +- 5 files changed, 9 insertions(+), 8 deletions(-) diff --git a/Doc/library/sqlite3.rst b/Doc/library/sqlite3.rst index eef4cb6afd20b1..c0b98ad3f3f8de 100644 --- a/Doc/library/sqlite3.rst +++ b/Doc/library/sqlite3.rst @@ -213,7 +213,7 @@ Module functions and constants The :mod:`sqlite3` module internally uses a statement cache to avoid SQL parsing overhead. If you want to explicitly set the number of statements that are cached for the connection, you can set the *cached_statements* parameter. The currently - implemented default is to cache 100 statements. + implemented default is to cache 128 statements. If *uri* is true, *database* is interpreted as a URI. This allows you to specify options. For example, to open a database in read-only mode diff --git a/Doc/whatsnew/3.10.rst b/Doc/whatsnew/3.10.rst index ce65500b4564ab..94fe8db480fc7d 100644 --- a/Doc/whatsnew/3.10.rst +++ b/Doc/whatsnew/3.10.rst @@ -1237,10 +1237,10 @@ Add audit events for :func:`~sqlite3.connect/handle`, :meth:`~sqlite3.Connection.load_extension`. (Contributed by Erlend E. Aasland in :issue:`43762`.) - :mod:`sqlite3` now utilizes :meth:`functools.lru_cache` to implement the connection statement cache. The statement cache can be accessed via the new -method :meth:`sqlite3.Connection.statement_cache`. +method :meth:`sqlite3.Connection.statement_cache`. As a small optimisation, the +default statement cache size has been increased from 100 to 128. (Contributed by Erlend E. Aasland in :issue:`42862`.) sys diff --git a/Lib/sqlite3/test/dbapi.py b/Lib/sqlite3/test/dbapi.py index be071c28aa36a5..28ff785e345b88 100644 --- a/Lib/sqlite3/test/dbapi.py +++ b/Lib/sqlite3/test/dbapi.py @@ -207,11 +207,11 @@ def test_statement_cache(self): info = cache.cache_info() self.assertEqual(info.hits, 1) self.assertEqual(info.misses, 1) - self.assertEqual(info.maxsize, 100) + self.assertEqual(info.maxsize, 128) self.assertEqual(info.currsize, 1) def test_statement_cache_maxsize(self): - maxsize = 5 + maxsize = 8 testsize = maxsize + 1 cx = sqlite.connect(':memory:', cached_statements=maxsize) for i in range(testsize): diff --git a/Misc/NEWS.d/next/Library/2021-01-13-00-02-44.bpo-42862.Z6ACLN.rst b/Misc/NEWS.d/next/Library/2021-01-13-00-02-44.bpo-42862.Z6ACLN.rst index cacd9e5ba573ce..c5eed4ef99c107 100644 --- a/Misc/NEWS.d/next/Library/2021-01-13-00-02-44.bpo-42862.Z6ACLN.rst +++ b/Misc/NEWS.d/next/Library/2021-01-13-00-02-44.bpo-42862.Z6ACLN.rst @@ -1,4 +1,5 @@ :mod:`sqlite3` now utilizes :meth:`functools.lru_cache` to implement the connection statement cache. The statement cache can be accessed via the new -method :meth:`sqlite3.Connection.statement_cache`. Patch by Erlend E. -Aasland. +method :meth:`sqlite3.Connection.statement_cache`. As a small optimisation, the +default statement cache size has been increased from 100 to 128. +Patch by Erlend E. Aasland. diff --git a/Modules/_sqlite/connection.c b/Modules/_sqlite/connection.c index b0963d4a80641b..6c5f29a7b4fe98 100644 --- a/Modules/_sqlite/connection.c +++ b/Modules/_sqlite/connection.c @@ -105,7 +105,7 @@ pysqlite_connection_init(pysqlite_Connection *self, PyObject *args, PyObject* isolation_level = NULL; PyObject* factory = NULL; int check_same_thread = 1; - int cached_statements = 100; + int cached_statements = 128; int uri = 0; double timeout = 5.0; int rc; From 2795aca91e9bb25ff7f6e611d6ecac04df478435 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Fri, 14 May 2021 23:53:29 +0200 Subject: [PATCH 07/16] Move What's New to 3.11 --- Doc/whatsnew/3.10.rst | 6 ------ Doc/whatsnew/3.11.rst | 9 +++++++++ 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/Doc/whatsnew/3.10.rst b/Doc/whatsnew/3.10.rst index 94fe8db480fc7d..233ee8b201f0e6 100644 --- a/Doc/whatsnew/3.10.rst +++ b/Doc/whatsnew/3.10.rst @@ -1237,12 +1237,6 @@ Add audit events for :func:`~sqlite3.connect/handle`, :meth:`~sqlite3.Connection.load_extension`. (Contributed by Erlend E. Aasland in :issue:`43762`.) -:mod:`sqlite3` now utilizes :meth:`functools.lru_cache` to implement the -connection statement cache. The statement cache can be accessed via the new -method :meth:`sqlite3.Connection.statement_cache`. As a small optimisation, the -default statement cache size has been increased from 100 to 128. -(Contributed by Erlend E. Aasland in :issue:`42862`.) - sys --- diff --git a/Doc/whatsnew/3.11.rst b/Doc/whatsnew/3.11.rst index 9058b261560873..f8a6ac9c6ae7dc 100644 --- a/Doc/whatsnew/3.11.rst +++ b/Doc/whatsnew/3.11.rst @@ -86,6 +86,15 @@ New Modules Improved Modules ================ +sqlite3 +------- + +:mod:`sqlite3` now utilizes :meth:`functools.lru_cache` to implement the +connection statement cache. The statement cache can be accessed via the new +method :meth:`sqlite3.Connection.statement_cache`. As a small optimisation, the +default statement cache size has been increased from 100 to 128. +(Contributed by Erlend E. Aasland in :issue:`42862`.) + Optimizations ============= From 05ce0e4406eb3d3b09824a60017568ff86c4b59e Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Wed, 19 May 2021 10:09:36 +0200 Subject: [PATCH 08/16] Address review: don't expose cache API --- Doc/library/sqlite3.rst | 10 ------- Doc/whatsnew/3.11.rst | 9 ------ Lib/sqlite3/test/dbapi.py | 28 ------------------- .../2021-01-13-00-02-44.bpo-42862.Z6ACLN.rst | 5 ++-- Modules/_sqlite/clinic/connection.c.h | 20 +------------ Modules/_sqlite/connection.c | 15 ---------- 6 files changed, 3 insertions(+), 84 deletions(-) diff --git a/Doc/library/sqlite3.rst b/Doc/library/sqlite3.rst index c0b98ad3f3f8de..4010e1a4daff27 100644 --- a/Doc/library/sqlite3.rst +++ b/Doc/library/sqlite3.rst @@ -515,16 +515,6 @@ Connection Objects .. XXX what's a db_row-based solution? - .. method:: statement_cache() - - Returns the LRU statement cache for this connection. The statement - cache is implemented using :meth:`functools.lru_cache`, so it's - possible to inspect its state by calling :func:`~functools.cache_info` - or :func:`~functools.cache_parameters` on the returned object. The - cache can be cleared by calling :func:`~functools.cache_clear` on - the returned object. - - .. versionadded:: 3.10 .. attribute:: text_factory diff --git a/Doc/whatsnew/3.11.rst b/Doc/whatsnew/3.11.rst index f8a6ac9c6ae7dc..9058b261560873 100644 --- a/Doc/whatsnew/3.11.rst +++ b/Doc/whatsnew/3.11.rst @@ -86,15 +86,6 @@ New Modules Improved Modules ================ -sqlite3 -------- - -:mod:`sqlite3` now utilizes :meth:`functools.lru_cache` to implement the -connection statement cache. The statement cache can be accessed via the new -method :meth:`sqlite3.Connection.statement_cache`. As a small optimisation, the -default statement cache size has been increased from 100 to 128. -(Contributed by Erlend E. Aasland in :issue:`42862`.) - Optimizations ============= diff --git a/Lib/sqlite3/test/dbapi.py b/Lib/sqlite3/test/dbapi.py index 28ff785e345b88..a98abaf38c9453 100644 --- a/Lib/sqlite3/test/dbapi.py +++ b/Lib/sqlite3/test/dbapi.py @@ -197,33 +197,6 @@ def test_open_uri(self): gc_collect() -class StatementCacheTests(unittest.TestCase): - def test_statement_cache(self): - query = 'select * from sqlite_master' - cx = sqlite.connect(':memory:') - cx.execute(query) # cache miss - cx.execute(query) # cache hit - cache = cx.statement_cache() - info = cache.cache_info() - self.assertEqual(info.hits, 1) - self.assertEqual(info.misses, 1) - self.assertEqual(info.maxsize, 128) - self.assertEqual(info.currsize, 1) - - def test_statement_cache_maxsize(self): - maxsize = 8 - testsize = maxsize + 1 - cx = sqlite.connect(':memory:', cached_statements=maxsize) - for i in range(testsize): - cx.execute(f'select {i}') - cache = cx.statement_cache() - info = cache.cache_info() - self.assertEqual(info.hits, 0) - self.assertEqual(info.misses, testsize) - self.assertEqual(info.maxsize, maxsize) - self.assertEqual(info.currsize, maxsize) - - class CursorTests(unittest.TestCase): def setUp(self): self.cx = sqlite.connect(":memory:") @@ -975,7 +948,6 @@ def suite(): ExtensionTests, ModuleTests, SqliteOnConflictTests, - StatementCacheTests, ThreadTests, ] return unittest.TestSuite( diff --git a/Misc/NEWS.d/next/Library/2021-01-13-00-02-44.bpo-42862.Z6ACLN.rst b/Misc/NEWS.d/next/Library/2021-01-13-00-02-44.bpo-42862.Z6ACLN.rst index c5eed4ef99c107..beda25fd335b08 100644 --- a/Misc/NEWS.d/next/Library/2021-01-13-00-02-44.bpo-42862.Z6ACLN.rst +++ b/Misc/NEWS.d/next/Library/2021-01-13-00-02-44.bpo-42862.Z6ACLN.rst @@ -1,5 +1,4 @@ :mod:`sqlite3` now utilizes :meth:`functools.lru_cache` to implement the -connection statement cache. The statement cache can be accessed via the new -method :meth:`sqlite3.Connection.statement_cache`. As a small optimisation, the -default statement cache size has been increased from 100 to 128. +connection statement cache. As a small optimisation, the default +statement cache size has been increased from 100 to 128. Patch by Erlend E. Aasland. diff --git a/Modules/_sqlite/clinic/connection.c.h b/Modules/_sqlite/clinic/connection.c.h index c42285c44a85c1..f231ecc2ae78be 100644 --- a/Modules/_sqlite/clinic/connection.c.h +++ b/Modules/_sqlite/clinic/connection.c.h @@ -2,24 +2,6 @@ preserve [clinic start generated code]*/ -PyDoc_STRVAR(pysqlite_connection_statement_cache__doc__, -"statement_cache($self, /)\n" -"--\n" -"\n" -"Return the connection statement cache."); - -#define PYSQLITE_CONNECTION_STATEMENT_CACHE_METHODDEF \ - {"statement_cache", (PyCFunction)pysqlite_connection_statement_cache, METH_NOARGS, pysqlite_connection_statement_cache__doc__}, - -static PyObject * -pysqlite_connection_statement_cache_impl(pysqlite_Connection *self); - -static PyObject * -pysqlite_connection_statement_cache(pysqlite_Connection *self, PyObject *Py_UNUSED(ignored)) -{ - return pysqlite_connection_statement_cache_impl(self); -} - PyDoc_STRVAR(pysqlite_connection_cursor__doc__, "cursor($self, /, factory=)\n" "--\n" @@ -728,4 +710,4 @@ pysqlite_connection_exit(pysqlite_Connection *self, PyObject *const *args, Py_ss #ifndef PYSQLITE_CONNECTION_LOAD_EXTENSION_METHODDEF #define PYSQLITE_CONNECTION_LOAD_EXTENSION_METHODDEF #endif /* !defined(PYSQLITE_CONNECTION_LOAD_EXTENSION_METHODDEF) */ -/*[clinic end generated code: output=9620be094a4fea95 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=c1bf09db3bcd0105 input=a9049054013a1b77]*/ diff --git a/Modules/_sqlite/connection.c b/Modules/_sqlite/connection.c index 6c5f29a7b4fe98..c56d84aa1b072f 100644 --- a/Modules/_sqlite/connection.c +++ b/Modules/_sqlite/connection.c @@ -329,20 +329,6 @@ int pysqlite_connection_register_cursor(pysqlite_Connection* connection, PyObjec } -/*[clinic input] -_sqlite3.Connection.statement_cache as pysqlite_connection_statement_cache - -Return the connection statement cache. -[clinic start generated code]*/ - -static PyObject * -pysqlite_connection_statement_cache_impl(pysqlite_Connection *self) -/*[clinic end generated code: output=a88b63d0c46650ba input=32465d5ec271ca8c]*/ -{ - return Py_NewRef(self->statement_cache); -} - - /*[clinic input] _sqlite3.Connection.cursor as pysqlite_connection_cursor @@ -1954,7 +1940,6 @@ static PyMethodDef connection_methods[] = { PYSQLITE_CONNECTION_SET_AUTHORIZER_METHODDEF PYSQLITE_CONNECTION_SET_PROGRESS_HANDLER_METHODDEF PYSQLITE_CONNECTION_SET_TRACE_CALLBACK_METHODDEF - PYSQLITE_CONNECTION_STATEMENT_CACHE_METHODDEF {NULL, NULL} }; From 839f2641f6c9c9305ca240b26f4a668bb0dea7f8 Mon Sep 17 00:00:00 2001 From: Erlend Egeberg Aasland Date: Wed, 19 May 2021 21:12:11 +0200 Subject: [PATCH 09/16] Remove empty line Co-authored-by: Pablo Galindo --- Modules/_sqlite/connection.c | 1 - 1 file changed, 1 deletion(-) diff --git a/Modules/_sqlite/connection.c b/Modules/_sqlite/connection.c index c56d84aa1b072f..7a574185e01802 100644 --- a/Modules/_sqlite/connection.c +++ b/Modules/_sqlite/connection.c @@ -328,7 +328,6 @@ int pysqlite_connection_register_cursor(pysqlite_Connection* connection, PyObjec return 0; } - /*[clinic input] _sqlite3.Connection.cursor as pysqlite_connection_cursor From 8d1c13804ba7e07798efe436f6a7a6666567d986 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Thu, 20 May 2021 00:51:58 +0200 Subject: [PATCH 10/16] Address review: document Windows test workaround --- Lib/sqlite3/test/dbapi.py | 6 ++++++ Lib/sqlite3/test/hooks.py | 3 +++ 2 files changed, 9 insertions(+) diff --git a/Lib/sqlite3/test/dbapi.py b/Lib/sqlite3/test/dbapi.py index a98abaf38c9453..a4a4b650fc444f 100644 --- a/Lib/sqlite3/test/dbapi.py +++ b/Lib/sqlite3/test/dbapi.py @@ -181,6 +181,9 @@ def __fspath__(self): path = Path() with sqlite.connect(path) as cx: cx.execute('create table test(id integer)') + + # bpo-42862: addCleanup fails to unlink TESTFN on Windows unless we + # explicitly close the connection and run the GC. cx.close() gc_collect() @@ -193,6 +196,9 @@ def test_open_uri(self): with sqlite.connect('file:' + TESTFN + '?mode=ro', uri=True) as cx: with self.assertRaises(sqlite.OperationalError): cx.execute('insert into test(id) values(1)') + + # bpo-42862: addCleanup fails to unlink TESTFN on Windows unless we + # explicitly close the connection and run the GC. cx.close() gc_collect() diff --git a/Lib/sqlite3/test/hooks.py b/Lib/sqlite3/test/hooks.py index b8ab3b4c24cfa7..f81f0306be132a 100644 --- a/Lib/sqlite3/test/hooks.py +++ b/Lib/sqlite3/test/hooks.py @@ -261,6 +261,9 @@ def trace(statement): con2.execute("create table bar(x)") cur.execute(queries[1]) self.assertEqual(traced_statements, queries) + + # bpo-42862: addCleanup fails to unlink TESTFN on Windows unless we + # explicitly close the connections and run the GC. con1.close() con2.close() gc_collect() From 0067cefd9b75c96c5be80b997136e84e652d7d18 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Thu, 20 May 2021 00:56:31 +0200 Subject: [PATCH 11/16] Address review: check PyLong_FromLong ret value --- Modules/_sqlite/connection.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Modules/_sqlite/connection.c b/Modules/_sqlite/connection.c index 7a574185e01802..f2cc6945ca0807 100644 --- a/Modules/_sqlite/connection.c +++ b/Modules/_sqlite/connection.c @@ -77,8 +77,11 @@ static PyObject * new_statement_cache(pysqlite_Connection *self, int maxsize) { PyObject *args[] = { PyLong_FromLong(maxsize), }; + if (args[0] == NULL) { + return NULL; + } PyObject *inner = PyObject_Vectorcall(_lru_cache, args, 1, NULL); - Py_XDECREF(args[0]); + Py_DECREF(args[0]); if (inner == NULL) { return NULL; } From a95a386adad9bce2e667ac23bac1beb69b2eae28 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Thu, 20 May 2021 01:05:44 +0200 Subject: [PATCH 12/16] Address review: add global state and add LRU cache func to it --- Modules/_sqlite/connection.c | 20 ++------------------ Modules/_sqlite/connection.h | 1 - Modules/_sqlite/module.c | 25 +++++++++++++++++++++++++ Modules/_sqlite/module.h | 6 ++++++ 4 files changed, 33 insertions(+), 19 deletions(-) diff --git a/Modules/_sqlite/connection.c b/Modules/_sqlite/connection.c index f2cc6945ca0807..6d8546e4b1a76b 100644 --- a/Modules/_sqlite/connection.c +++ b/Modules/_sqlite/connection.c @@ -56,23 +56,6 @@ static const char * const begin_statements[] = { static int pysqlite_connection_set_isolation_level(pysqlite_Connection* self, PyObject* isolation_level, void *Py_UNUSED(ignored)); static void _pysqlite_drop_unused_cursor_references(pysqlite_Connection* self); -static PyObject *_lru_cache = NULL; - -int -load_functools_lru_cache(PyObject *Py_UNUSED(module)) -{ - PyObject *functools = PyImport_ImportModule("functools"); - if (functools == NULL) { - return -1; - } - _lru_cache = PyObject_GetAttrString(functools, "lru_cache"); - Py_DECREF(functools); - if (_lru_cache == NULL) { - return -1; - } - return 0; -} - static PyObject * new_statement_cache(pysqlite_Connection *self, int maxsize) { @@ -80,7 +63,8 @@ new_statement_cache(pysqlite_Connection *self, int maxsize) if (args[0] == NULL) { return NULL; } - PyObject *inner = PyObject_Vectorcall(_lru_cache, args, 1, NULL); + pysqlite_state *state = pysqlite_get_state(NULL); + PyObject *inner = PyObject_Vectorcall(state->lru_cache, args, 1, NULL); Py_DECREF(args[0]); if (inner == NULL) { return NULL; diff --git a/Modules/_sqlite/connection.h b/Modules/_sqlite/connection.h index 7fa0bca9460237..3af254e52f1d8a 100644 --- a/Modules/_sqlite/connection.h +++ b/Modules/_sqlite/connection.h @@ -114,6 +114,5 @@ int pysqlite_check_thread(pysqlite_Connection* self); int pysqlite_check_connection(pysqlite_Connection* con); int pysqlite_connection_setup_types(PyObject *module); -int load_functools_lru_cache(PyObject *module); #endif diff --git a/Modules/_sqlite/module.c b/Modules/_sqlite/module.c index 258a5b6485ae68..c60007e2059536 100644 --- a/Modules/_sqlite/module.c +++ b/Modules/_sqlite/module.c @@ -55,6 +55,14 @@ PyObject* _pysqlite_converters = NULL; int _pysqlite_enable_callback_tracebacks = 0; int pysqlite_BaseTypeAdapted = 0; +pysqlite_state pysqlite_global_state; + +pysqlite_state * +pysqlite_get_state(PyObject *Py_UNUSED(module)) +{ + return &pysqlite_global_state; +} + static PyObject* module_connect(PyObject* self, PyObject* args, PyObject* kwargs) { @@ -260,6 +268,23 @@ static int converters_init(PyObject* module) return res; } +static int +load_functools_lru_cache(PyObject *module) +{ + PyObject *functools = PyImport_ImportModule("functools"); + if (functools == NULL) { + return -1; + } + + pysqlite_state *state = pysqlite_get_state(module); + state->lru_cache = PyObject_GetAttrString(functools, "lru_cache"); + Py_DECREF(functools); + if (state->lru_cache == NULL) { + return -1; + } + return 0; +} + static PyMethodDef module_methods[] = { {"connect", (PyCFunction)(void(*)(void))module_connect, METH_VARARGS | METH_KEYWORDS, module_connect_doc}, diff --git a/Modules/_sqlite/module.h b/Modules/_sqlite/module.h index 9aede92ea33c9e..a40e86e9c4da72 100644 --- a/Modules/_sqlite/module.h +++ b/Modules/_sqlite/module.h @@ -29,6 +29,12 @@ #define PYSQLITE_VERSION "2.6.0" #define MODULE_NAME "sqlite3" +typedef struct { + PyObject *lru_cache; +} pysqlite_state; + +extern pysqlite_state *pysqlite_get_state(PyObject *module); + extern PyObject* pysqlite_Error; extern PyObject* pysqlite_Warning; extern PyObject* pysqlite_InterfaceError; From 9882ebd2e68c0fc63da860281e12f7f51bcbf339 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Wed, 26 May 2021 00:20:35 +0200 Subject: [PATCH 13/16] Fix rebase --- Modules/_sqlite/connection.c | 1 - 1 file changed, 1 deletion(-) diff --git a/Modules/_sqlite/connection.c b/Modules/_sqlite/connection.c index 6d8546e4b1a76b..890b907b9579f6 100644 --- a/Modules/_sqlite/connection.c +++ b/Modules/_sqlite/connection.c @@ -275,7 +275,6 @@ connection_clear(pysqlite_Connection *self) static void connection_dealloc(pysqlite_Connection *self) { - PyObject_GC_UnTrack(self); PyTypeObject *tp = Py_TYPE(self); PyObject_GC_UnTrack(self); (void)connection_clear(self); From 5a4aacce13b4ca8a6821166bac5a2006972eaa38 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Wed, 2 Jun 2021 14:18:35 +0200 Subject: [PATCH 14/16] Revert test workaround --- Lib/sqlite3/test/dbapi.py | 11 ----------- Lib/sqlite3/test/hooks.py | 7 ------- 2 files changed, 18 deletions(-) diff --git a/Lib/sqlite3/test/dbapi.py b/Lib/sqlite3/test/dbapi.py index a4a4b650fc444f..39c9bf5b61143d 100644 --- a/Lib/sqlite3/test/dbapi.py +++ b/Lib/sqlite3/test/dbapi.py @@ -25,7 +25,6 @@ import sqlite3 as sqlite import sys -from test.support import gc_collect from test.support.os_helper import TESTFN, unlink @@ -182,11 +181,6 @@ def __fspath__(self): with sqlite.connect(path) as cx: cx.execute('create table test(id integer)') - # bpo-42862: addCleanup fails to unlink TESTFN on Windows unless we - # explicitly close the connection and run the GC. - cx.close() - gc_collect() - def test_open_uri(self): self.addCleanup(unlink, TESTFN) with sqlite.connect(TESTFN) as cx: @@ -197,11 +191,6 @@ def test_open_uri(self): with self.assertRaises(sqlite.OperationalError): cx.execute('insert into test(id) values(1)') - # bpo-42862: addCleanup fails to unlink TESTFN on Windows unless we - # explicitly close the connection and run the GC. - cx.close() - gc_collect() - class CursorTests(unittest.TestCase): def setUp(self): diff --git a/Lib/sqlite3/test/hooks.py b/Lib/sqlite3/test/hooks.py index f81f0306be132a..8c60bdcf5d70aa 100644 --- a/Lib/sqlite3/test/hooks.py +++ b/Lib/sqlite3/test/hooks.py @@ -23,7 +23,6 @@ import unittest import sqlite3 as sqlite -from test.support import gc_collect from test.support.os_helper import TESTFN, unlink @@ -262,12 +261,6 @@ def trace(statement): cur.execute(queries[1]) self.assertEqual(traced_statements, queries) - # bpo-42862: addCleanup fails to unlink TESTFN on Windows unless we - # explicitly close the connections and run the GC. - con1.close() - con2.close() - gc_collect() - def suite(): tests = [ From 401b0575b4dc8cd5b3c0ba144cbcf2269b8a284e Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Thu, 3 Jun 2021 21:33:47 +0200 Subject: [PATCH 15/16] Improve naming: get_statement => get_statement_from_cache --- Modules/_sqlite/cursor.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Modules/_sqlite/cursor.c b/Modules/_sqlite/cursor.c index ace579eaa20c66..8073f3bbb78c78 100644 --- a/Modules/_sqlite/cursor.c +++ b/Modules/_sqlite/cursor.c @@ -416,7 +416,7 @@ static int check_cursor(pysqlite_Cursor* cur) } static PyObject * -get_statement(pysqlite_Cursor *self, PyObject *operation) +get_statement_from_cache(pysqlite_Cursor *self, PyObject *operation) { PyObject *args[] = { operation, }; PyObject *cache = self->connection->statement_cache; @@ -496,7 +496,7 @@ _pysqlite_query_execute(pysqlite_Cursor* self, int multiple, PyObject* operation (void)pysqlite_statement_reset(self->statement); } - PyObject *stmt = get_statement(self, operation); + PyObject *stmt = get_statement_from_cache(self, operation); Py_XSETREF(self->statement, (pysqlite_Statement *)stmt); if (!self->statement) { goto error; From dfb47a8b33929eb0bf946149727481f5ae84a68e Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Thu, 3 Jun 2021 21:35:22 +0200 Subject: [PATCH 16/16] Remove spurious change in connection_dealloc --- Modules/_sqlite/connection.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/_sqlite/connection.c b/Modules/_sqlite/connection.c index 91ccb1698f1ea6..c43f74ceace303 100644 --- a/Modules/_sqlite/connection.c +++ b/Modules/_sqlite/connection.c @@ -287,7 +287,7 @@ connection_dealloc(pysqlite_Connection *self) { PyTypeObject *tp = Py_TYPE(self); PyObject_GC_UnTrack(self); - (void)connection_clear(self); + tp->tp_clear((PyObject *)self); /* Clean up if user has not called .close() explicitly. */ connection_close(self);