Skip to content

Conversation

@corona10
Copy link
Member

@corona10corona10 commented Aug 27, 2025

@corona10
Copy link
MemberAuthor

Might be better approach than #138173, but issue is that max_length will be shorter than current, not sure that this apporach will be fine.

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.

Nice!

@corona10corona10 marked this pull request as ready for review August 28, 2025 15:49
// Need space for _DEOPT
max_length--;

max_length-=2;

Choose a reason for hiding this comment

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

Needs comment on why 2:
// One for possible _DEOPT, one because _CHECK_VALIDITY itself might _DEOPT

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. Will wait for Mark to agree.

@corona10
Copy link
MemberAuthor

Gentle ping @markshannon: I plan to create a separate PR to move this to the thread state after merging this PR.

@corona10corona10 merged commit 5edfe55 into python:mainSep 9, 2025
60 checks passed
lkollar pushed a commit to lkollar/cpython that referenced this pull request Sep 9, 2025
# deallocators may be traced as well.
defsetUp(self):
ifos.environ.get('PYTHON_UOPS_OPTIMIZE') =='0':
self.skipTest("Line tracing behavior differs when JIT optimizer is disabled")
Copy link
Member

Choose a reason for hiding this comment

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

Wait, why/how does the behavior differ? Sounds like a bug.

Choose a reason for hiding this comment

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

Probably should open another issue for this.

Comment on lines +1285 to +1289
char*env_var=Py_GETENV("PYTHON_UOPS_OPTIMIZE");
boolis_noopt= true;
if (env_var==NULL||*env_var=='\0'||*env_var>'0'){
is_noopt= false;
}
Copy link
Member

Choose a reason for hiding this comment

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

Why was this change made? I think the previous code (without the additional variable) made more sense.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@corona10@Fidget-Spinner@brandtbucher