develooper Front page | perl.perl5.porters | Postings from January 2016

Re: reworked context stack system

Thread Previous | Thread Next
Dave Mitchell
January 6, 2016 10:28
Re: reworked context stack system
Message ID:
On Mon, Jan 04, 2016 at 07:25:20PM +0100, Vincent Pit (VPIT) wrote:
> Le 04/01/2016 17:50, Dave Mitchell a écrit :
> >The just-pushed branch smoke-me/davem/contextsB4 contains about 200 commits
> >that heavily rework the scope / context stack system (all that PUSHBLOCK,
> >POPSUB etc stuff). If it smokes ok, and if there aren't any objections,
> >I intend to merge it in a few days' time.
> Great work, thanks.
> However, I am afraid this might break much more than just what is "likely"
> according to In particular, if I understand your description
> correctly, this :

[ snip 3 of my paragraphs ]

> has a potential of breaking a lot of things, since destruction timing might
> change.

Addressing each each of the three things in turn:

> >In dounwind(), the current savestack frame is processed before each context
> >is popped for every context type; formerly this was only done for sub-like
> >context frames. This action has been removed from POPSUB/cx_popsub and
> >placed into its own macro, CX_LEAVE_SCOPE(cx), which must be called before
> >cx_popsub etc.

All this does is make the destruction order for dounwind() match the order
of a series of individual LEAVEFOO ops, so for example in

    eval { ... { .... { ...; { die if COND } ... } ... } .... }

if the die doesn't trigger, the scopes are unwound by pp_leaveloop,
pp_leaveloop and pp_leaveloop; if it does, then by dounwind(). The two
different exit paths will now restore and free vars etc in exactly the
same order. If that makes any user-visible differences, it should be
regarded as a bug fix.

Note that for 5.20.0, FC changed it from dounwind() not calling
leave_scope() at all (so that all savestack frames were processed *after*
all contexts had been popped) to dowunwind calling leave_scope() every
time it popped a sub context. This causes not problems as far as I'm
aware. I have merely extended it so that dounwind() now calls
leave_scope() (if there's anything on the savestack) for every context it
pops, rather than just for sub one.

> >dounwind() Now also does a cx_popblock() on the last popped frame (formerly
> >it only did the cx_popsub() etc actions on each frame).

This is another measure that makes dounwind() now behave exactly like a
serries of pp_leavefoo()s. It ensures that all the scope vars like
PL_curpm are restored at the exit from dounwind, rather than leaving them
"dangling" at the innermost scope. In practise this won't make any
difference most of the time, since most callers of dounwind() usually
proceed to  immediately follow up with cx_popeval(), cx_popblock() or
whatever, so the vars get restored shortly afterwards. But it is logically
more correct.

> >The temps stack is now freed on scope exit; previously, temps created
> >during the last statement of a block wouldn't be freed until the next
> >nextstate following the block (apart from an existing hack that did this
> >for recursive subs in scalar context); and in something like f(g()),
> >the temps created by the last statement in g() would formerly not be
> >freed until the statement following the return from f().

This is explicitly a bug fix:

    [perl #124248] temporary objects created in returning statements are
                   DESTROYed too late

In general I would be very surprised if this branch broke any perl-level
code, unless that code was already relying on buggy behaviour which this
branch fixes.

> Concerning Scope::Upper, it might be impossible to fix after your changes.

I've spent the last day looking at Scope::Upper. I think its current
implementation can't be made to work, because my branch makes it so that
popping one context won't necessarily call leave_scope(), unless the code
associated with that scope happens to so something that triggers something
going on the save stack (like doing 'my' or 'local'). Formerly every
context scope entry type did a SAVETMPS(), which guaranteed at least one
savestack entry, and so triggering a leave_scope().

A possible new implementation might be to insert a custom op after every
pp_enterfoo (or at least for those pp_enterfoo's that are in the path of
a 'localise' etc), that at a minimum SAVE() a dummy entry to force a
leave_scope() at every level, or possibly push a destructor that can be
used to handle the actions (thus avoiding the need to hack the chain of
savestack_ix's like su_init() does at the moment). This is based on my
understanding of the code that su_init() and su_pop() are at the heart of
the injection mechanism.

> I
> don't really care since I don't use it personally and the performance gains
> are nice, but I won't hide that it would make me feel like having written it
> for the community (it was a request) was a complete waste of my time.

Well, it relies completely on undocumented internal implementation details
of the perl context stack system; in particular, it assumes that every
context stack push will also trigger at least one scopestack push and at
least one savestack push, and that when its SAVEDESTRUCTOR_X() is called,
that the leave_scope() which invokes it was called by LEAVE() as opposed
to any other caller type. All three of those change in my branch.

In England there is a special word which means the last sunshine
of the summer. That word is "spring".

Thread Previous | Thread Next Perl Programming lists via nntp and http.
Comments to Ask Bjørn Hansen at | Group listing | About