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 2003/12/11 10:32:49 UTC
[mp2 filters] allocating modperl_wbucket_t or not
Currently we allocate a wbucket of type modperl_wbucket_t for each
filter invocation. If it's not a streaming filter (i.e. manipulates
bucket brigades or doesn't use read/print) it's never going to need
wbucket. wbucket consumes 16404 bytes, the filter struct without
wbucket consumes 60 bytes. In order not to penalize non-streaming
filters, I suggest not to allocate wbucket if and not until it's first
used.
So we save a lot of wasteful memory allocations if streaming API is
not used (allocating 60 bytes instead of 16464). If the streaming API
is used we pay with if(!wb) conditional for each
$f->print and $f->flush calls, since there is no special entry point
for streaming filters (a filter can be anything).
Here is the patch. Notice that you have to rebuild from scratch if you
test it, since sources under xs/ have a broken dependency on header
files under src/, therefore they won't see the change in the size of
modperl_filter_t and you will get all kind of weird segfaults (I spent
quite some time trying to understand what's going on until it hit me
and I'd rebuilt from scratch).
Index: src/modules/perl/modperl_filter.c
===================================================================
RCS file: /home/cvs/modperl-2.0/src/modules/perl/modperl_filter.c,v
retrieving revision 1.77
diff -u -r1.77 modperl_filter.c
--- src/modules/perl/modperl_filter.c 11 Dec 2003 07:38:41 -0000 1.77
+++ src/modules/perl/modperl_filter.c 11 Dec 2003 09:20:45 -0000
@@ -20,6 +20,19 @@
#define MP_FILTER_POOL(f) f->r ? f->r->pool : f->c->pool
+/* allocate wbucket memory using malloc and not request pools, since
+ * we may need many of these if the filter is invoked multiple
+ * times */
+#define WBUCKET_INIT(wb, p, next_filter) \
+ if (!wb) { \
+ wb = (modperl_wbucket_t *)safemalloc(sizeof(*wb)); \
+ wb->pool = p; \
+ wb->filters = &next_filter; \
+ wb->outcnt = 0; \
+ wb->r = NULL; \
+ wb->header_parse = 0; \
+ }
+
/* this function is for tracing only, it's not optimized for performance */
static int is_modperl_filter(ap_filter_t *f)
{
@@ -264,9 +277,7 @@
filter->mode = mode;
filter->f = f;
filter->pool = p;
- filter->wbucket.pool = p;
- filter->wbucket.filters = &f->next;
- filter->wbucket.outcnt = 0;
+ filter->wbucket = NULL;
if (mode == MP_INPUT_FILTER_MODE) {
filter->bb_in = NULL;
@@ -739,7 +750,8 @@
filter->flush = 0;
}
- filter->rc = modperl_wbucket_flush(&filter->wbucket, add_flush_bucket);
+ WBUCKET_INIT(filter->wbucket, filter->pool, filter->f->next);
+ filter->rc = modperl_wbucket_flush(filter->wbucket, add_flush_bucket);
if (filter->rc != APR_SUCCESS) {
return filter->rc;
}
@@ -779,7 +791,8 @@
const char *buf,
apr_size_t *len)
{
- return modperl_wbucket_write(aTHX_ &filter->wbucket, buf, len);
+ WBUCKET_INIT(filter->wbucket, filter->pool, filter->f->next);
+ return modperl_wbucket_write(aTHX_ filter->wbucket, buf, len);
}
apr_status_t modperl_output_filter_handler(ap_filter_t *f,
Index: src/modules/perl/modperl_types.h
===================================================================
RCS file: /home/cvs/modperl-2.0/src/modules/perl/modperl_types.h,v
retrieving revision 1.69
diff -u -r1.69 modperl_types.h
--- src/modules/perl/modperl_types.h 19 Aug 2003 05:01:22 -0000 1.69
+++ src/modules/perl/modperl_types.h 11 Dec 2003 09:20:46 -0000
@@ -193,7 +193,7 @@
ap_filter_t *f;
char *leftover;
apr_ssize_t remaining;
- modperl_wbucket_t wbucket;
+ modperl_wbucket_t *wbucket;
apr_bucket *bucket;
apr_bucket_brigade *bb_in;
apr_bucket_brigade *bb_out;
__________________________________________________________________
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: [mp2 filters] allocating modperl_wbucket_t or not
Posted by Geoffrey Young <ge...@modperlcookbook.org>.
>> you're still way ahead of the game here, so go with what you think is
>> best
>> (thanks for asking, though :)
>
>
> Are you talking about premature optimization, or are you just implying
> that you hadn't caught up with filters implementation yet?
definitely the latter - I'm still very far behind in my understanding of all
the low-C-level filter stuff. that's what I meant :)
>
>> if you have the time and can explain what a wbucket is, that would be
>> great,
>> too :)
>
>
> modperl_wbucket_t is a struct that mod_perl uses to pass data to from
> response handler to the first filter, or from any mod_perl filter to the
> next filter. e.g. it's used every time ($r|$f)->(print()|flush()) and
> some other variants are called.
> It includes a buffer of fixed size and a few other records. It's very
> efficient in the response handlers, since it's allocated only once for
> each request.
>
> Only filters that use streaming API and call $f->print() need this buffer.
excellent, thanks!
--Geoff
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [mp2 filters] allocating modperl_wbucket_t or not
Posted by Stas Bekman <st...@stason.org>.
Philippe M. Chiasson wrote:
> On Thu, 2003-12-11 at 11:35, Stas Bekman wrote:
>
>>Stas Bekman wrote:
>>
>>>Geoffrey Young wrote:
>>>
>>>
>>>>Stas Bekman wrote:
>>>>
>>>>
>>>>>Currently we allocate a wbucket of type modperl_wbucket_t for each
>>>>>filter invocation. If it's not a streaming filter (i.e. manipulates
>>>>>bucket brigades or doesn't use read/print) it's never going to need
>>>>>wbucket. wbucket consumes 16404 bytes, the filter struct without
>>>>>wbucket consumes 60 bytes. In order not to penalize non-streaming
>>>>>filters, I suggest not to allocate wbucket if and not until it's first
>>>>>used.
>>>
>>>
>>>I'm getting too comphy with pools and this patch of course forgets to
>>>free memory, now that it has moved to malloc.
>>
>>Here is a proper patch, which now frees the memory for wbucket as well.
>
>
> Looks like a safe and good idea to me : stas++
Thanks folks, now committed.
__________________________________________________________________
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: [mp2 filters] allocating modperl_wbucket_t or not
Posted by "Philippe M. Chiasson" <go...@cpan.org>.
On Thu, 2003-12-11 at 11:35, Stas Bekman wrote:
> Stas Bekman wrote:
> > Geoffrey Young wrote:
> >
> >>
> >> Stas Bekman wrote:
> >>
> >>> Currently we allocate a wbucket of type modperl_wbucket_t for each
> >>> filter invocation. If it's not a streaming filter (i.e. manipulates
> >>> bucket brigades or doesn't use read/print) it's never going to need
> >>> wbucket. wbucket consumes 16404 bytes, the filter struct without
> >>> wbucket consumes 60 bytes. In order not to penalize non-streaming
> >>> filters, I suggest not to allocate wbucket if and not until it's first
> >>> used.
> >
> >
> > I'm getting too comphy with pools and this patch of course forgets to
> > free memory, now that it has moved to malloc.
>
> Here is a proper patch, which now frees the memory for wbucket as well.
Looks like a safe and good idea to me : stas++
> Index: src/modules/perl/modperl_filter.c
> ===================================================================
> RCS file: /home/cvs/modperl-2.0/src/modules/perl/modperl_filter.c,v
> retrieving revision 1.77
> diff -u -r1.77 modperl_filter.c
> --- src/modules/perl/modperl_filter.c 11 Dec 2003 07:38:41 -0000 1.77
> +++ src/modules/perl/modperl_filter.c 11 Dec 2003 19:26:57 -0000
> @@ -20,6 +20,25 @@
>
> [...]
--
--------------------------------------------------------------------------------
Philippe M. Chiasson /gozer\@(cpan|ectoplasm)\.org/ 88C3A5A5 (122FF51B/C634E37B)
http://gozer.ectoplasm.org/ F9BF E0C2 480E 7680 1AE5 3631 CB32 A107 88C3 A5A5
Q: It is impossible to make anything foolproof because fools are so ingenious.
perl -e'$$=\${gozer};{$_=unpack(P7,pack(L,$$));/^JAm_pH\n$/&&print||$$++&&redo}'
Re: [mp2 filters] allocating modperl_wbucket_t or not
Posted by Stas Bekman <st...@stason.org>.
Stas Bekman wrote:
> Geoffrey Young wrote:
>
>>
>> Stas Bekman wrote:
>>
>>> Currently we allocate a wbucket of type modperl_wbucket_t for each
>>> filter invocation. If it's not a streaming filter (i.e. manipulates
>>> bucket brigades or doesn't use read/print) it's never going to need
>>> wbucket. wbucket consumes 16404 bytes, the filter struct without
>>> wbucket consumes 60 bytes. In order not to penalize non-streaming
>>> filters, I suggest not to allocate wbucket if and not until it's first
>>> used.
>
>
> I'm getting too comphy with pools and this patch of course forgets to
> free memory, now that it has moved to malloc.
Here is a proper patch, which now frees the memory for wbucket as well.
Index: src/modules/perl/modperl_filter.c
===================================================================
RCS file: /home/cvs/modperl-2.0/src/modules/perl/modperl_filter.c,v
retrieving revision 1.77
diff -u -r1.77 modperl_filter.c
--- src/modules/perl/modperl_filter.c 11 Dec 2003 07:38:41 -0000 1.77
+++ src/modules/perl/modperl_filter.c 11 Dec 2003 19:26:57 -0000
@@ -20,6 +20,25 @@
#define MP_FILTER_POOL(f) f->r ? f->r->pool : f->c->pool
+/* allocate wbucket memory using malloc and not request pools, since
+ * we may need many of these if the filter is invoked multiple
+ * times */
+#define WBUCKET_INIT(wb, p, next_filter) \
+ if (!wb) { \
+ wb = (modperl_wbucket_t *)safemalloc(sizeof(*wb)); \
+ wb->pool = p; \
+ wb->filters = &next_filter; \
+ wb->outcnt = 0; \
+ wb->r = NULL; \
+ wb->header_parse = 0; \
+ }
+
+#define FILTER_FREE(filter) \
+ if (filter->wbucket) { \
+ safefree(filter->wbucket); \
+ } \
+ safefree(filter);
+
/* this function is for tracing only, it's not optimized for performance */
static int is_modperl_filter(ap_filter_t *f)
{
@@ -264,9 +283,7 @@
filter->mode = mode;
filter->f = f;
filter->pool = p;
- filter->wbucket.pool = p;
- filter->wbucket.filters = &f->next;
- filter->wbucket.outcnt = 0;
+ filter->wbucket = NULL;
if (mode == MP_INPUT_FILTER_MODE) {
filter->bb_in = NULL;
@@ -393,7 +410,7 @@
status = modperl_errsv(aTHX_ status, r, s);
}
- safefree(filter);
+ FILTER_FREE(filter);
SvREFCNT_dec((SV*)args);
MP_INTERP_PUTBACK(interp);
@@ -739,7 +756,8 @@
filter->flush = 0;
}
- filter->rc = modperl_wbucket_flush(&filter->wbucket, add_flush_bucket);
+ WBUCKET_INIT(filter->wbucket, filter->pool, filter->f->next);
+ filter->rc = modperl_wbucket_flush(filter->wbucket, add_flush_bucket);
if (filter->rc != APR_SUCCESS) {
return filter->rc;
}
@@ -779,7 +797,8 @@
const char *buf,
apr_size_t *len)
{
- return modperl_wbucket_write(aTHX_ &filter->wbucket, buf, len);
+ WBUCKET_INIT(filter->wbucket, filter->pool, filter->f->next);
+ return modperl_wbucket_write(aTHX_ filter->wbucket, buf, len);
}
apr_status_t modperl_output_filter_handler(ap_filter_t *f,
@@ -800,7 +819,7 @@
filter = modperl_filter_new(f, bb, MP_OUTPUT_FILTER_MODE,
0, 0, 0);
status = modperl_run_filter(filter);
- safefree(filter);
+ FILTER_FREE(filter);
}
switch (status) {
@@ -834,7 +853,7 @@
filter = modperl_filter_new(f, bb, MP_INPUT_FILTER_MODE,
input_mode, block, readbytes);
status = modperl_run_filter(filter);
- safefree(filter);
+ FILTER_FREE(filter);
}
switch (status) {
Index: src/modules/perl/modperl_types.h
===================================================================
RCS file: /home/cvs/modperl-2.0/src/modules/perl/modperl_types.h,v
retrieving revision 1.69
diff -u -r1.69 modperl_types.h
--- src/modules/perl/modperl_types.h 19 Aug 2003 05:01:22 -0000 1.69
+++ src/modules/perl/modperl_types.h 11 Dec 2003 19:26:57 -0000
@@ -193,7 +193,7 @@
ap_filter_t *f;
char *leftover;
apr_ssize_t remaining;
- modperl_wbucket_t wbucket;
+ modperl_wbucket_t *wbucket;
apr_bucket *bucket;
apr_bucket_brigade *bb_in;
apr_bucket_brigade *bb_out;
__________________________________________________________________
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: [mp2 filters] allocating modperl_wbucket_t or not
Posted by Stas Bekman <st...@stason.org>.
Geoffrey Young wrote:
>
> Stas Bekman wrote:
>
>>Currently we allocate a wbucket of type modperl_wbucket_t for each
>>filter invocation. If it's not a streaming filter (i.e. manipulates
>>bucket brigades or doesn't use read/print) it's never going to need
>>wbucket. wbucket consumes 16404 bytes, the filter struct without
>>wbucket consumes 60 bytes. In order not to penalize non-streaming
>>filters, I suggest not to allocate wbucket if and not until it's first
>>used.
I'm getting too comphy with pools and this patch of course forgets to free
memory, now that it has moved to malloc.
> you're still way ahead of the game here, so go with what you think is best
> (thanks for asking, though :)
Are you talking about premature optimization, or are you just implying that
you hadn't caught up with filters implementation yet?
> if you have the time and can explain what a wbucket is, that would be great,
> too :)
modperl_wbucket_t is a struct that mod_perl uses to pass data to from response
handler to the first filter, or from any mod_perl filter to the next filter.
e.g. it's used every time ($r|$f)->(print()|flush()) and some other variants
are called.
It includes a buffer of fixed size and a few other records. It's very
efficient in the response handlers, since it's allocated only once for each
request.
Only filters that use streaming API and call $f->print() need this buffer.
__________________________________________________________________
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: [mp2 filters] allocating modperl_wbucket_t or not
Posted by Geoffrey Young <ge...@modperlcookbook.org>.
Stas Bekman wrote:
> Currently we allocate a wbucket of type modperl_wbucket_t for each
> filter invocation. If it's not a streaming filter (i.e. manipulates
> bucket brigades or doesn't use read/print) it's never going to need
> wbucket. wbucket consumes 16404 bytes, the filter struct without
> wbucket consumes 60 bytes. In order not to penalize non-streaming
> filters, I suggest not to allocate wbucket if and not until it's first
> used.
you're still way ahead of the game here, so go with what you think is best
(thanks for asking, though :)
if you have the time and can explain what a wbucket is, that would be great,
too :)
--Geoff
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org