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-144540: Eliminate_MAKE_HEAP_FUNCTION for owned references#146320

Open
Sacul0457 wants to merge 1 commit intopython:mainfrom
Sacul0457:Remove-Redundant-_MAKE_HEAP_SAFE
Open

gh-144540: Eliminate_MAKE_HEAP_FUNCTION for owned references#146320
Sacul0457 wants to merge 1 commit intopython:mainfrom
Sacul0457:Remove-Redundant-_MAKE_HEAP_SAFE

Conversation

@Sacul0457
Copy link
Contributor

@Sacul0457Sacul0457 commentedMar 23, 2026•
edited by bedevere-appbot
Loading

If the reference of the returned object is not borrowed, we remove the_MAKE_HEAP_FUNCTION. Hence I ended up changing thetest_make_heap_safe_optimized_owned test to test that_MAKE_HEAP_FUNCTION is no longer in the trace as the value returned is not a borrowed reference.

Also, for a "return of a return", given the inner function returns a borrowed reference, the double_MAKE_HEAP_FUNCTION is reduced to just 1 :)

44OPTIMIZED:_MAKE_HEAP_SAFE_r11 (0,target=3,operand0=0,operand1=0)45OPTIMIZED:_RETURN_VALUE_r11 (0,target=3,operand0=0,operand1=0)46OPTIMIZED:_CHECK_VALIDITY_r11 (0,jump_target=71,operand0=0,operand1=0)47OPTIMIZED:_SET_IP_r11 (0,target=10,operand0=0x1b5f9947d24,operand1=0)48OPTIMIZED:_RETURN_VALUE_r11 (0,target=10,operand0=0,operand1=0)

I hope I didn't misunderstand the issue...

@cocolato
Copy link
Contributor

I think this might be a problem because in creation functions such assym_new_not_null andsym_new_unknown, :

JitOptRef
_Py_uop_sym_new_unknown(JitOptContext*ctx)
{
JitOptSymbol*res=sym_new(ctx);
if (res==NULL) {
returnout_of_space_ref(ctx);
}
returnPyJitRef_Wrap(res);
}
JitOptRef
_Py_uop_sym_new_not_null(JitOptContext*ctx)
{
JitOptSymbol*res=sym_new(ctx);
if (res==NULL) {
returnout_of_space_ref(ctx);
}
res->tag=JIT_SYM_NON_NULL_TAG;
returnPyJitRef_Wrap(res);
}

PyJitRef_Wrap produces a ref with tag = 0.
This means thattag = 0 is the default state, indicating “we have no specific information about borrowing/ownership,” rather than “definitely owned.” Only those explicitly marked with tag = 1 viaPyJitRef_Borrow() are “known to be borrowed.”

@cocolato
Copy link
Contributor

Mark also reminded me in the last PR:#144414 (review).

@Fidget-Spinner
Copy link
Member

Hmm Mark is indeed right. However, currently the only place that can generate borrowed references are LOAD_FAST_BORROW. I suspect we need to create a new tag bit for this to work.

We only support good perf on 64 bit systems right now. I propose we drop the 32 bit Windows support so that we have 3 tag bits to use and can support "unknown" references.

cocolato reacted with thumbs up emoji

@cocolato
Copy link
Contributor

I propose we drop the 32 bit Windows support so that we have 3 tag bits to use and can support "unknown" references.

I also don’t think it’s necessary to maintain the performance of legacy systems

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

Reviewers

@markshannonmarkshannonAwaiting requested review from markshannonmarkshannon is a code owner

@tomasr8tomasr8Awaiting requested review from tomasr8tomasr8 is a code owner

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

@savannahostrowskisavannahostrowskiAwaiting requested review from savannahostrowskisavannahostrowski is a code owner

Assignees

No one assigned

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Sacul0457@cocolato@Fidget-Spinner
[8]ページ先頭
©2009-2026 Movatter.jp