Skip to content

Conversation

@eendebakpt
Copy link
Contributor

@eendebakpt eendebakpt commented Apr 7, 2022

Merge BINARY_SUBSCR_LIST_INT and BINARY_SUBSCR_TUPLE_INT. This reduces the number of opcodes by 1 and eliminates some duplicated code.

Details The reason for this PR is not performance, but here microbenchmarks to check there is no slowdown:
list: Mean +- std dev: [base] 14.7 ns +- 0.2 ns -> [patch] 14.7 ns +- 0.2 ns: 1.00x faster
tuple: Mean +- std dev: [base] 14.6 ns +- 0.2 ns -> [patch] 14.8 ns +- 0.4 ns: 1.02x slower
mixed: Mean +- std dev: [base] 25.7 us +- 0.9 us -> [patch] 25.0 us +- 0.3 us: 1.03x faster

Geometric mean: 1.01x faster

Code

import pyperf
runner = pyperf.Runner()

setup='l=[1,2,None,4,5,6,7,8,9,10]; t=(1,None,3,4)'

runner.timeit(name=f"list", stmt=f"x=l[3]",setup=setup)
runner.timeit(name=f"tuple", stmt=f"x=t[2]",setup=setup)
code="""
for ii in range(500):
	idx=ii%4
	a=l[idx]
	b=t[idx]	
"""
runner.timeit(name=f"mixed", stmt=code,setup=setup)

See #91407

https://bugs.python.org/issue47251

@eendebakpt eendebakpt marked this pull request as ready for review April 7, 2022 20:00
Copy link
Member

@corona10 corona10 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMHO, you may need to run all pyperformance benchmarks with a reliable machine.

@markshannon
Copy link
Member

Please read https://github.com/python/cpython/blob/main/Python/adaptive.md#implementation-of-specialized-instructions
This adds branches into specialized instructions, which we are trying to avoid.

We could reduce code duplication by merging the code paths after the type check and array pointer extraction.
Something like:

PyObject **array

tuple:
     DEOPT_IF(Py_TYPE(obj) != &PyTuple_Type);
     array = &PyTuple_GET_ITEM(0);
     goto shared_code;
tuple:
     DEOPT_IF(Py_TYPE(obj) != &PyList_Type);
     array = &PyList_GET_ITEM(0);
     goto shared_code;

@eendebakpt eendebakpt marked this pull request as draft April 9, 2022 11:43
@eendebakpt eendebakpt changed the title bpo-47251: Merge BINARY_SUBSCR_LIST_INT with BINARY_SUBSCR_LIST_TUPLE gh-91407: Merge BINARY_SUBSCR_LIST_INT with BINARY_SUBSCR_TUPLE_INT Apr 15, 2022
@eendebakpt eendebakpt closed this Apr 25, 2022
@eendebakpt eendebakpt deleted the combine_list_tuple_opcode branch June 26, 2025 13:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants