perl-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Geoffrey Young <ge...@modperlcookbook.org>
Subject Re: DECLINE_CMD
Date Fri, 09 May 2003 12:44:04 GMT

> Hmm, I haven't realized that it's used in _module.c as well. So my idea 
> introduces a bit of an overhead to allocate the SV, when it only wants 
> an int.
> But since this happens only during the server startup, I suppose it's 
> not a problem.

that was my thought.

[snip]
> 
> A few more comments below:
> 
>> Index: lib/ModPerl/Code.pm
> 
> [...]
> 
>>  $proto
>>  {
>> +    dTHX;
> 
> 
> It's going to be much more efficient to pass pTHX_ to the function.

ok, I'll try that.  I really have no idea how threads work, I just figured 
out that I needed something to get it to compile :)

> 
>> +          if (strEQ(name, "$name")) {
>> +              return newSVpv($alias{$name},0);
> 
> 
> Use newSVpvn, you know the length already (faster).

well, then I need two if statements - one for DECLINE_CMD and one for 
DIR_MAGIC_TYPE.  I'm not sure that the added logic is worth the (one time 
and slight) hit.

> 
>> +          }
>> +$ifdef[1]
>> +EOF
>> +            }
>> +            else {
>> +                print $c_fh <<EOF;
>>  $ifdef[0]
>>            if (strEQ(name, "$name")) {
>> -              return $alias{$name};
>> +              return newSViv($alias{$name});
> 
> 
> Use strnEQ (faster), like the orig int functions do. 

the original functions use strEQ - I didn't change that for the int ones and 
only carried it over for the string ones.

> You don't really 
> have to make special cases for these two constants. Just use the old 
> generation code and put 'return SVpvn($alias{$name});' if perl sees 
> these two constants, otherwise put 'return SViv($alias{$name});'

yeah :)

as per SViv vs newSViv, if we remove the newSV stuff then is it still save 
to remove the newSV from val here:

 > I think it should be:
 >
 > -        newCONSTSUB(stash, (char *)name, newSViv(val));
 > +        newCONSTSUB(stash, (char *)name, val);

?

> 
>> -typedef int (*constants_lookup)(const char *);
>> +typedef SV *(*constants_lookup)(const char *);
> 
> 
> So pTHX_ goes here:
> 
> -typedef int (*constants_lookup)(const char *);
> +typedef SV *(*constants_lookup)(pTHX_ const char *);
> 
> and aTHX_ in the caller...
> 
>> +                    SvIV(modperl_constants_lookup_apache(SvPV(val, 
>> len)));
>>              }
>>          }
>>
>> @@ -695,7 +695,7 @@
>>              }
>>              else {
>>                  cmd->req_override =
>> -                    modperl_constants_lookup_apache(SvPV(val, len));
>> +                    SvIV(modperl_constants_lookup_apache(SvPV(val, 
>> len)));
> 
> 
> it's probably safe to use SvIVX here, but since this code is not 
> run-time critical SvIV is just fine.

cool, thanks for the comments.  I'll have (another) patch soon.

--Geoff


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Mime
View raw message