Issue43267
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) * ![]() |
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) * ![]() |
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:19 | erlendaasland | set | messages: + msg387464 |
2021-02-21 10:03:35 | serhiy.storchaka | set | messages: + msg387448 |
2021-02-19 19:59:37 | erlendaasland | set | messages: + msg387346 |
2021-02-19 17:33:46 | serhiy.storchaka | set | nosy:
+ serhiy.storchaka messages: + msg387334 |
2021-02-19 10:45:36 | erlendaasland | create |