[issue44958] [sqlite3] only reset statements when needed

Change by Erlend E. Aasland : -- pull_requests: +28589 pull_request: https://github.com/python/cpython/pull/30379 ___ Python tracker ___

[issue44958] [sqlite3] only reset statements when needed

Pablo Galindo Salgado added the comment: New changeset 7b88f63e1dd4006b1a08b9c9f087dd13449ecc76 by Erlend Egeberg Aasland in branch 'main': bpo-44958: Revert GH-27844 (GH-28574) https://github.com/python/cpython/commit/7b88f63e1dd4006b1a08b9c9f087dd13449ecc76 --

[issue44958] [sqlite3] only reset statements when needed

Change by Erlend E. Aasland : -- pull_requests: +26956 pull_request: https://github.com/python/cpython/pull/28574 ___ Python tracker ___

[issue44958] [sqlite3] only reset statements when needed

Erlend E. Aasland added the comment: I'll revert PR 27844 for now (except the tests). Since SQLite works better when we keep the number of non-reset statements to a minimum, we need to ensure that we reset statements when we're done with them (sqlite3_step() returns SQLITE_DONE or an

[issue44958] [sqlite3] only reset statements when needed

Erlend E. Aasland added the comment: Explicitly resetting statements when we're done with them removes the performance regression; SQLite works more efficient when we keep the number of non-reset statements low. -- ___ Python tracker

[issue44958] [sqlite3] only reset statements when needed

Erlend E. Aasland added the comment: > I'm unable to reproduce this regression on my machine (macOS, debug build, no > optimisations) [...] Correction: I _am_ able to reproduce this. -- ___ Python tracker

[issue44958] [sqlite3] only reset statements when needed

Erlend E. Aasland added the comment: I'm unable to reproduce this regression on my machine (macOS, debug build, no optimisations). Are you able to reproduce, Ken? -- ___ Python tracker

[issue44958] [sqlite3] only reset statements when needed

Erlend E. Aasland added the comment: Ouch, that's quite a regression! Thanks for the heads up! I'll have a look at it right away. -- ___ Python tracker ___

[issue44958] [sqlite3] only reset statements when needed

Ken Jin added the comment: Erlend, I suspect that 050d1035957379d70e8601e6f5636637716a264b may have introduced a perf regression in pyperformance's sqlite_synth benchmark: https://speed.python.org/timeline/?exe=12==sqlite_synth=1=50=off=on=on The benchmark code is here

[issue44958] [sqlite3] only reset statements when needed

Erlend E. Aasland added the comment: > Now that pysqlite_statement_reset() is only used in cursor.c, I suggest to > move it to cursor.c and make it a static function. I'll wait until PR 25984 is merged before opening a PR for relocating pysqlite_statement_reset(). --

[issue44958] [sqlite3] only reset statements when needed

Erlend E. Aasland added the comment: Now that pysqlite_statement_reset() is only used in cursor.c, I suggest to move it to cursor.c and make it a static function. -- ___ Python tracker

[issue44958] [sqlite3] only reset statements when needed

miss-islington added the comment: New changeset 3e3ff09058a71b92c32d1d7dc32470c0cf49300c by Erlend Egeberg Aasland in branch 'main': bpo-44958: Fix ref. leak introduced in GH-27844 (GH-28490) https://github.com/python/cpython/commit/3e3ff09058a71b92c32d1d7dc32470c0cf49300c -- nosy:

[issue44958] [sqlite3] only reset statements when needed

Change by Erlend E. Aasland : -- pull_requests: +26886 pull_request: https://github.com/python/cpython/pull/28490 ___ Python tracker ___

[issue44958] [sqlite3] only reset statements when needed

Pablo Galindo Salgado added the comment: New changeset 050d1035957379d70e8601e6f5636637716a264b by Erlend Egeberg Aasland in branch 'main': bpo-44958: Only reset `sqlite3` statements when needed (GH-27844) https://github.com/python/cpython/commit/050d1035957379d70e8601e6f5636637716a264b

[issue44958] [sqlite3] only reset statements when needed

Erlend E. Aasland added the comment: In msg399939, item 2 lacks one more "reset path": > 2. at cursor exit, if there's an active statement Rewording this to: 2. when a statement is removed from a cursor; that is either at cursor dealloc, or when the current statement is replaced.

[issue44958] [sqlite3] only reset statements when needed

Erlend E. Aasland added the comment: I did a quick count of sqlite3_reset()s in the sqlite3 test suite: - main: 2976 calls - PR 27844: 1730 calls Since we never call sqlite3_reset() with a NULL pointer, all sqlite3_reset() calls we execute hold the SQLite db mutex; reducing the number of

[issue44958] [sqlite3] only reset statements when needed

Change by Erlend E. Aasland : -- Removed message: https://bugs.python.org/msg399931 ___ Python tracker ___ ___ Python-bugs-list

[issue44958] [sqlite3] only reset statements when needed

Erlend E. Aasland added the comment: Ref. Serhiy's msg387858 in bpo-43350: "Maybe the code could be rewritten in more explicit way and call pysqlite_statement_reset() only when it is necessary [...]" Currently, we try to reset statements in all "statement exit" routes. IMO, it would be

[issue44958] [sqlite3] only reset statements when needed

Change by Erlend E. Aasland : -- keywords: +patch pull_requests: +26307 stage: -> patch review pull_request: https://github.com/python/cpython/pull/27844 ___ Python tracker

[issue44958] [sqlite3] only reset statements when needed

New submission from Erlend E. Aasland : Ref. Serhiy's msg387858 in bpo-43350: "Maybe the code could be rewritten in more explicit way and call pysqlite_statement_reset() only when it is necessary [...]" Currently, we try to reset statements in all "statement exit" routes. IMO, it would be