Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

gh-139757: Add BINARY_OP_SUBSCR_USTR_INT#143389

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
Fidget-Spinner merged 11 commits intopython:mainfromchris-eibl:substr_cstr_int
Jan 4, 2026

Conversation

@chris-eibl
Copy link
Member

@chris-eiblchris-eibl commentedJan 3, 2026•
edited
Loading

Since#140800BINARY_OP_SUBSCR_STR_INT only specializes for compact ASCII strings. Let's introduceBINARY_OP_SUBSCR_USTR_INT to specialize again for reading an ASCII character from any string.

passes the tests and brings bm_tomli back to normal, i.e.15 % speedup
@Fidget-Spinner
Copy link
Member

I verified a 9% speedup on this PR for thetomli_loads benchmark on pyperformance:

Mean +- std dev: [spec_off] 2.24 sec +- 0.03 sec -> [spec_on] 2.05 sec +- 0.02 sec: 1.09x faster

Apparently, we regressed earlier inhttps://github.com/python/cpython/pull/140800/files. This caused the 10% slowdown in tomli loads. Which seemingly uses a lot of unicode strings.

I'm going to merge this PR to remove the perf regression.

chris-eibl reacted with thumbs up emoji

Copy link
Member

@Fidget-SpinnerFidget-Spinner left a comment

Choose a reason for hiding this comment

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

Just need tests and two comments. Thanks!

res = PyStackRef_FromPyObjectBorrow(res_o);
}

macro(BINARY_OP_SUBSCR_NCSTR_INT) =

Choose a reason for hiding this comment

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

When you have the time, please change then name toBINARY_OP_SUBSCR_USTR_INT.

chris-eibl reacted with thumbs up emoji
}

macro(BINARY_OP_SUBSCR_NCSTR_INT) =
_GUARD_TOS_INT + _GUARD_NOS_UNICODE + unused/5 + _BINARY_OP_SUBSCR_NCSTR_INT + _POP_TOP_INT + POP_TOP;

Choose a reason for hiding this comment

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

Is POP_TOP_UNICODE instead of POP_TOP safe here?

chris-eibl reacted with thumbs up emoji
@chris-eiblchris-eibl changed the titleAdd BINARY_OP_SUBSCR_NCSTR_INTgh-139757: Add BINARY_OP_SUBSCR_USTR_INTJan 4, 2026
@chris-eiblchris-eibl added performancePerformance or resource usage interpreter-core(Objects, Python, Grammar, and Parser dirs) labelsJan 4, 2026
self.assertNotIn("_GUARD_TOS_UNICODE", uops)
self.assertIn("_BINARY_OP_ADD_UNICODE", uops)

def test_binary_subcsr_ustr_int_narrows_to_str(self):
Copy link
MemberAuthor

Choose a reason for hiding this comment

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

I thought of paramterizingtest_binary_subcsr_str_int_narrows_to_str andtest_binary_op_subscr_str_int to get rid of duplication, but it resulted in worse readable code, because the input and the expectations have to be varied ...

@Fidget-Spinner
Copy link
Member

Sorry for leaving 3 separate review comments instead of one review. I only picked up on some of these while looking over them again.

Copy link
Member

@Fidget-SpinnerFidget-Spinner left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks.

Co-authored-by: Chris Eibl <138194463+chris-eibl@users.noreply.github.com>
@Fidget-SpinnerFidget-Spinner merged commite6bfe4d intopython:mainJan 4, 2026
70 checks passed
@bedevere-bot
Copy link

⚠️⚠️⚠️ Buildbot failure⚠️⚠️⚠️

Hi! The buildbotARM64 macOS 3.x (tier-2) has failed when building commite6bfe4d.

