gh-144540: Eliminate`_MAKE_HEAP_FUNCTION` for owned references#146320

Draft

Sacul0457 wants to merge 1 commit intopython:mainfrom

Sacul0457:Remove-Redundant-_MAKE_HEAP_SAFE

Draft

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

Copy link

Contributor

Sacul0457 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...

Issue:Optimize reference tracking and eliminate branching during returns and yields #144540

optimize _MAKE_HEAP_FUNCTION for owned references

d5e9fdb

Sacul0457 requested review fromFidget-Spinner,markshannon,savannahostrowski andtomasr8 ascode owners

March 23, 2026 07:48

bedevere-appbot mentioned this pull request

Mar 23, 2026

Optimize reference tracking and eliminate branching during returns and yields#144540

Open

bedevere-appbot added the awaiting review label

Mar 23, 2026

Copy link

Contributor

cocolato commentedMar 23, 2026

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

cpython/Python/optimizer_symbols.c

Lines 690 to 709 infb8d8d9

	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.”

Copy link

Contributor

cocolato commentedMar 23, 2026

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

Copy link

Member

Fidget-Spinner commentedMar 23, 2026

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.