From 789b63ca87613488586b3e2825fdeb2b64326c24 Mon Sep 17 00:00:00 2001 From: Richard Leach Date: Sat, 29 Nov 2025 00:25:01 +0000 Subject: [PATCH 1/3] Perl_sv_backoff - only copy the buffer contents if SvOK(sv) When undoing an OOK, `Perl_sv_backoff` will always copy the shifted buffer contents down to the start of the buffer. That's required when the contents are live, unfortunate when the contents are defunct but tiny, and can have a noticeably bad effect on performance when the contents are defunct but large. See https://github.com/Perl/perl5/issues/23967 for an example of the latter. With this commit the copy will only occur if `SvOK(sv)`. This test is expected to indicate whether the buffer contents are active or defunct. --- sv.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/sv.c b/sv.c index 146e7ac61d8b..4e161322646f 100644 --- a/sv.c +++ b/sv.c @@ -1336,6 +1336,8 @@ wrapper instead. /* prior to 5.000 stable, this function returned the new OOK-less SvFLAGS prior to 5.23.4 this function always returned 0 + prior to 5.43.x, the contents of the string buffer were always copied + down to the start of the buffer. Now, this only happens if SvOK(sv) */ void @@ -1355,7 +1357,11 @@ Perl_sv_backoff(SV *const sv) SvLEN_set(sv, SvLEN(sv) + delta); SvPV_set(sv, SvPVX(sv) - delta); SvFLAGS(sv) &= ~SVf_OOK; - Move(s, SvPVX(sv), SvCUR(sv)+1, char); + + /* Don't copy a buffer if the contents are already defunct. */ + if (SvOK(sv)) + Move(s, SvPVX(sv), SvCUR(sv)+1, char); + return; } From 3fab4648b2db63d7dba6dff15863cbeddda2e4f7 Mon Sep 17 00:00:00 2001 From: Richard Leach Date: Sat, 29 Nov 2025 00:31:51 +0000 Subject: [PATCH 2/3] Perl_leave_scope - sv_backoff shouldn't do an unnecessay string copy When a `my` SV goes out of scope, any OOK hack on its string buffer is undone by `Perl_sv_backoff`. If the SV is `SvOK`, a copy of the buffer contents will occur, but since the contents are defunct at this point, the copy is unnecessary. See https://github.com/Perl/perl5/issues/23967 as an example of where this unnecessary copy had a noticeable effect on performance. This commit essentially inlines the necessary parts of `sv_backoff` to avoid the copy, without excessive messing around with `sv`'s flags at the call site in `Perl_leave_scope`. --- scope.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/scope.c b/scope.c index b8063c27760b..dddd1018bf7c 100644 --- a/scope.c +++ b/scope.c @@ -1440,8 +1440,20 @@ Perl_leave_scope(pTHX_ I32 base) if (SvTYPE(sv) == SVt_PVHV && HvHasAUX(sv)) Perl_hv_kill_backrefs(aTHX_ MUTABLE_HV(sv)); - else if(SvOOK(sv)) - sv_backoff(sv); + else if(SvOOK(sv)) { + /* Inlined sv_backoff() - the buffer contents are + * defunct and there's no need to copy them. All that + * is needed is resetting SvLEN and the SvPVX pointer. */ + assert(SvTYPE(sv) != SVt_PVHV); /* the branch above */ + assert(SvTYPE(sv) != SVt_PVAV); + + STRLEN delta; + SvOOK_offset(sv, delta); + + SvLEN_set(sv, SvLEN(sv) + delta); + SvPV_set(sv, SvPVX(sv) - delta); + SvFLAGS(sv) &= ~SVf_OOK; + } if (SvMAGICAL(sv)) { /* note that backrefs (either in HvAUX or magic) From 21977deb1e5f036d9279971aadb4b201f155960a Mon Sep 17 00:00:00 2001 From: Richard Leach Date: Tue, 2 Dec 2025 23:03:37 +0000 Subject: [PATCH 3/3] Perldelta for GH#23968: OOK buffer copy only when SvOK(sv) --- pod/perldelta.pod | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/pod/perldelta.pod b/pod/perldelta.pod index 37f2fd23163f..4e87d9d6228b 100644 --- a/pod/perldelta.pod +++ b/pod/perldelta.pod @@ -348,6 +348,19 @@ well. =item * +C, which undoes the OOK string offset mechanism now only +moves the string buffer contents to the start of the buffer if `SvOK(sv)` +is true. Historically, the buffer contents were always moved, but if the +buffer contents are defunct and no longer required, doing that incurs an +unnecessary cost proportional to the size of the shifted contents. + +(The assumption in this change is that `SvOK(sv)` is a valid indicator of +whether the string buffer contents are "live" or not.) + +See GH#23967 for an example of where such a copy was noticeable. + +=item * + XXX =back