IV / NV upgrades often seem unnecessary and costly

[richardleach]

Description
Many arithmetic or comparison operators change the SV type of their operands when the operands are of different type.

For example, when adding an IV and an NV, the NV will always be upgraded:

perl -MDevel::Peek -e '$nv = 1.33; Dump($nv); $res = 2 + $nv; Dump($res); Dump($nv);'
SV = NV(0x561243b9ee90) at 0x561243b9eea8
  REFCNT = 1
  FLAGS = (NOK,pNOK)
  NV = 1.33
SV = NV(0x561243b9ef38) at 0x561243b9ef50
  REFCNT = 1
  FLAGS = (NOK,pNOK)
  NV = 3.33
SV = PVNV(0x561243b6e040) at 0x561243b9eea8
  REFCNT = 1
  FLAGS = (NOK,pIOK,pNOK)
  IV = 1
  NV = 1.33
  PV = 0

When the operands are only SvIOK(sv) and SvNOK(sv) like this, it's not clear what the benefits are of changing the SV type, rather than just fetching the actual value and converting it within the op, and not altering the SV at all. (Conversion could be done either directly in the likes of the SvIV_nomg and SvNV_nomg macros, or by calling a new inline.h function from the likes of those macros.)

Changing the SV type comes with performance costs associated with:

• allocating a different body type (if required) - this also uses additional memory and changes memory locality
• updating the type/flags in the SV head
• calling non-inline functions (e.g. S_sv_2iuv_common or Perl_sv_2iv_flags) in sv.c

The interpreter doesn't take account of how the SV is going to be used elsewhere, so:

• a cascade of potentially-unnecessary SV upgrades could occur
• the SV may repeatedly flip-flop between SvIOK(sv) and SvNOK(sv) - perhaps in different ops - triggering repeated calls to sv.c functions, updates to SV head flags, and probably still a I/UV <-> NV value conversion every time
• the SV may immediately be destroyed, so the upgrade cost were incurred for naught

In comparison, while not upgrading an IV or NV that will repeatedly be used in the same one operation (e.g. as a constant) would also cause repeated value conversion, doing this inline should (hopefully) be much cheaper, especially with modern CPUs.

Questions

1. Is there actually a very good reason - in core - for the current behaviour and it should be left as-is?
2. Would changes likely cause much/any breakage on CPAN?
3. If changes are viable, should new macros/functions be introduced or the existing ones modified?

[haarg]

The interpreter doesn't take account of how the SV is going to be used elsewhere, so:

* a cascade of potentially-unnecessary SV upgrades could occur

* the SV may repeatedly flip-flop between SvIOK(sv) and SvNOK(sv) - perhaps in different ops - triggering repeated calls to sv.c functions, updates to SV head flags, and probably still a I/UV <-> NV value conversion every time

I'm not sure where you see these happening. An SV won't flip-flop between IOK and NOK. If the value can be represented accurately as both an IV and NV, it will end up with both flags set. Or, if it can't be represented accurately, it will end up with the IOKp and NOKp flags set (as appropriate), indicating that a converted value is stored, but not fully accurate.

The general reason for this is to reduce the number of conversions needed, and I believe to reduce the number of allocations done. If you always save the result of your conversion, you never have to allocate memory for them in the future. These may not be reasonable decisions at this point.

A related issue to this is that having the type change due to read operations makes writing serialization libraries significantly harder. While you aren't supposed to care about value types within perl, that doesn't work when you need to serialize to something like JSON that does distinguish between strings and numbers. This means that even round tripping a value requires either a schema or having horrible heuristics in your encoder. If the upgrading behavior was changed to in some way always preserve the "origin type" of a value, I would be very happy. Doing less upgrades may be one way to help that.

[richardleach]

I can definitely appreciate how this makes writing serialization libraries difficult!

It definitely seems like there's potential to reduce at least some conversions to PVNVs, especially when the operands are both/all NVs to begin with!

An SV won't flip-flop

I should have provided some examples. ;) A stock Debian perl, built with PERL_PRESERVE_IVUV, was used for the following.

Forgot what I had in mind for a cascade, but in this following nonsense example with IVs and NVs, everything is a PVNV by the end:
perl -MDevel::Peek -e '$nv1 = $nv2 = 1/3; $iv1 = 2; $iv2 =88; $nv2 += $iv1 * $nv1; ($iv2 < $nv2 || $nv2 + $nv1 > $iv2); Dump($nv1); Dump($nv2); Dump($iv1); Dump($iv2);'

Sure, that's a predictable and potentially useful outcome of the individual interactions, but it definitely seems unnecessary that both @nvs and @after arrays are full of PVNVs at the end of these one-liners:
perl -MDevel::Peek -e '@nvs = (4/3, 1/3, 5/3, 2/3); @after = sort @nvs; Dump(@nvs); Dump(@after)'

