Front page | perl.perl5.porters |
Postings from January 2014
Re: leak in const_av_xsub?
Thread Previous
|
Thread Next
From:
bulk88
Date:
January 6, 2014 02:06
Subject:
Re: leak in const_av_xsub?
Message ID:
BLU0-SMTP11181449EE1CC7B9A0F2420DFB70@phx.gbl
Father Chrysostomos wrote:
> Daniel Dragan wrote:
>> In const_av_xsub at
>> http://perl5.git.perl.org/perl.git/blob/db9cb8483f3d2ac7f4fe573c8623e33721ea6139:op.c#l12347
>> I see " ST(0) = newSViv((IV)AvFILLp(av)+1);". No sv_2mortal near by,
>> and we aren't using TARG. I'm not sure this if this is leaking. Its some
>> pretty new code by Father Chrysostomos
>> http://perl5.git.perl.org/perl.git/commit/6f1b3ab07ea077b8191fe508269d778bc64a5f50
>> added in 5.19.3.
>
> Thank you for finding that. That is indeed a leak.
>
> ./miniperl -Ilib -le 'warn $$; use constant foo => 1..3; while(1){ scalar &foo }'
>
>> I'm in the process of optimizing const_av_xsub ATM.
>
> I will wait before fixing it, then. Unless you want to
> include it in your refactoring.
>
I also discovered that G_SCALAR path
http://perl5.git.perl.org/perl.git/blob/c3970b8037ca5769191bce298ba0202ce318fc78:/op.c#l12347
AKA " ST(0) = newSViv((IV)AvFILLp(av)+1);", is never executed during
a make test (put in function call breakpoint, DebugBreak, never ran).
Your "-Ilib -le 'warn $$; use constant foo => 1..3; while(1){ scalar
&foo }'" does trigger the G_SCALAR break point. What I have so far, and
it doesn't assert.
----------------------------------------------------------
@@ -12331,27 +12331,62 @@ static void
const_av_xsub(pTHX_ CV* cv)
{
dVAR;
dXSARGS;
AV * const av = MUTABLE_AV(XSANY.any_ptr);
- SP -= items;
+ U32 gimme_v;
+ assert((SP - items) == MARK);
+ SP = MARK; /* remove all incoming args, more efficient than items */
assert(av);
#ifndef DEBUGGING
if (!av) {
- XSRETURN(0);
+ goto end;
+// XSRETURN(0);
}
#endif
if (SvRMAGICAL(av))
Perl_croak(aTHX_ "Magical list constants are not supported");
- if (GIMME_V != G_ARRAY) {
- EXTEND(SP, 1);
- ST(0) = newSViv((IV)AvFILLp(av)+1);
- XSRETURN(1);
- }
- EXTEND(SP, AvFILLp(av)+1);
- Copy(AvARRAY(av), &ST(0), AvFILLp(av)+1, SV *);
- XSRETURN(AvFILLp(av)+1);
+// if (GIMME_V != G_ARRAY) {
+// EXTEND(SP, 1);
+// ST(0) = newSViv((IV)AvFILLp(av)+1);
+// XSRETURN(1);
+// }
+// EXTEND(SP, AvFILLp(av)+1);
+// Copy(AvARRAY(av), &ST(0), AvFILLp(av)+1, SV *);
+// XSRETURN(AvFILLp(av)+1);
+ gimme_v = GIMME_V;
+ if(gimme_v != G_VOID) {
+ I32 stack_grow = 1;
+ I32 avlastindex = AvFILLp(av); /* 0 based */
+ I32 avitems; /* 1 based */
+ stack_grow += -(gimme_v == G_ARRAY) & avlastindex;
+ if(gimme_v == G_ARRAY) assert(stack_grow == AvFILLp(av)+1);
+ EXTEND(SP, stack_grow);
+ SP += 1;
+ avitems = avlastindex+1;
+ if(gimme_v != G_ARRAY) {
+ dXSTARG;
+ SETs(TARG);
+ sv_setiv(TARG, (IV)avitems);
+ assert(ST(0) == TARG);
+ DebugBreak();
+ }
+ else {
+ SV ** dstsp = SP;
+ assert(gimme_v == G_ARRAY);
+ //fprintf(stderr, "%u %u\n", &ST(0), dstsp);
+ assert(&ST(0) == dstsp);
+ assert((AvFILLp(av)+1) == avlastindex+1);
+ SP += avlastindex;
+ //fprintf(stderr, "%u %u\n", SP, (PL_stack_base + ax +
((AvFILLp(av)+1) - 1)));
+ assert(SP == (PL_stack_base + ax + ((AvFILLp(av)+1) - 1)));
+ Copy(AvARRAY(av), dstsp, avitems, SV *);
+ }
+ }
+ end:
+ PUTBACK;
+ return;
}
/*
* Local variables:
* c-indentation-style: bsd
----------------------------------------------------------------
Thread Previous
|
Thread Next