Inline tuple ops. - #21828
Conversation
3a8d611 to
8738c7e
Compare
8738c7e to
466060c
Compare
JukkaL
left a comment
JukkaL
left a comment
There was a problem hiding this comment.
Thanks for the PR! Left a comment about splitting CPySequenceTuple_GetItem into separate fast and slow paths. Also, can you measure the impact on a small micro-benchmark, and post the results? Sometimes C compilers do something unexpected and an optimization doesn't produce the results you'd expect.
Also, can you add a [mypyc] prefix to the PR title -- this can make it easier to route/prioritize reviews, and to construct release notes, where mypyc changes go into a separate section.
| return (0 <= n && n < size) || (-size <= n && n < 0); | ||
| } | ||
|
|
||
| static inline PyObject *CPySequenceTuple_GetItem(PyObject *tuple, CPyTagged index) |
There was a problem hiding this comment.
This looks kind of a large function to inline -- it can slow down compilation and/or increase size of generated code, which can cause L1 cache misses. Also I've seen that compilers sometimes don't want to inline a function if it's big enough, even if declared as inline.
We could instead only inline the fast path -- non-negative, short integer index, no error. The remaining cases would be handled by another, non-inlined function. You can find an example of this in CPyList_GetItem and CPyList_GetItem_.
2 participants
Mypyc tuple ops were not yet inlined. I've left
CPySequenceTuple_GetSliceun-inlined since it's still kind of heavy.