GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-143732: add specialization for `FOR_ITER` by NekoAsakura · Pull Request #148745 · python/cpython

Conversation

@NekoAsakura

@NekoAsakura NekoAsakura commented Apr 19, 2026 •
edited by bedevere-app Bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member
main branch ratio
for_iter_dict_items 110.23 ms ± 1.74 ms 99.29 ms ± 1.24 ms 1.11× faster
for_iter_dict_keys 85.20 ms ± 1.03 ms 76.20 ms ± 1.02 ms 1.12× faster
for_iter_dict_values 84.91 ms ± 1.15 ms 76.27 ms ± 1.24 ms 1.11× faster
for_iter_set 96.88 ms ± 1.05 ms 87.46 ms ± 1.16 ms 1.11× faster
for_iter_reversed 79.99 ms ± 0.77 ms 72.03 ms ± 0.79 ms 1.11× faster
for_iter_enumerate 123.65 ms ± 1.94 ms 110.14 ms ± 1.82 ms 1.12× faster
for_iter_zip 131.47 ms ± 1.38 ms 122.17 ms ± 2.87 ms 1.08× faster
for_iter_list 59.21 ms ± 1.17 ms 59.48 ms ± 1.06 ms 1.00× slower
for_iter_tuple 53.78 ms ± 0.61 ms 53.77 ms ± 0.74 ms 1.00× faster
for_iter_range 72.62 ms ± 0.80 ms 72.55 ms ± 1.08 ms 1.00× faster
  • Issue: Broader specialization in the Specializing Adaptive Interpreter for better JIT performance #143732

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
stonebig, Fidget-Spinner, cocolato, Object905, and maurycy reacted with heart emoji eendebakpt reacted with eyes emoji
All reactions
  • ❤️ 5 reactions
  • 👀 1 reaction
@NekoAsakura
pythongh-143732: add specialization for FOR_ITER
dc169d8
@NekoAsakura
NekoAsakura requested review from Fidget-Spinner, markshannon, savannahostrowski and tomasr8 as code owners April 19, 2026 06:19
@bedevere-app bedevere-app Bot added the awaiting review label Apr 19, 2026
@bedevere-app bedevere-app Bot mentioned this pull request Apr 19, 2026
Broader specialization in the Specializing Adaptive Interpreter for better JIT performance #143732
Open
NekoAsakura added 3 commits April 19, 2026 02:41
@NekoAsakura
Merge branch 'main' into pythongh-143732/for-iter-type-recording
66c11de
@NekoAsakura
Make slot compatible with FOR_ITER_GEN
3a934bf
@NekoAsakura
Minimise diff
5700f6a
cocolato
cocolato reviewed Apr 19, 2026
View reviewed changes

@cocolato cocolato left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

LGTM, thanks for doing this!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
NekoAsakura and maurycy reacted with heart emoji
All reactions
  • ❤️ 2 reactions
Comment thread Python/bytecodes.c Outdated
}

macro(FOR_ITER) = _SPECIALIZE_FOR_ITER + _FOR_ITER;
macro(FOR_ITER) = _SPECIALIZE_FOR_ITER + _RECORD_NOS_GEN_FUNC + _RECORD_NOS_TYPE + _FOR_ITER;

@cocolato cocolato Apr 19, 2026

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

cpython/Python/optimizer_symbols.c

Lines 725 to 726 in ad7d361

case JIT_SYM_RECORDED_GEN_FUNC_TAG:
return &PyGen_Type;

We don't need _RECORD_NOS_GEN_FUNC here, because the GEN_FUNC_TAG recorded here does not contribute to the current optimization.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@NekoAsakura NekoAsakura Apr 19, 2026

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Every FOR_ITER specialisation's record list must be a prefix of FOR_ITER's.
_RECORD_NOS_GEN_FUNC writes a gen func or NULL to slot 0, matching what FOR_ITER_GEN reads from it.
https://github.com/python/cpython/actions/runs/24623062259/job/71997168333

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
cocolato reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
Comment thread Python/optimizer_bytecodes.c
PyType_Watch(TYPE_WATCHER_ID, (PyObject *)probable);
_Py_BloomFilter_Add(dependencies, probable);
sym_set_type(iter, probable);
int32_t orig_target = (this_instr - 1)->target;

@cocolato cocolato Apr 19, 2026 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

