You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@perl.apache.org by Stas Bekman <st...@stason.org> on 2004/09/10 00:25:44 UTC
Re: cvs commit: modperl-2.0/xs/tables/current/ModPerl FunctionTable.pm
gozer@apache.org wrote:
> gozer 2004/09/09 15:16:38
> +/* XXX: There is no XS accessible splice() */
> +static void modperl_av_remove_entry(pTHX_ AV *av, I32 index)
> +{
> + I32 i;
> + AV *tmpav = newAV();
> +
> + /* stash the entries _before_ the item to delete */
> + for (i=0; i<=index; i++) {
> + av_store(tmpav, i, SvREFCNT_inc(av_shift(av)));
> + }
> +
> + /* make size at the beginning of the array */
> + av_unshift(av, index-1);
> +
> + /* add stashed entries back */
> + for (i=0; i<index; i++) {
> + av_store(av, i, *av_fetch(tmpav, i, 0));
> + }
> +
> + SvREFCNT_dec(tmpav);
shouldn't it just be sv_free'd? how do you know when the enclosing scope
will force the free'ing when you can safely free it right here.
--
__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: cvs commit: modperl-2.0/xs/tables/current/ModPerl FunctionTable.pm
Posted by "Philippe M. Chiasson" <go...@ectoplasm.org>.
Stas Bekman wrote:
> Philippe M. Chiasson wrote:
>
>>
>>Stas Bekman wrote:
>>
>>
>>>Philippe M. Chiasson wrote:
>>>
>>>
>>>>Stas Bekman wrote:
>>>>
>>>>
>>>>
>>>>>gozer@apache.org wrote:
>>>>>
>>>>>
>>>>>
>>>>>>gozer 2004/09/09 15:16:38
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>>+/* XXX: There is no XS accessible splice() */
>>>>>>+static void modperl_av_remove_entry(pTHX_ AV *av, I32 index)
>>>>>>+{
>>>>>>+ I32 i;
>>>>>>+ AV *tmpav = newAV();
>>>>>>+
>>>>>>+ /* stash the entries _before_ the item to delete */
>>>>>>+ for (i=0; i<=index; i++) {
>>>>>>+ av_store(tmpav, i, SvREFCNT_inc(av_shift(av)));
>>>>>>+ }
>>>>>>+ + /* make size at the beginning of the array */
>>>>>>+ av_unshift(av, index-1);
>>>>>>+ + /* add stashed entries back */
>>>>>>+ for (i=0; i<index; i++) {
>>>>>>+ av_store(av, i, *av_fetch(tmpav, i, 0));
>>>>>>+ }
>>>>>>+ + SvREFCNT_dec(tmpav);
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>shouldn't it just be sv_free'd? how do you know when the enclosing
>>>>>scope will force the free'ing when you can safely free it right here.
>>>>
>>>>
>>>> [...]
>>So I thought calling either sv_clear/sv_free directly wasn't recommended.
>
>
> But here you know exactly that you have nothing else referencing the sv. I
> believe that warning in sv_clear about refcnt=0 has to do with the
> possibility of other svs referencing it. If you look at the perl
> internals, all the short functions with temp svs are written in this way.
>
>
>>I don't really care either way though.
>
>
> In fact recently I first wrote this function as you did;
>
> /* prepends the passed sprintf-like arguments to ERRSV, which also
> * gets stringified on the way */
> void modperl_errsv_prepend(pTHX_ const char *pat, ...)
> {
> SV *sv;
> va_list args;
>
> va_start(args, pat);
> sv = vnewSVpvf(pat, &args);
> va_end(args);
>
> sv_catsv(sv, ERRSV);
> sv_copypv(ERRSV, sv);
> sv_free(sv); /* was: SvREFCNT_dec(sv) */
> }
>
> when I showed it to Rafael, he said why don't you use sv_free instead of
> SvREFCNT_dec.
Cool, changed accordingly.
--
--------------------------------------------------------------------------------
Philippe M. Chiasson m/gozer\@(apache|cpan|ectoplasm)\.org/ GPG KeyID : 88C3A5A5
http://gozer.ectoplasm.org/ F9BF E0C2 480E 7680 1AE5 3631 CB32 A107 88C3A5A5
Re: cvs commit: modperl-2.0/xs/tables/current/ModPerl FunctionTable.pm
Posted by Stas Bekman <st...@stason.org>.
Philippe M. Chiasson wrote:
>
>
> Stas Bekman wrote:
>
>> Philippe M. Chiasson wrote:
>>
>>>
>>> Stas Bekman wrote:
>>>
>>>
>>>> gozer@apache.org wrote:
>>>>
>>>>
>>>>> gozer 2004/09/09 15:16:38
>>>>
>>>>
>>>>
>>>>
>>>>> +/* XXX: There is no XS accessible splice() */
>>>>> +static void modperl_av_remove_entry(pTHX_ AV *av, I32 index)
>>>>> +{
>>>>> + I32 i;
>>>>> + AV *tmpav = newAV();
>>>>> +
>>>>> + /* stash the entries _before_ the item to delete */
>>>>> + for (i=0; i<=index; i++) {
>>>>> + av_store(tmpav, i, SvREFCNT_inc(av_shift(av)));
>>>>> + }
>>>>> + + /* make size at the beginning of the array */
>>>>> + av_unshift(av, index-1);
>>>>> + + /* add stashed entries back */
>>>>> + for (i=0; i<index; i++) {
>>>>> + av_store(av, i, *av_fetch(tmpav, i, 0));
>>>>> + }
>>>>> + + SvREFCNT_dec(tmpav);
>>>>
>>>>
>>>>
>>>>
>>>> shouldn't it just be sv_free'd? how do you know when the enclosing
>>>> scope will force the free'ing when you can safely free it right here.
>>>
>>>
>>>
>>> I was under the impression that
>>> SV *av = newAV();
>>> [...]
>>> SvREFCNT_dec(av);
>>>
>>> would achieve just that.
>>
>>
>>
>> Yes, but with what price:
>>
>> #if defined(__GNUC__) && !defined(__STRICT_ANSI__) &&
>> !defined(PERL_GCC_PEDANTIC)
>> # define SvREFCNT_dec(sv) \
>> ({ \
>> SV *_sv = (SV*)(sv); \
>> if (_sv) { \
>> if (SvREFCNT(_sv)) { \
>> if (--(SvREFCNT(_sv)) == 0) \
>> Perl_sv_free2(aTHX_ _sv); \
>> } else { \
>> sv_free(_sv); \
>> } \
>> } \
>> })
>> #else
>>
>>> Are you saying you'd want to see:
>>> SV *av = newAV();
>>> [...]
>>> av_undef(av);
>>
>>
>>
>> SV *av = newAV();
>> ...
>> sv_free(av).
>>
>> won't that work just as well. I guess you get not much speed
>> improvement here, but it's clear that this is a temp and you free it
>> right there.
>
>
> I read perlapi and basically got the idea that the only _clean_ way to
> destroy
> a SV * is by decreasing it's refcnt
>
> sv_free Decrement an SV’s reference count, and if it drops to zero,
> call "sv_clear" to invoke destructors and free up any memory
> used by the body; finally, deallocate the SV’s head itself.
> Normally called via a wrapper macro "SvREFCNT_dec".
>
> sv_clear
> Clear an SV: call any destructors, free up any memory
> used by
> the body, and free the body itself. The SV’s head is not
> freed,
> although its type is set to all 1’s so that it won’t
> inadver-
> tently be assumed to be live during global destruction etc.
> This function should only be called when REFCNT is zero.
> Most
> of the time you’ll want to call "sv_free()" (or its macro
> wrap-
> per "SvREFCNT_dec") instead.
>
> So I thought calling either sv_clear/sv_free directly wasn't recommended.
But here you know exactly that you have nothing else referencing the sv. I
believe that warning in sv_clear about refcnt=0 has to do with the
possibility of other svs referencing it. If you look at the perl
internals, all the short functions with temp svs are written in this way.
> I don't really care either way though.
In fact recently I first wrote this function as you did;
/* prepends the passed sprintf-like arguments to ERRSV, which also
* gets stringified on the way */
void modperl_errsv_prepend(pTHX_ const char *pat, ...)
{
SV *sv;
va_list args;
va_start(args, pat);
sv = vnewSVpvf(pat, &args);
va_end(args);
sv_catsv(sv, ERRSV);
sv_copypv(ERRSV, sv);
sv_free(sv); /* was: SvREFCNT_dec(sv) */
}
when I showed it to Rafael, he said why don't you use sv_free instead of
SvREFCNT_dec.
--
__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: cvs commit: modperl-2.0/xs/tables/current/ModPerl FunctionTable.pm
Posted by "Philippe M. Chiasson" <go...@ectoplasm.org>.
Stas Bekman wrote:
> Philippe M. Chiasson wrote:
>
>>
>>Stas Bekman wrote:
>>
>>
>>>gozer@apache.org wrote:
>>>
>>>
>>>>gozer 2004/09/09 15:16:38
>>>
>>>
>>>
>>>> +/* XXX: There is no XS accessible splice() */
>>>> +static void modperl_av_remove_entry(pTHX_ AV *av, I32 index)
>>>> +{
>>>> + I32 i;
>>>> + AV *tmpav = newAV();
>>>> +
>>>> + /* stash the entries _before_ the item to delete */
>>>> + for (i=0; i<=index; i++) {
>>>> + av_store(tmpav, i, SvREFCNT_inc(av_shift(av)));
>>>> + }
>>>> + + /* make size at the beginning of the array */
>>>> + av_unshift(av, index-1);
>>>> + + /* add stashed entries back */
>>>> + for (i=0; i<index; i++) {
>>>> + av_store(av, i, *av_fetch(tmpav, i, 0));
>>>> + }
>>>> + + SvREFCNT_dec(tmpav);
>>>
>>>
>>>
>>>shouldn't it just be sv_free'd? how do you know when the enclosing
>>>scope will force the free'ing when you can safely free it right here.
>>
>>
>>I was under the impression that
>>SV *av = newAV();
>>[...]
>>SvREFCNT_dec(av);
>>
>>would achieve just that.
>
>
> Yes, but with what price:
>
> #if defined(__GNUC__) && !defined(__STRICT_ANSI__) &&
> !defined(PERL_GCC_PEDANTIC)
> # define SvREFCNT_dec(sv) \
> ({ \
> SV *_sv = (SV*)(sv); \
> if (_sv) { \
> if (SvREFCNT(_sv)) { \
> if (--(SvREFCNT(_sv)) == 0) \
> Perl_sv_free2(aTHX_ _sv); \
> } else { \
> sv_free(_sv); \
> } \
> } \
> })
> #else
>
>>Are you saying you'd want to see:
>>SV *av = newAV();
>>[...]
>>av_undef(av);
>
>
> SV *av = newAV();
> ...
> sv_free(av).
>
> won't that work just as well. I guess you get not much speed improvement
> here, but it's clear that this is a temp and you free it right there.
I read perlapi and basically got the idea that the only _clean_ way to destroy
a SV * is by decreasing it's refcnt
sv_free Decrement an SV’s reference count, and if it drops to zero,
call "sv_clear" to invoke destructors and free up any memory
used by the body; finally, deallocate the SV’s head itself.
Normally called via a wrapper macro "SvREFCNT_dec".
sv_clear
Clear an SV: call any destructors, free up any memory used by
the body, and free the body itself. The SV’s head is not freed,
although its type is set to all 1’s so that it won’t inadver-
tently be assumed to be live during global destruction etc.
This function should only be called when REFCNT is zero. Most
of the time you’ll want to call "sv_free()" (or its macro wrap-
per "SvREFCNT_dec") instead.
So I thought calling either sv_clear/sv_free directly wasn't recommended.
I don't really care either way though.
--
--------------------------------------------------------------------------------
Philippe M. Chiasson m/gozer\@(apache|cpan|ectoplasm)\.org/ GPG KeyID : 88C3A5A5
http://gozer.ectoplasm.org/ F9BF E0C2 480E 7680 1AE5 3631 CB32 A107 88C3A5A5
Re: cvs commit: modperl-2.0/xs/tables/current/ModPerl FunctionTable.pm
Posted by Stas Bekman <st...@stason.org>.
Philippe M. Chiasson wrote:
>
>
> Stas Bekman wrote:
>
>> gozer@apache.org wrote:
>>
>>> gozer 2004/09/09 15:16:38
>>
>>
>>
>>> +/* XXX: There is no XS accessible splice() */
>>> +static void modperl_av_remove_entry(pTHX_ AV *av, I32 index)
>>> +{
>>> + I32 i;
>>> + AV *tmpav = newAV();
>>> +
>>> + /* stash the entries _before_ the item to delete */
>>> + for (i=0; i<=index; i++) {
>>> + av_store(tmpav, i, SvREFCNT_inc(av_shift(av)));
>>> + }
>>> + + /* make size at the beginning of the array */
>>> + av_unshift(av, index-1);
>>> + + /* add stashed entries back */
>>> + for (i=0; i<index; i++) {
>>> + av_store(av, i, *av_fetch(tmpav, i, 0));
>>> + }
>>> + + SvREFCNT_dec(tmpav);
>>
>>
>>
>> shouldn't it just be sv_free'd? how do you know when the enclosing
>> scope will force the free'ing when you can safely free it right here.
>
>
> I was under the impression that
> SV *av = newAV();
> [...]
> SvREFCNT_dec(av);
>
> would achieve just that.
Yes, but with what price:
#if defined(__GNUC__) && !defined(__STRICT_ANSI__) &&
!defined(PERL_GCC_PEDANTIC)
# define SvREFCNT_dec(sv) \
({ \
SV *_sv = (SV*)(sv); \
if (_sv) { \
if (SvREFCNT(_sv)) { \
if (--(SvREFCNT(_sv)) == 0) \
Perl_sv_free2(aTHX_ _sv); \
} else { \
sv_free(_sv); \
} \
} \
})
#else
> Are you saying you'd want to see:
> SV *av = newAV();
> [...]
> av_undef(av);
SV *av = newAV();
...
sv_free(av).
won't that work just as well. I guess you get not much speed improvement
here, but it's clear that this is a temp and you free it right there.
--
__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: cvs commit: modperl-2.0/xs/tables/current/ModPerl FunctionTable.pm
Posted by "Philippe M. Chiasson" <go...@ectoplasm.org>.
Stas Bekman wrote:
> gozer@apache.org wrote:
>
>>gozer 2004/09/09 15:16:38
>
>
>> +/* XXX: There is no XS accessible splice() */
>> +static void modperl_av_remove_entry(pTHX_ AV *av, I32 index)
>> +{
>> + I32 i;
>> + AV *tmpav = newAV();
>> +
>> + /* stash the entries _before_ the item to delete */
>> + for (i=0; i<=index; i++) {
>> + av_store(tmpav, i, SvREFCNT_inc(av_shift(av)));
>> + }
>> +
>> + /* make size at the beginning of the array */
>> + av_unshift(av, index-1);
>> +
>> + /* add stashed entries back */
>> + for (i=0; i<index; i++) {
>> + av_store(av, i, *av_fetch(tmpav, i, 0));
>> + }
>> +
>> + SvREFCNT_dec(tmpav);
>
>
> shouldn't it just be sv_free'd? how do you know when the enclosing scope
> will force the free'ing when you can safely free it right here.
I was under the impression that
SV *av = newAV();
[...]
SvREFCNT_dec(av);
would achieve just that.
Are you saying you'd want to see:
SV *av = newAV();
[...]
av_undef(av);
?
--
--------------------------------------------------------------------------------
Philippe M. Chiasson m/gozer\@(apache|cpan|ectoplasm)\.org/ GPG KeyID : 88C3A5A5
http://gozer.ectoplasm.org/ F9BF E0C2 480E 7680 1AE5 3631 CB32 A107 88C3A5A5