mPUSHn()/mPUSHi()/mPUSHu() convert from slow sv_set() to newSV()+2mor…

[bulk88]

…tal()

commit d82b684 - Apr 30 2004 - Steve Hay
Document limitations in PUSHi et al macros and add new mPUSHi et al macros

which is day 1 of these macros, introduced this not ideal pattern doing "sv_setiv(PUSHmortal,)". sv_set*() funcs are much more complex/more work at runtime, since they must undo everything prior in the SV* and maybe upgrade the body and copy over fields from the prev "body[less]". newSV*() funcs dont have to do that. sv_2mortal() is very small and light weight internally, so rmving sv_newmortal() then adding sv_2mortal() isn't a problem, but for full disclosure, this commit makes better macros mPUSHn()/mPUSHi()/mPUSHu() than before, but this commit isn't perfect. sv_2mortal() is public api, and while it has exactly 1 conditional fn call in it (stack grower), it is a little bit heavier and does more work than a perfection hand written solution (see PUSH_EXTEND_MORTAL__SV_C). sv_2mortal() contains 4 branches, PUSH_EXTEND_MORTAL__SV_C has 1 branch.

Another benefit of this commit is XS code that looks like if(r=c_syscall()) PUSHs(sv_2mortal(newSViv(r)));
is better than
if(r=c_syscall()) sv = newSV(); PUSHs(sv); sv_setiv(r); b/c var r, will effortlessly flow from the volatile retval register to a volatile outgoing "C stk" register (xfering control to newSViv()) and doesn't need to be save/restored from a non-vol storage location to preserve var r around a extern linked C fn.

The 2004 commit above also has a same title medium sized P5P thread. This 2004 patch doesn't have a spotless history as a patch, and it went through multiple revisions b/c of typos and mistakes at the time, until it was decided to publish it. That commit probably did the "sv_setiv(PUSHmortal,)" pattern b/c of copy pasting from the existing TARG macros. And the commit is an improvement/fix for flaws or unfriendlyness that TARG macros had.

too lazy to edit Devel::PPPort, but everyone at P5P neglects that area of core

---

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

[bulk88]

bump, randomly ran into this annoyance again fixing a random cpan xs mod, Id rather see this centrally fixed in blead perl headers than me overriding stock perls by hand as a CPAN XS patch author

And for next Christmas, for Santa to add this bug fix to Devel::PPPort, back porting the bug fix to all Perl VM binaries in the world.

[khwilliamson]

I object to pushing this p.r. until the commit message reads sensibly.

[Leont]

I object to pushing this p.r. until the commit message reads sensibly.

Yeah that is a recurring challenge. Let's try to find a path out of this situation.

Your commit message reads as a stream-of-thought. In my experience, writing any good documentation starts with the asking questions that the reader is expected to have. For the body of a commit message that could be for example

1. Why is it necessary? It may fix a bug, it may add a feature, it may improve performance, reliability, stability, or just be a change for the sake of correctness.
2. How does it address the issue? For short obvious patches this part can be omitted, but it should be a high level description of what the approach was.
3. What effects does the patch have? (In addition to the obvious ones, this may include benchmarks, side effects, etc.)

Something like this perhaps:

Convert mPUSH* from sv_set* to newSV*

Previously these would create an undefined mortal value and then upgrade it to the right type, it's faster to create an SV of the appropriate type and then mortalize it because sv_upgrade is relatively slow.