perl -MDevel::Peek -e '@nvs = (4/3, 1/3, 5/3, 2/3); @after = grep { $_ > 1 } @nvs; Dump(@nvs); Dump(@after);'

(For comparison operators and pp_sort, Perl_do_ncmp (see also #17778) is where this is happening.)

The ease with which upgrades currently happen makes me wonder how often the memory savings of separate (and shadow) IV/NV bodies aren't actually achieved.

As a related aside, I'd love to try an experiment in which SV heads are made bigger to fit an extra union, used for e.g.:

• always-present IV and NV slots for scalars
• fill & max for AVs
• keys & max for SVs
• hot members from GV/CV/etc.

Obviously, that sort of change would waste memory for types (e.g. RVs) that couldn't use the extra space. However, not having to chase an extra pointer could potentially speed up a lot of common ops. It would be interesting to see the effects (even if all bad) on real applications.

For the "flip-flop" point:

1. A not wholly-contrived example:

In this loop, the first comparison with the integer causes the NV to upgrade to a PVNV with the flags (NOK,pIOK,pNOK). In the subsequent multiplication, perl has to change the flags to (NOK,pNOK), then with the subsequent comparison, change them back to (NOK,pIOK,pNOK), and so on.

perl -MDevel::Peek -e 'my $nv = 4/3; Dump($nv); while (999 > $nv) { Dump($nv); $nv *= 7/3; Dump($nv) }

Reducing the number of conversions needed is definitely logical, but the side effect here is repeated 1 bit changes to the SV head, in addition to storing both an updated lossy IV and the updated NV in the xpvnv body, despite the fact that we never intend to use the lossy IV.

2. A more contrived example:

In this loop, the NV again becomes a PVNV, and at the end of most iterations it has flags (NOK,pIOK,pNOK), but after some iterations the flags are (IOK,NOK,pIOK,pNOK).
perl -MDevel::Peek -e 'my $nv = 1/4; for (1..10) { $nv = 1 + ($_ * 1/4) + $nv; print "bazaah\n" if ($nv > 100) ;Dump($nv); }'

Admittedly, when the NV conversion isn't lossy, like here, this sort of toggling has a better chance of being beneficial.

[haarg]

That's not really flip-flopping. That's a new value. It's pretty natural for it to have a different type and different flags.

[richardleach]

Not sure if you were referring to both flip-flop examples or just one?

Certainly, in the case of:
perl -MDevel::Peek -e 'my $nv = 4/3; Dump($nv); while (999 > $nv) { Dump($nv); $nv *= 7/3; Dump($nv) }, the new value being explicitly stored (in $nv) results from multiplication of two NVs. It doesn't seem reasonable for the comparison operator to then modify $nv when, under the hood, the operator subsequently realizes that there's no lossless conversion anyway, upgrades the integer to a PVNV, and then does a floating point comparison on the two NVs.

[demerphq]

On Sat, 16 May 2020, 16:42 Richard Leach, ***@***.***> wrote: Not sure if you were referring to both flip-flop examples or just one? Certainly, in the case of: perl -MDevel::Peek -e 'my $nv = 4/3; Dump($nv); while (999 > $nv) { Dump($nv); $nv *= 7/3; Dump($nv) }, the new value being explicitly stored (in $nv) results from multiplication of two NVs. It doesn't seem reasonable for the comparison operator to then modify $nv when, under the hood, the operator subsequently realizes that there's no lossless conversion anyway, upgrades the integer to a PVNV, and then does a floating point comparison on the two NVs.

For me this feels like a bug in our constant logic. When we see a constant as argument to a comparison operator we should "upgrade it all" at compile time so the object ends up being a PVNV with the appropriate slots populated. A comparison with a constant should never modify the non constant. Yves —

…

You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#17777 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAZ5R4JQVIYIOGZWAW7KNTRR2Q5RANCNFSM4M5IBVOA> .

[richardleach]

For me this feels like a bug in our constant logic.

I see your point about how constants are represented. Note that it's not just comparison operators, the same "both-sides become PVNVs" behaviour also comes about from arithmetic operators like pp_add (e.g. 1 + $nv).

(It also happens if the IV is not a constant (e.g. $iv = 42; $nv = 4/3; ( $iv > $nv);), which may be less surprising but still seems like there's a potentially unnecessary conversion.)

As alluded to earlier, this behaviour occurs under PERL_PRESERVE_IVUV: SvIV_please_nomg(sv) leads to S_sv_2iuv_common(), which first upgrades the NV to PVNV and only afterwards checks whether a numerical upgrade is actually possible.

Comments early in S_sv_2iuv_common() suggest surprise that the function is being passed a NV and I inferred that the function was originally intended to convert PVs to IVs. (Prior to the introduction of PERL_PRESERVE_IVUV, NV ->IV/UV conversions were not that frequent?) I haven't had time for code archaeology, but was this function co-opted with the introduction of PERL_PRESERVE_IVUV and now is used for more than originally intended?

I've been experimenting with Perl_do_ncmp for #17778, but is still a work in progress.

[tonycoz]

Looking at the code involved, I think the current behaviour (in the non-constant case at least) is reasonable.

There's two parts to this:

1. pp_add (and other basic math functions) try to work in integers if possible, if we start with integers and the result is in range for an integer the math is always precise.

2. SvIV_please()/sv_2iv_common() do the upgrade early since if the value is an integer that can be cached to avoid the call to sv_2iv_common(). Note that the caching occurs even when this isn't an integer, since the pIOK flag is set and the SvIV_please() macro can still avoid the function call.

I can see some value in adding a use float that behaves like use integer that works in a similar way, ie. pp_f_add would just be SETn(left+right);.

[richardleach]

Tony, thanks for looking into the code involved. TLDR; I'm happy to close this issue.

I spent a bit of time trying to refactor Perl_do_ncmp to DWIW, but it's easy to trash performance with extra branches and, as you pointed out, use of the pIOK flag often helps keep the current code fast.

The current code was always faster (1-15%) than a decent DWIW variant, using an only-upgrade-if-you-can-do-so-losslessly helper function in sv.c, for the following kind of test case:
./perl -Ilib -e 'my $x = <some_num>; my $y=<some_other_num>; ( $x != $y ) for (1..1000000000);'
for any mix of IV/NV, NV/IV, NV/NV. (Instructions, branches, branch misses, cache loads, and cache misses were also higher with DWIW.)

I didn't test the impact of changes on numerical sorting whenever ((priv & OPpSORT_INTEGER) || all_SIVs) is not true, but imagine that the pIOK flag is a definite benefit there?

For reference, DWIW was faster (~15%, with fewer instructions, branches, branch misses, cache loads, and cache misses) for the following, where an IV<->NV comparison alternates with the NV value changing:
./perl -Ilib -e 'my $i = 1000000000; my $nv = 4/3; while ($i > $nv) { $nv += 4/3; }'

I've no idea how often the latter sort of scenario crops up though, so it doesn't make an argument for changing the current behaviour. Similarly, I don't know how often having PVNVs rather than NVs in a large application causes extra cache misses, or how frequently NVs are upgraded in an operation and go out of scope without being used again.

Happy to open new issues for the following things that have been mentioned:

• Preemptively convert constants to PVNVs to reduce modification of non-constants
• Add a "use float" pragma

[demerphq]

On Mon, 25 May 2020 at 14:41, Richard Leach ***@***.***> wrote: Tony, thanks for looking into the code involved. TLDR; I'm happy to close this issue. I spent a bit of time trying to refactor *Perl_do_ncmp* to DWIW, but it's easy to trash performance with extra branches and, as you pointed out, use of the pIOK flag often helps keep the current code fast. The current code was always faster (1-15%) than a decent DWIW variant, using an *only-upgrade-if-you-can-do-so-losslessly* helper function in sv.c, for the following kind of test case: ./perl -Ilib -e 'my $x = <some_num>; my $y=<some_other_num>; ( $x != $y ) for (1..1000000000);' for any mix of IV/NV, NV/IV, NV/NV. (Instructions, branches, branch misses, cache loads, and cache misses were also higher with DWIW.) I didn't test the impact of changes on numerical sorting whenever ((priv & OPpSORT_INTEGER) || all_SIVs) is not true, but imagine that the pIOK flag is a definite benefit there? For reference, DWIW was faster (~15%, with fewer instructions, branches, branch misses, cache loads, and cache misses) for the following, where an IV<->NV comparison alternates with the NV value changing: ./perl -Ilib -e 'my $i = 1000000000; my $nv = 4/3; while ($i > $nv) { $nv += 4/3; }' I've no idea how often the latter sort of scenario crops up though, so it doesn't make an argument for changing the current behaviour. Similarly, I don't know how often having PVNVs rather than NVs in a large application causes extra cache misses, or how frequently NVs are upgraded in an operation and go out of scope without being used again. Happy to open new issues for the following things that have been mentioned: - Preemptively convert constants to PVNVs to reduce modification of non-constants

IMO yes please. Yves

[tonycoz]

Happy to open new issues for the following things that have been mentioned:

* Preemptively convert constants to PVNVs to reduce modification of non-constants

* Add a "use float" pragma

Yes please.