What do you need to do:

  1. Don't panic.
  2. Checkthe buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. Go to the page of the buildbot that failed (https://buildbot.python.org/#/builders/725/builds/12456) and take a look at the build logs.
  4. Check if the failure is related to this commit (e6bfe4d) or if it is a false positive.
  5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/#/builders/725/builds/12456

Failed tests:

  • test_urllib2net

Summary of the results of the build (if available):

==

Click to see traceback logs
remote:Enumerating objects: 54, done.remote:Counting objects:   1% (1/54)remote:Counting objects:   3% (2/54)remote:Counting objects:   5% (3/54)remote:Counting objects:   7% (4/54)remote:Counting objects:   9% (5/54)remote:Counting objects:  11% (6/54)remote:Counting objects:  12% (7/54)remote:Counting objects:  14% (8/54)remote:Counting objects:  16% (9/54)remote:Counting objects:  18% (10/54)remote:Counting objects:  20% (11/54)remote:Counting objects:  22% (12/54)remote:Counting objects:  24% (13/54)remote:Counting objects:  25% (14/54)remote:Counting objects:  27% (15/54)remote:Counting objects:  29% (16/54)remote:Counting objects:  31% (17/54)remote:Counting objects:  33% (18/54)remote:Counting objects:  35% (19/54)remote:Counting objects:  37% (20/54)remote:Counting objects:  38% (21/54)remote:Counting objects:  40% (22/54)remote:Counting objects:  42% (23/54)remote:Counting objects:  44% (24/54)remote:Counting objects:  46% (25/54)remote:Counting objects:  48% (26/54)remote:Counting objects:  50% (27/54)remote:Counting objects:  51% (28/54)remote:Counting objects:  53% (29/54)remote:Counting objects:  55% (30/54)remote:Counting objects:  57% (31/54)remote:Counting objects:  59% (32/54)remote:Counting objects:  61% (33/54)remote:Counting objects:  62% (34/54)remote:Counting objects:  64% (35/54)remote:Counting objects:  66% (36/54)remote:Counting objects:  68% (37/54)remote:Counting objects:  70% (38/54)remote:Counting objects:  72% (39/54)remote:Counting objects:  74% (40/54)remote:Counting objects:  75% (41/54)remote:Counting objects:  77% (42/54)remote:Counting objects:  79% (43/54)remote:Counting objects:  81% (44/54)remote:Counting objects:  83% (45/54)remote:Counting objects:  85% (46/54)remote:Counting objects:  87% (47/54)remote:Counting objects:  88% (48/54)remote:Counting objects:  90% (49/54)remote:Counting objects:  92% (50/54)remote:Counting objects:  94% (51/54)remote:Counting objects:  96% (52/54)remote:Counting objects:  98% (53/54)remote:Counting objects: 100% (54/54)remote:Counting objects: 100% (54/54), done.remote:Compressing objects:   3% (1/27)remote:Compressing objects:   7% (2/27)remote:Compressing objects:  11% (3/27)remote:Compressing objects:  14% (4/27)remote:Compressing objects:  18% (5/27)remote:Compressing objects:  22% (6/27)remote:Compressing objects:  25% (7/27)remote:Compressing objects:  29% (8/27)remote:Compressing objects:  33% (9/27)remote:Compressing objects:  37% (10/27)remote:Compressing objects:  40% (11/27)remote:Compressing objects:  44% (12/27)remote:Compressing objects:  48% (13/27)remote:Compressing objects:  51% (14/27)remote:Compressing objects:  55% (15/27)remote:Compressing objects:  59% (16/27)remote:Compressing objects:  62% (17/27)remote:Compressing objects:  66% (18/27)remote:Compressing objects:  70% (19/27)remote:Compressing objects:  74% (20/27)remote:Compressing objects:  77% (21/27)remote:Compressing objects:  81% (22/27)remote:Compressing objects:  85% (23/27)remote:Compressing objects:  88% (24/27)remote:Compressing objects:  92% (25/27)remote:Compressing objects:  96% (26/27)remote:Compressing objects: 100% (27/27)remote:Compressing objects: 100% (27/27), done.remote:Total 28 (delta 26), reused 2 (delta 1), pack-reused 0 (from 0)From https://github.com/python/cpython * branch                    main       -> FETCH_HEADNote:switching to 'e6bfe4d8869e046a91d091611d3c7b5dccdaf0d6'.You are in 'detached HEAD' state. You can look around, make experimentalchanges and commit them, and you can discard any commits you make in thisstate without impacting any branches by switching back to a branch.If you want to create a new branch to retain commits you create, you maydo so (now or later) by using -c with the switch command. Example:  git switch -c <new-branch-name>Or undo this operation with:  git switch -Turn off this advice by setting config variable advice.detachedHead to falseHEAD is now at e6bfe4d8869 gh-139757: Add BINARY_OP_SUBSCR_USTR_INT (GH-143389)Switched to and reset branch 'main'make:*** [buildbottest] Error 2

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@Fidget-SpinnerFidget-SpinnerFidget-Spinner approved these changes

@markshannonmarkshannonAwaiting requested review from markshannonmarkshannon is a code owner

@tomasr8tomasr8Awaiting requested review from tomasr8tomasr8 is a code owner

Assignees

No one assigned

Labels

interpreter-core(Objects, Python, Grammar, and Parser dirs)performancePerformance or resource usage

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@chris-eibl@Fidget-Spinner@bedevere-bot
[8]ページ先頭
©2009-2026 Movatter.jp