On Tue, May 03, 2016 at 07:40:22PM +0100, Nicholas Clark wrote: > On Tue, May 03, 2016 at 04:56:31PM +0100, Dave Mitchell wrote: > > Rik, > > I hadn't realised until recently that the Coro breakage caused by > > consting the MGF vtable array in commit v5.21.7-98-gc910fea, is still > > unresolved. > > > > I propose that we revert this commit for 5.24 (we need another RC anyway > > for the SvGROW) issue. > > What does this gain? > > Coro still won't compile against 5.24.x without other changes (due to your > context stack reworking), and still won't compile against 5.22.x (because this > wouldn't be backported). > > > This should have relatively minimal effect (apart from 4K extra memory > > usage per process), since all it would be doing is removing a constraint > > on XS code (the vtable array can now be modified by XS code again). > > So as best I can tell, all this achieves is a 4K increase in memory usage > per process. It doesn't fix Coro. What's the upside? Apparently it is hard to make Coro work without the vtable being modifiable. The vtable consting, although desirable, is a relatively trivial gain for perl, and its loss is relatively small. It just means that an extra 4K from a 2Mb executable can be shared across processes when 2 or more perls are running on the same system. And allows us to stop XS code from doing weird stuff with some of our internals. So it's a partial fix for Coro. I have no idea what use Coro makes of the context stack, or whether its doing something subtle which is fundamentally unfixable under the recent context changes in blead; but it may happen that its easily fixable within Coro. In that case, that makes it theoretically possible for a Coro to be released that runs on perl 5.24.0 and later. Without the const revert, we may remove this possibility altogether. I had a very quick look at the Coro src; the diff below allows it to compile under blead (with the revert), and it then passes 18 of its 20 test scripts. I have no idea whether the two failing scripts represent a "nearly there" state or a "fundamentally unfixable" state. diff --git a/Coro-6.49/Coro/State.xs b/Coro-6.49/Coro/State.xs index 6d631bd..c2cbe7b 100644 --- a/Coro-6.49/Coro/State.xs +++ b/Coro-6.49/Coro/State.xs @@ -1395,7 +1395,18 @@ runops_trace (pTHX) PUSHMARK (SP); PUSHs (&PL_sv_yes); PUSHs (fullname); - PUSHs (CxHASARGS (cx) ? sv_2mortal (newRV_inc ((SV *)cx->blk_sub.argarray)) : &PL_sv_undef); + { + AV *aa; +# if PERL_VERSION_ATLEAST(5,23,8) + aa = ((AV*)(AvARRAY(MUTABLE_AV( + PadlistARRAY(CvPADLIST(cx->blk_sub.cv))[ + CvDEPTH(cx->blk_sub.cv)]))[0])); +#else + aa = cx->blk_sub.argarray; +#endif + + PUSHs (CxHASARGS (cx) ? sv_2mortal (newRV_inc ((SV *)aa)) : &PL_sv_undef); + } PUTBACK; cb = hv_fetch ((HV *)SvRV (coro_current), "_trace_sub_cb", sizeof ("_trace_sub_cb") - 1, 0); if (cb) call_sv (*cb, G_KEEPERR | G_EVAL | G_VOID | G_DISCARD); -- Indomitable in retreat, invincible in advance, insufferable in victory -- Churchill on MontgomeryThread Previous | Thread Next