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-125837: SplitLOAD_CONST into three.#125972

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
markshannon merged 34 commits intopython:mainfromfaster-cpython:split-load-const
Oct 29, 2024

Conversation

@markshannon
Copy link
Member

@markshannonmarkshannon commentedOct 25, 2024•
edited by github-actionsbot
Loading

SplitsLOAD_CONST into three instructions

  • LOAD_INT for ints inrange(256). Avoids the need for a space in theco_consts tuple and avoids an incref
  • LOAD_CONST_IMMORTAL for other immortal objects. Avoids an incref
  • LOAD_CONST for the remaining constants

📚 Documentation preview 📚:https://cpython-previews--125972.org.readthedocs.build/

efimov-mikhail and vasily-v-ryabov reacted with thumbs up emoji
@picnixz
Copy link
Member

LOAD_INT for ints in range(256)

I think the range is -5 -> 256 (included) right? (at least _PyLong_SMALL_INTS works that way, but maybe 256 should be excluded even though it's stored in the _PyLong_SMALL_INTS).

@markshannon
Copy link
MemberAuthor

I think the range is -5 -> 256 (included) right? (at least _PyLong_SMALL_INTS works that way, but maybe 256 should be excluded even though it's stored in the _PyLong_SMALL_INTS).

I wantLOAD_INT 0 to push0 not-5 to keep things simple.
SoLOAD_INT will only coverrange(256) even though the range of statically allocated ints is larger.

We can always add -5,-4,-3,-2 and -1 toLOAD_COMMON_CONST later if we want.

picnixz, Eclips4, and efimov-mikhail reacted with thumbs up emoji

@markshannon
Copy link
MemberAuthor

Performance for tier 1 appears to be about 0.5% faster.

Stats show the following fraction of constants loaded:

OpcodeFraction
LOAD_INT47%
LOAD_CONST_IMMORTAL42%
LOAD_CONST11%

With a reduction in increfs of immortal objects of about 20%.

@brettcannonbrettcannon removed their request for reviewOctober 25, 2024 20:01
for (inti=0;i<b->b_iused;i++) {
if (OPCODE_HAS_CONST(b->b_instr[i].i_opcode)) {
intopcode=b->b_instr[i].i_opcode;
if (OPCODE_HAS_CONST(opcode)&&opcode!=LOAD_SMALL_INT) {
Copy link
Member

Choose a reason for hiding this comment

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

hasconst is defined in thedis docs as "Sequence of bytecodes that access a constant". I think it will make more sense if we clarify that it means "access a constant fromco_consts" and then removeLOAD_SMALL_INT fromhasconst andOPCODE_HAS_CONST.

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

That makes sense.LOAD_COMMON_CONST is not inhasconst.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe it's worth testing whether creating integers within therange(256) will not affectco_consts?

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

I don't want to go overboard on testing internal details of the bytecode compiler.
LOAD_CONST with a smallint is still correct, just a bit less efficient thanLOAD_SMALL_INT.
We already have plenty of tests intest_dis that test this

Eclips4 reacted with thumbs up emoji
@markshannonmarkshannon merged commitfaa3272 intopython:mainOct 29, 2024
63 checks passed
@markshannonmarkshannon deleted the split-load-const branchNovember 18, 2024 14:37
picnixz pushed a commit to picnixz/cpython that referenced this pull requestDec 8, 2024
* Add LOAD_CONST_IMMORTAL opcode* Add LOAD_SMALL_INT opcode* Remove RETURN_CONST opcode
ebonnal pushed a commit to ebonnal/cpython that referenced this pull requestJan 12, 2025
* Add LOAD_CONST_IMMORTAL opcode* Add LOAD_SMALL_INT opcode* Remove RETURN_CONST opcode
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@JelleZijlstraJelleZijlstraJelleZijlstra left review comments

@picnixzpicnixzpicnixz left review comments

@Eclips4Eclips4Eclips4 left review comments

@iritkatrieliritkatrieliritkatriel approved these changes

@ericsnowcurrentlyericsnowcurrentlyAwaiting requested review from ericsnowcurrentlyericsnowcurrently is a code owner

@ncoghlanncoghlanAwaiting requested review from ncoghlanncoghlan is a code owner

@warsawwarsawAwaiting requested review from warsawwarsaw is a code owner

@Fidget-SpinnerFidget-SpinnerAwaiting requested review from Fidget-SpinnerFidget-Spinner is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@markshannon@picnixz@JelleZijlstra@iritkatriel@Eclips4
[8]ページ先頭
©2009-2025 Movatter.jp