Skip to content

gh-129068: allow concurrent iteration over range iterators #131921

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

tom-pytel
Copy link
Contributor

@tom-pytel tom-pytel commented Mar 30, 2025

Before anything, I'm not very familiar with the jit system so what I did to the ITER_RANGE micro-ops is maybe not in line with the master plan (In the sense of removing a uop, jit does run). But this PR does pass the test from: 1533d1d (which has been added here), and succeeds on the script from: #131199. TSAN also doesn't complain on them.

The fast range iterator has been made free-thread safe by reducing the state to a single variable, calculating the length each time it is needed is not horrible. Removed _ITER_JUMP_RANGE because the length guard needs to be atomic in _ITER_NEXT_RANGE, it can always be put back as a noop or extra check.

Removed the _PyObject_IsUniquelyReferenced() checks from FOR_ITER_RANGE opcode as they are no longer needed.

Took consideration of performance concern in: #129068, timings for this case are as follows:

gil old:       0.6393114424998203
gil new:       0.6314519609997660
gil newopt:    0.6323117769998134

nogil old:     0.7528951632993994
nogil new:     0.7935700326001097
nogil newopt:  0.6848774949000471  # opmitized "math"

New gil-enabled build is probably a little faster due to the removal of _ITER_JUMP_RANGE.

Script:

from timeit import timeit

t = timeit('for v in range_it: pass', 'range_it = range(100000)', number=1000)

print(f't: {t}')

Latest pyperformance bench gives 1% IMPROVEMENT for this PR in free-threaded over main (probably just variance).

There are three jit test_capi.test_opt tests which fail now, which looks like due to the changes to the micro-op behavior.

FAIL: test_for_iter_range (test.test_capi.test_opt.TestUops.test_for_iter_range)
AssertionError: '_GUARD_NOT_EXHAUSTED_RANGE' not found in ['_START_EXECUTOR', '_MAKE_WARM', '_SET_IP', '_CHECK_PERIODIC', '_CHECK_VALIDITY', '_ITER_CHECK_RANGE', '_ITER_NEXT_RANGE_TIER_TWO', 

FAIL: test_pop_jump_if_none (test.test_capi.test_opt.TestUops.test_pop_jump_if_none)
AssertionError: '_GUARD_IS_NOT_NONE_POP' unexpectedly found in ['_START_EXECUTOR', '_MAKE_WARM', '_SET_IP', '_CHECK_PERIODIC', '_CHECK_VALIDITY', '_ITER_CHECK_RANGE', 

FAIL: test_int_type_propagate_through_range (test.test_capi.test_opt.TestUopsOptimization.test_int_type_propagate_through_range)
AssertionError: '_GUARD_BOTH_INT' unexpectedly found in ['_START_EXECUTOR', '_MAKE_WARM', '_SET_IP', '_CHECK_PERIODIC', '_CHECK_VALIDITY', '_ITER_CHECK_RANGE', '_ITER_NEXT_RANGE_TIER_TWO', 

P.S. No idea why the "generated files up to date" test fails, I did regen-all.

@tom-pytel
Copy link
Contributor Author

Ping @colesbury, @Yhg1s. Sorry to bug you guys with two PRs this weekend. Please consider this one an experiment and let me know if it is worth continuing with, and if so what would need to be changed?

@python-cla-bot
Copy link

python-cla-bot bot commented Apr 6, 2025

All commit authors signed the Contributor License Agreement.

CLA signed

serhiy-storchaka and others added 2 commits April 8, 2025 14:26
…seTuple() (pythonGH-128374)

Non-tuple sequences are deprecated as argument for the "(items)" format unit
in PyArg_ParseTuple() and other argument parsing functions if items contains
format units which store borrowed buffer or reference (e.g. "s" and "O").

str and bytearray are no longer accepted as valid sequences.
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.

2 participants