We should add an assert to make sure the last uop is _RECORD_NOS_TYPE

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@cocolato cocolato added the skip news label Apr 19, 2026
@NekoAsakura
Add uop assert
1f48326
markshannon
markshannon requested changes May 1, 2026
View reviewed changes

@markshannon markshannon left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I've a few suggestions inline.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
NekoAsakura reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
Comment thread Python/optimizer_bytecodes.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Python/optimizer_bytecodes.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Python/bytecodes.c
}

tier2 op(_ITER_NEXT_INLINE, (iternext_fn/4, iter, null_or_index -- iter, null_or_index, next)) {
volatile iternextfunc iternext_v = (iternextfunc)iternext_fn;

@markshannon markshannon May 1, 2026 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Why volatile? It shouldn't be necessary.
Also function pointers may not be the same size as normal pointers.
Can you add assert(sizeof(iternextfunc) == sizeof(uintptr_t)); to be on the safe side.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@NekoAsakura NekoAsakura May 2, 2026

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Because mmap pages usually sit too far from tp_iternext for the offset to reach, we have to use volatile to force compiler to emit callq *%rax (target read from a register) instead of callq <rel32> (target baked in as a fixed offset). Otherwise the call jumps to the wrong address and segfaults.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@markshannon markshannon May 2, 2026

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Does this fail just for x86 or for AArch64 as well?

This is a bug in the JIT and we fix it, rather than add workarounds.
I think we should be adding a trampoline here, but it seems that we are not.
If you're looking at the stencils, can you see how it is being patched (what patch function is being called)?

@brandtbucher

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
brandtbucher reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

@NekoAsakura NekoAsakura May 3, 2026

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Both. (aarch64 was tested under qemu-user)
Trampoline path doesn't appear to be designed for _JIT_* symbols:

cpython/Tools/jit/_stencils.py

Lines 434 to 446 in 7c9ad27

def symbol_to_value(symbol: str) -> tuple[HoleValue, str | None]:
"""
Convert a symbol name to a HoleValue and a symbol name.
Some symbols (starting with "_JIT_") are special and are converted to their
own HoleValues.
"""
if symbol.startswith("_JIT_"):
try:
return HoleValue[symbol.removeprefix("_JIT_")], None
except KeyError:
pass
return HoleValue.ZERO, symbol

cpython/Tools/jit/_stencils.py

Lines 288 to 292 in 7c9ad27

if (
hole.kind
in {"R_AARCH64_CALL26", "R_AARCH64_JUMP26", "ARM64_RELOC_BRANCH26"}
and hole.value is HoleValue.ZERO
and hole.symbol not in self.symbols

cpython/Tools/jit/_stencils.py

Lines 305 to 309 in 7c9ad27

# x86_64 Darwin trampolines for external symbols
elif (
hole.kind == "X86_64_RELOC_BRANCH"
and hole.value is HoleValue.ZERO
and hole.symbol not in self.symbols

Even if we extended it to those, we'd need a stub per instance. I don't think it gains us anything.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@markshannon markshannon May 3, 2026

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

We only need a trampoline if the jitted code is too far from the executable. See #148822

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@bedevere-app bedevere-app Bot added awaiting changes and removed awaiting review labels May 1, 2026
@bedevere-app

bedevere-app Bot commented May 1, 2026

Copy link
Copy Markdown

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

And if you don't make the requested changes, you will be poked with soft cushions!
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
NekoAsakura added 3 commits May 1, 2026 16:15
@NekoAsakura
address feedback
06ce7a4
@NekoAsakura
Merge branch 'main' into pythongh-143732/for-iter-type-recording
4003e8c
@NekoAsakura
uniform nos recording across FOR_ITER
0f69071
@read-the-docs-community

read-the-docs-community Bot commented May 2, 2026 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

Documentation build overview

📚 cpython-previews | 🛠️ Build #32525858 | 📁 Comparing a0551fe against main (4b33308)

  🔍 Preview build  

81 files changed · + 1 added · ± 80 modified

+ Added

  • c-api/sentinel.html

± Modified

  • glossary.html
  • c-api/buffer.html
  • c-api/code.html
  • c-api/concrete.html
  • c-api/descriptor.html
  • c-api/exceptions.html
  • c-api/gen.html
  • c-api/intro.html
  • c-api/perfmaps.html
  • c-api/set.html
  • and 70 more...
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
NekoAsakura added 2 commits May 2, 2026 09:28
@NekoAsakura
Merge branch 'main' into pythongh-143732/for-iter-type-recording
724e285
@NekoAsakura
Merge branch 'main' into pythongh-148871/load-common-constant
cbd17a2
@markshannon markshannon mentioned this pull request May 3, 2026
JIT does not emit trampolines for jumps to addresses encoded as operands #149316
Open
@markshannon

markshannon commented May 3, 2026

Copy link
Copy Markdown
Member

OK, well let's go with your approach for now. We already do the same here

I've created #149316 so we won't need the "volatile"
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@markshannon
markshannon self-requested a review May 3, 2026 09:54
markshannon
markshannon reviewed May 3, 2026
View reviewed changes
Comment thread Python/bytecodes.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
markshannon
markshannon reviewed May 3, 2026
View reviewed changes
Comment thread Python/optimizer_bytecodes.c Outdated
op(_FOR_ITER_TIER_TWO, (iter, null_or_index -- iter, null_or_index, next)) {
PyTypeObject *type = sym_get_type(iter);
if (type != NULL && type != &PyGen_Type && type->tp_iternext != NULL) {
ADD_OP(_ITER_NEXT_INLINE, 0, (uintptr_t)type->tp_iternext);

@markshannon markshannon May 3, 2026

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

We need the watcher here as well.

What we do elsewhere is something like this:

bool definite = true;
PyTypeObject *type = sym_get_type(iter);
if (type == NULL) {
    type = sym_get_probable_type(iter);
    definite = false;
}
if (type != NULL) {
   // Add if not definite, otherwise NOP
   // Add watcher(s)
}

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
NekoAsakura added 2 commits May 3, 2026 10:07
@NekoAsakura
address feedback
49baba9
@NekoAsakura
Merge branch 'main' into pythongh-143732/for-iter-type-recording
fc9da2c
markshannon
markshannon reviewed May 4, 2026
View reviewed changes
Comment thread Python/bytecodes.c Outdated

macro(FOR_ITER_VIRTUAL) =
unused/1 + // Skip over the counter
_RECORD_NOS + // Required for family-uniform recording (gh-148571).

@markshannon markshannon May 4, 2026

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

The analyzer should take the union of all the family members when considering what to record.
Can you try removing this?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@cocolato cocolato May 4, 2026

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I create an pr to resolve this: #149359

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
NekoAsakura reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@cocolato cocolato mentioned this pull request May 4, 2026
gh-149358: Generating JIT recorder transformers and stabilizing family layouts #149359
Merged
NekoAsakura added 3 commits May 4, 2026 08:35
@NekoAsakura
update family-uniform recording
a0551fe
@NekoAsakura
Merge branch 'main' into pythongh-143732/for-iter-type-recording
52e0325
@NekoAsakura
update test
826b21d
@markshannon
markshannon self-requested a review May 4, 2026 16:26
markshannon
markshannon reviewed May 4, 2026
View reviewed changes

@markshannon markshannon left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

This looks good now, thanks.

We can improve the tooling later. The generated code is good with the workarounds.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
NekoAsakura reacted with heart emoji
All reactions
  • ❤️ 1 reaction
@markshannon
markshannon self-requested a review May 4, 2026 16:27
markshannon
markshannon approved these changes May 4, 2026
View reviewed changes
@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting changes labels May 4, 2026
@markshannon
markshannon merged commit 9846407 into python:main May 4, 2026
86 of 87 checks passed

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot removed the awaiting merge label May 4, 2026
@NekoAsakura
NekoAsakura deleted the gh-143732/for-iter-type-recording branch May 4, 2026 17:34
@savannahostrowski savannahostrowski mentioned this pull request May 7, 2026
JIT hang in xml.etree.iterparse #149481
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cocolato cocolato cocolato left review comments
@markshannon markshannon markshannon approved these changes
@tomasr8 tomasr8 Awaiting requested review from tomasr8 tomasr8 is a code owner
@Fidget-Spinner Fidget-Spinner Awaiting requested review from Fidget-Spinner Fidget-Spinner is a code owner
@savannahostrowski savannahostrowski Awaiting requested review from savannahostrowski

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@NekoAsakura @markshannon @cocolato
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.