Joe Schaefer wrote:
Stas Bekman <[EMAIL PROTECTED]> writes:

[...]


Thanks for the drawing. Now I don't understand how do you get to the
apreq_value_t pointer from table_entry->val. Do you just subtract the
size of the apreq_value_t struct and based on the knowledge that the
memory allocation is continues, you magically retrieve the whole struct?


Essentially, yes. That's one of the things the offsetof macro is good for. For specifics, see how apreq_attr_to_type is defined in apreq.h.


I think I understand the trick now. it wasn't obvious at all ;)


Yeah, it took me a month to get used to it.  You seem to have figured it
out in far less time :-).

Thanks Joe ;) I had to ask Eric Cholet about the data[1] trick, which ensures a continuous memory. I didn't know that. and the C FAQ entry 2.6 is far from being clear about that.


and since newSVpv copies the string, you no longer can get to the
beginning of the struct. Now I understand why you wanted to mark it as
SvFAKE (that would prevent the copy).

Now that I understand this thing I can think intelligently ;) For
example how about making SV into a dual-var, ala $!. it might not work
if IV slot is used to store the original pointer, since using val in
the numeric context will need to make use of this field. Same with NV?
I'm just thinking how can we save the overhead of messing with MAGIC
if we can.


Yeah, me too. The patch just adds an sv_magic() call whenever newSVpv is called, which shouldn't be too costly (no magic lookups are caused
by APR::Tables, just the overhead of adding magic to any returned scalars).

OK, my take on this is as follows: I don't think we should change the current implementation in mod_perl because it's very non-generic. If tomorrow someone wants to use APR::Table to do something else, they may want to change the core implementation too, how do we say 'no' in that case?


I think it's OK to duplicate the implementation and change it the way APREQ needs. Sort of sub-classing APR::Table. However if you think of a way to rewrite the current implementation to make it easier to sub-class XS please suggest how. Though it should happen without adding an extra overhead. APR::Table is a very busy API, used by many mod_perl functions, so if we can avoid adding any overhead at all, it's a good thing for an overall performance.

What do you think?

It's not that I don't want to change it, I'm just trying to keep the number of mod_perl calls (memory usage) to bare minimum for the maximum performance.

So you say that due to the bug we can't sub-class get() to do newSVpv
while using the pointer to apreq_value_t and then setting SvCUR to the
real string?


The bug I was referring to is tickled by the numeric tests (4,6,8,14) in
modperl/env.t when I used this definition of mpxs_apr_table_str_2sv:

MP_INLINE static SV *mpxs_apr_table_str_2sv(pTHX_ const char *str)
{
    SV *sv;
    if (str == NULL)
        return &PL_sv_undef;

    sv = newSV(0);
    SvUPGRADE(sv, SVt_PV);
    SvCUR(sv) = strlen(str);
    SvPVX(sv) = (char *)str;
    SvPOK_on(sv);
    /* LEAVE SvLEN == 0 */
    return sv_2mortal(sv);
}

IMO, this would be (close to the) ideal approach, because a few
nice things happen behind the scenes:

SvLEN = 0 prevents perl from attempting to call Safefree()
on the SvPVX during sv_free, which would cause a segfault
since that memory was malloced from an apr_pool. SvLEN = 0 also
prevents perl from "stealing" the SvPVX during assignment, (an
optimization in sv_setsv). SvLEN = 0 causes assignment to induce a string copy, which is _perfectly fine_ since the apreq subclass'
action can happen right on the return stack, *before* any perl assignment is made.


The real problem with SvLEN=0 is sv_2iv, which would normally upgrade the SVt_PV to a numeric type, balks at having SvLEN = 0. sv_2iv winds up converting *any* such string to having a numeric 0
IV. That breaks the numeric tests in modperl/env.t, at least with perl 5.8.0.

:(


Another idea I had is to append the 'str' pointer after \0. I guess that won't survive copying and may introduce a leakage.



__________________________________________________________________
Stas Bekman            JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/     mod_perl Guide ---> http://perl.apache.org
mailto:[EMAIL PROTECTED] http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org   http://ticketmaster.com


--------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]



Reply via email to