Dedicated SV copying code in place of Perl_sv_setsv_flags #23202

richardleach · 2025-04-15T23:29:12Z

Perl_sv_setsv_flags is the heavyweight function for assigning the value(s) of
a source SV to a destination SV. It contains many branches for preparing the
destination SV prior to assignment. However:

If the destination SV has just been created, much of that logic isn't needed.
When cloning a SV, simple assignments (particularly IVs and PVs) dominate.

This set of commits:

Extracts the "is this CoWable?" test from Perl_sv_setsv_flags into a macro.
Adds Perl_sv_freshcopy_flags and two static helper functions.
Modifies Perl_newSVsv_flags and Perl_sv_mortalcopy_flags to use them.
Standardizes a number of call sites that did their own things but really
should use Perl_newSVsv_flags or Perl_sv_mortalcopy_flags.

Using perl's test harness as a guide:

Bodyless code handles 45% of calls to Perl_newSVsv_flags and
57% of calls to Perl_sv_mortalcopy_flags.
The SVt_PV/SVp_POK code handles 32% of calls to
Perl_newSVsv_flags and 36% of calls to Perl_sv_mortalcopy_flags.
S_sv_freshcopy_flags code handles 95% of the remainder in
Perl_newSVsv_flags and 91% of the remainder in to Perl_sv_mortalcopy_flags.

With these changes compared with a build of blead:

perl -e 'for (1..100_000) { my $x = [ (1) x 1000 ]; }' runs 10% faster
perl -e 'for (1..100_000) { my $x = [ ("Perl") x 250 ]; }' runs 45% faster

This set of changes does require a perldelta entry and I'll write one post-merge.

pp.c

pp_hot.c

sv.c

Leont · 2025-04-29T20:52:21Z

Cloning is rather unfortunate choice of words, given that it has a very specific meaning in our codebase that is quite different from what this PR is about. Renaming the PR may be helpful.

richardleach · 2025-05-08T17:06:30Z

I've made a lot of changes following earlier comments - thanks for those - and have finally force-pushed.

These changes aren't complete. For example:

Measured performance seems worse than in the PR version, so I need to look into that
Might change sflag handling/ SvFLAGS(dsv) setting
Not settled on struct membet initialisation
Might still rename the function that is currently Perl_newSVsv_flags and have newSVsv_flags be a macro that checks (ssv) before calling the sv.c function.

richardleach · 2025-05-08T17:13:02Z

sv.c

+                   if (sflags & SVf_IsCOW) {
+                        sv_buf_to_rw(ssv);
+                    }


I'm unclear if there's a bug here carried over from Perl_sv_setsv_flags.

If (!(sflags & SVf_IsCOW)) above, sflags isn't updated to reflect that SVf_Is_COW is now set on ssv, but why would we not want to do sv_buf_to_rw(ssv) in that situation?

Since around the 5.10 era, `Perl_sv_setsv_flags` has unconditionally set `SvPOK_only(dsv)` in the `SVp_POK` branch. The associated comment reads: /* Whichever path we take through the next code, we want this true, and doing it now facilitates the COW check. */ Things have changed since 5.10 though, in particular using the `‎SVf_POK` to distinguish between a value that started off as a string from one that was originally an integer/float and later stringified. This commit: * Removes the `SvPOK_only(dsv)` in favour of `SvOK_off(dsv)` and hoisting the copying of `sflags` over. * Transforms the subsequent now-redundant `SVf_POK` toggles into asserts (to help reduce) the chance of inadvertent behaviour changes.

`Perl_sv_setsv_flags` is a hot function that contains liberal sprinklings of `SvOK_off()`. This commit changes two instances, where the operand SV cannot possibly be using the OOK hack, to do direct flag twiddling instead. `SvOK_off()` does two things: 1. Toggles off some flags: SvFLAGS(sv) &= ~(SVf_OK|SVf_IVisUV|SVf_UTF8) 2. Checks for use of the OOK hack and undoes it: ((void)(SvOOK(sv) && (sv_backoff(sv),0))) At least some compilers seem to struggle to figure out when `SvOOK(sv)` cannot be true and to then elide the call to `sv_backoff()`. This is desirable when: 1. ssv & dsv are both lower types than SVt_PV and cannot support OOK 2. inside a block following a conditional check that OOK is not in use In the two cases identified, the flag toggling is now done explicitly.

Perl_sv_freshcopy_flags creates a fresh SV that contains the values of it source SV argument. It's like calling `new_SV(dsv)` followed by `sv_setsv_flags(dsv, ssv, flags`, but is optimized for a brand new destination SV and the most common code paths. The intended initial users for this new function were: * Perl_sv_mortalcopy_flags (still in sv.c) * Perl_newSVsv_flags (now a simple function in sv_inline.h) Perl_sv_freshcopy_flags handles the following cases: * SVt_NULL * SVt_IV * SVt_NV * SVt_PV * SVt_LAST For everything else, it calls S_sv_freshcopy_PVxx which handles: * SVt_INVLIST * SVt_PVIV * SVt_PVNV * SVt_PVMG - with no GET magic For everything else, there's a fall back to sv_setsv_flags. S_sv_freshcopy_POK is a dedicated helper for string swipe/COW/copy logic and is called from both Perl_sv_freshcopy_flags and S_sv_freshcopy_PVxx. With these changes compared with the previous commit: * `perl -e 'for (1..100_000_0) { my $x = { (1) x 1000 }; }'` runs about 20% faster * `perl -e 'for (1..100_000_0) { my $x = { ("Perl") x 250 }' runs about 40% faster * `perl -e 'for (1..100_000_0) { my $x = { a => 1, b => 2, c => 3, d => 4, e => 5 }; }'` is a touch faster, but within the margin for error * `perl -e 'for (1..100_000_0) { my $x = { a => "Perl", b => "Perl", c => "Perl", d => "Perl", e => "Perl" } ; }'` runs about 17% faster

Besides using the just-introduced faster path for SV copying, this allows the check for SV_GMAGIC to be pushed into the called function without having to worry about SV leaks. Two additional micro-optimizations are also in this commit: * A pointer to xav_fill is cached for use in the loop. This can be used directly to update AvFILLp(av), rather than having to get there from av's SV* each time. * The value of the loop iterator, i, is directly written into xav_fill, rather than getting the value in that slot, incrementing it (to get the same value as i), and writing it back.

richardleach added the defer-next-dev This PR should not be merged yet, but await the next development cycle label Apr 15, 2025