classification
Title: [sqlite3] Redundant type checks in pysqlite_statement_bind_parameter()
Type: enhancement Stage:
Components: Library (Lib) Versions: Python 3.10
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: berker.peksag, erlendaasland, serhiy.storchaka
Priority: normal Keywords:

Created on 2021-02-19 10:45 by erlendaasland, last changed 2021-02-21 15:58 by erlendaasland.

Messages (5)
msg387302 - (view) Author: Erlend Egeberg Aasland (erlendaasland) * Date: 2021-02-19 10:45
The type checks at the start of pysqlite_statement_bind_parameter() are redundant:
We first check for exact types (*_CheckExact()), and then we check again for exact or subtyped versions (*_Check()).

(Adding to the redundantness: the result of this if..else "switch statement" is then reused as the expression in the subsequent switch statement.)

Suggesting to remove the redundant checks and merge the two switch statements.
msg387334 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2021-02-19 17:33
It is a kind of optimization. We first perform few fast checks for exact types, and then slower subclass checks.

I do not know whether this optimization is worth, but it was written so, and there is nothing wrong in it.
msg387346 - (view) Author: Erlend Egeberg Aasland (erlendaasland) * Date: 2021-02-19 19:59
> It is a kind of optimization.

PyLong_Check is very fast (only one comparison, AFAICS), so there is no gain in first doing PyLong_CheckExact and then PyLong_Check. Ditto for unicode.

PyFloat_Check is the most expensive check, but it comes before both PyUnicode_Check and PyObject_CheckBuffer (both a lot faster), so that's AFAICS the opposite of an optimisation. Correct me if I'm wrong.

If we want to optimise it we should do PyLong_Check, PyUnicode_Check, PyObject_CheckBuffer, and then PyFloat_Check, no?

> there is nothing wrong in it.

True. I'll argue that my suggestion will improve readability and maintainability, which should be worth considering.
msg387448 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2021-02-21 10:03
PyLong_Check is two memory reads, bits operation, and comparison with 0. And one non-inlined function call when limited API is used. PyLong_CheckExact is only one memory read and comparison.

I am not sure that the difference is significant enough to justify the optimization. I would require evidences that this optimization is significant if it would be proposed now. But the code is already written in such form, and I am not sure that it is worth to spend our time to change it.
msg387464 - (view) Author: Erlend Egeberg Aasland (erlendaasland) * Date: 2021-02-21 15:58
> I am not sure that the difference is significant enough to justify the optimization.

I would guess it's negligible. I did some quick timeit tests, but the results were pretty much equal, so I guess I'd have to use a more precise benchmark method.

> But the code is already written in such form, and I am not sure that it is worth to spend our time to change it.

Not even a 30% reduction in code size (for this function)?
History
Date User Action Args
2021-02-21 15:58:19erlendaaslandsetmessages: + msg387464
2021-02-21 10:03:35serhiy.storchakasetmessages: + msg387448
2021-02-19 19:59:37erlendaaslandsetmessages: + msg387346
2021-02-19 17:33:46serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg387334
2021-02-19 10:45:36erlendaaslandcreate