Skip to content

Commit

Permalink
S_fold_constants: remove early SvREADONLY(sv) to allow SvIsCOW(sv)
Browse files Browse the repository at this point in the history
Standard CONST PVs have the IsCOW flag set, meaning that COW can
be used when assigning the CONST to a variable, rather than making
a copy of the buffer. CONST PVs arising from constant folding have
been lacking this flag, leading to unnecessary copying of PV buffers.

This seems to have occurred because a common branch in S_fold_constants
marks SVs as READONLY before the new CONST OP is created. When the OP
is created, the Perl_ck_svconst() check function is called - this is
the same as when a standard CONST OP is created. If the SV is not
already marked as READONLY, the check function will try to set IsCOW
if it is safe to do so, then in either case will make sure that the
READONLY flag is set.

This commit therefore removes the SvREADONLY(sv) statement from
S_fold_constants(), allowing Perl_ck_svconst() to set the IsCOW
and READONLY flags itself. Minor test updates are also included.
  • Loading branch information
richardleach committed Feb 12, 2024
1 parent ef4a769 commit bd91d16
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 4 deletions.
4 changes: 2 additions & 2 deletions ext/Devel-Peek/t/Peek.t
Original file line number Diff line number Diff line change
Expand Up @@ -551,7 +551,7 @@ do_test('string with Unicode',
. '"\\\0 \[UTF8 "\\\x\{100\}\\\x\{0\}\\\x\{200\}"\]
CUR = 5
LEN = \\d+
COW_REFCNT = 1 # $] < 5.019007
COW_REFCNT = 1 # $] < 5.019007 || $] >=5.041000
');

do_test('reference to hash containing Unicode',
Expand All @@ -575,7 +575,7 @@ do_test('reference to hash containing Unicode',
PV = $ADDR "' . $cp200_bytes . '"\\\0 \[UTF8 "\\\x\{200\}"\]
CUR = 2
LEN = \\d+
COW_REFCNT = 1 # $] < 5.019007
COW_REFCNT = 1 # $] < 5.019007 || $] >=5.041000
', '',
$] >= 5.015
? undef
Expand Down
4 changes: 3 additions & 1 deletion op.c
Original file line number Diff line number Diff line change
Expand Up @@ -4980,7 +4980,9 @@ S_fold_constants(pTHX_ OP *const o)
SvPADTMP_off(sv);
else if (!SvIMMORTAL(sv)) {
SvPADTMP_on(sv);
SvREADONLY_on(sv);
/* Do not set SvREADONLY(sv) here. newSVOP will call
* Perl_ck_svconst, which will do it. Setting it early
* here prevents Perl_ck_svconst from setting SvIsCOW(sv).*/
}
newop = newSVOP(OP_CONST, 0, MUTABLE_SV(sv));
if (!is_stringify) newop->op_folded = 1;
Expand Down
4 changes: 3 additions & 1 deletion t/op/undef.t
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,9 @@ SKIP: {
my $out = runperl(stderr => 1,
progs => [ split /\n/, <<'EOS' ]);
require Devel::Peek;
my $f = q(x) x 40; $f = undef;
my $f = q(x) x 40;
chop $f; # Make sure that the PV buffer is not COWed
$f = undef;
Devel::Peek::Dump($f);
undef $f;
Devel::Peek::Dump($f);
Expand Down

0 comments on commit bd91d16

Please sign in to comment.