Skip to content

gh-153486: Factor non-negative compact int subscript guards#153487

Open
KRRT7 wants to merge 5 commits into
python:mainfrom
KRRT7:list-int-compact-guard-main
Open

gh-153486: Factor non-negative compact int subscript guards#153487
KRRT7 wants to merge 5 commits into
python:mainfrom
KRRT7:list-int-compact-guard-main

Conversation

@KRRT7

@KRRT7 KRRT7 commented Jul 10, 2026

Copy link
Copy Markdown

Factor a dedicated non-negative compact-int guard into the specialized subscript/store-subscript fast paths for list, tuple, and str.

This makes the specialization contract explicit and removes redundant checks from the specialized bodies. Negative indices still fall back to the generic path.

Issue: gh-153486

@KRRT7 KRRT7 requested a review from markshannon as a code owner July 10, 2026 07:31
@bedevere-app

bedevere-app Bot commented Jul 10, 2026

Copy link
Copy Markdown

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

@KRRT7

KRRT7 commented Jul 10, 2026

Copy link
Copy Markdown
Author

I ran a fresh set of perf checks for this on a Linux VM, rebuilding both main and this branch from scratch and benchmarking pinned to a single CPU (taskset -c 1 + chrt -f 99; pyperf system show reported turbo already disabled).

I used a medium pyperformance subset. The overall picture looks mixed rather than clearly better or worse:

  • richards: about 1.05x faster, reproduced on rerun
  • nbody: about 1.03x faster, reproduced on rerun
  • xml_etree_generate: about 1.02x-1.03x slower, reproduced on rerun

Other differences were either not significant or did not reproduce on rerun (json_dumps, json_loads, pickle, pickle_dict, regex_v8, spectral_norm, xml_etree_parse, xml_etree_iterparse, xml_etree_process).

@KRRT7

KRRT7 commented Jul 10, 2026

Copy link
Copy Markdown
Author

I dug into the negative-index case further and found that the important change here is at specialization time, not just in the specialized opcode body.

With this update, list subscripts using exact compact negative ints can specialize to BINARY_OP_SUBSCR_LIST_INT and stay on the specialized path, instead of remaining on generic BINARY_OP. I verified that on a bare-metal VM: on main, a site like f(L, idx) with idx = -1 stays generic, while with this change the same site specializes to BINARY_OP_SUBSCR_LIST_INT.

I then measured two focused microbenchmarks that exercise that case directly:

  1. Negative variable index
L = list(range(100))
idx = -1
x = L[idx]
  1. Mixed-sign workload
L = list(range(100))
idxs = (42, -1) * 100

def f(L, idxs):
    s = 0
    for idx in idxs:
        s += L[idx]
    return s

Pinned pyperf runs on that setup gave:

  • Negative variable index:

    • main: 13.7 ns +- 1.1 ns
    • patched: 6.66 ns +- 0.46 ns
    • 2.05x faster
  • Mixed-sign workload:

    • main: 3.39 us +- 0.14 us
    • patched: 2.36 us +- 0.08 us
    • 1.44x faster

So the win here is specifically for list subscript sites that use negative compact int indices and can now remain specialized.

@markshannon

Copy link
Copy Markdown
Member

Why only lists, not tuples or strings?

It is easier to keep specialization and execution consistent if specialization is just by compactness, not by sign, for all specializations.

@KRRT7

KRRT7 commented Jul 10, 2026

Copy link
Copy Markdown
Author

Why only lists, not tuples or strings?

It is easier to keep specialization and execution consistent if specialization is just by compactness, not by sign, for all specializations.

well, one question on scope: this PR currently covers the list negative-index specialization follow-up since it directly regressed the cases being optimized. For the other related specializations in this area, would you prefer they be folded into this PR as well, or handled in a separate follow-up PR?

@KRRT7

KRRT7 commented Jul 10, 2026

Copy link
Copy Markdown
Author

answered my own question there by splitting up the work based on what they were doing, I've opened #153487 with the intention that this PR gets merged first.

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