Skip to content

Perl_op_convert_list - only short circuit CONST OPs with an IsCOW SV #23296

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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 17, 2025

Conversation

richardleach
Copy link
Contributor

Currently, a CONST OP's SV will not have the IsCOW flag set if the PV buffer was truncated such that it is too small to be COWed. In which case, not short circuting Perl_op_convert_list causes the OP to undergo constant folding, resulting in a CONST OP with an SV that can participate in COW.

This means that the commit which introduced the short-circuit for CONST OPs accidentally introduced a performance regression:
a902d92

This commit adds an additional check to ensure that short circuiting does not happen when the CONST OP SV cannot be COWed. This is a short term workaround given the proximity to the next stable release. #23290 seems like the more appropriate fix, but is a bigger change, so will be held until the next development cycle.


  • This set of changes does not require a perldelta entry.

Currently, a CONST OP's SV will not have the `IsCOW` flag set if
the PV buffer was truncated such that it is too small to be COWed.
In which case, not short circuting `Perl_op_convert_list` causes
the OP to undergo constant folding, resulting in a CONST OP with
an SV that can participate in COW.

This means that the commit which introduced the short-circuit for
CONST OPs accidentally introduced a regression:
Perl@a902d92

This commit adds an additional check to ensure that short circuiting
does not happen when the CONST OP SV cannot be COWed. This is a
short term workaround given the proximity to the next stable release.
Perl#23290 seems like the more
appropriate fix, but is a bigger change, so will be held until the
next development cycle.
@richardleach richardleach requested a review from iabyn May 16, 2025 21:22
Copy link
Contributor

@iabyn iabyn left a comment

Choose a reason for hiding this comment

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

LGTM

@richardleach richardleach merged commit b20c83a into Perl:blead May 17, 2025
34 checks passed
@richardleach richardleach deleted the convert_list-IsCOW branch May 17, 2025 08:24
Sign up for free to 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.

2 participants