You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@perl.apache.org by Geoffrey Young <ge...@modperlcookbook.org> on 2004/01/16 21:07:05 UTC
apr_brigade_flatten
hi all
joe schaefer suggested on modperl@ that having access to apr_brigade_flatten
might be of use. at least in the context of Apache::Request::Upload you
could use
my $upload_data = $req->upload('file')->bb->flatten;
to essentially slurp up the file without iterating the brigade manually.
I'm not sure about how this would fit into filters, since I guess there are
multiple brigades, but I'm not sure.
anyway, any arguments against opening it up? (with tests, of course :)
--Geoff
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: apr_brigade_flatten
Posted by Geoffrey Young <ge...@modperlcookbook.org>.
>> yeah, I wasn't quite sure what to do here. I didn't like the idea of
>> length() returning an $rv and populating a $length argument, since it
>> seemed
>> more useful to simply have it return the length. but I'm deviating
>> from the
>> API here, so I dunno.
>
>
> at least mark it with XXX, so we know it's an untied end. We will how to
> deal with it better as this API will be used.
done
>> hmm, the length passed ends up being allocated? I was just trying to
>> simulate a user that used a big number since the size was unknown (and
>> who
>> didn't know about length())
>
>
> ok then. I'd add at least a comment that one should pass either a small
> known size (if they want to get at most X bytes) or call the length
> method. some people use tests as examples, while we don't have the
> normal docs up.
>
>>> I don't think flatten is effective, as it has to go through all the
>>> bucket twice - once because you need to figure out its length, second
>>> time to slurp data.
ok, I added some comments in the test
> I think your current patch (plus a few comments) is fine to go in. Can
> do pflatten on the next iteration.
committed. I changed the test name to flatten.pm, though, since it seemed
better than the generic brigade.pm.
--Geoff
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: apr_brigade_flatten
Posted by Stas Bekman <st...@stason.org>.
Geoffrey Young wrote:
>>>+ if (rv == APR_SUCCESS) {
>>>+ return newSViv((int)length);
>>>+ }
>>>+
>>>+ return &PL_sv_undef;
>>>+}
>>
>>
>>won't it be better to die if it fails and stick rv into $@? e.g. if you
>>try to read a bucket which has no data? Just thinking aloud here, we
>>have many places where don't quite handle error codes, but I tend to at
>>least stick XXX in them and hopefully figure it out when we get first
>>failures.
>
>
> yeah, I wasn't quite sure what to do here. I didn't like the idea of
> length() returning an $rv and populating a $length argument, since it seemed
> more useful to simply have it return the length. but I'm deviating from the
> API here, so I dunno.
at least mark it with XXX, so we know it's an untied end. We will how to deal
with it better as this API will be used.
> as for sticking the rv in $@, I don't see that being useful, since it's just
> an APR_ status code.
But you can convert the code into the error message. We attempt to do that in
several places. Very inconsistently though.
grep for modperl_apr_strerror and modperl_strerr
>>In the test script you end up allocating memory for 300k instead of
>>200k, just to throw 100k of it right away. a big waste.
>
>
> hmm, the length passed ends up being allocated? I was just trying to
> simulate a user that used a big number since the size was unknown (and who
> didn't know about length())
ok then. I'd add at least a comment that one should pass either a small known
size (if they want to get at most X bytes) or call the length method. some
people use tests as examples, while we don't have the normal docs up.
>>I don't think flatten is effective, as it has to go through all the
>>bucket twice - once because you need to figure out its length, second
>>time to slurp data.
>
>
> I was getting that feeling as I implemented it - the dependency on the
> length made it seem less than optimal for a "slurp" mode.
pflatten is more like 'slurp' and more suitable for Joe. though we already
agreed that he doesn't need the *flatten perl glue, and better handle that on
the C level with a slurp() wrapper as a perl glue.
>>More effecient
>>and simpler for the end user (though requires the dreaded pool object)
>
>
> yeah, that's the problem - requiring a pool is turning this all into less of
> a shortcut. but just because it takes more effort doesn't mean we shouldn't
> open them both up anyway.
it doesn't make it less a shortcut, since you usually do have a ready pool
object. It's just the use of pool that makes me nervous. Obviously copying the
string to protect from using the wrong pool is not an option here.
> so, I'll do them both and whip up a patch for the Apache::Request docs that
> use pflatten() instead.
I think your current patch (plus a few comments) is fine to go in. Can do
pflatten on the next iteration.
If Joe is going to implement slurp() I don't think A::R docs need to mention
any flatten at all. Just wait a bit and the need will disappear ;)
__________________________________________________________________
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: apr_brigade_flatten
Posted by Geoffrey Young <ge...@modperlcookbook.org>.
>> + if (rv == APR_SUCCESS) {
>> + return newSViv((int)length);
>> + }
>> +
>> + return &PL_sv_undef;
>> +}
>
>
> won't it be better to die if it fails and stick rv into $@? e.g. if you
> try to read a bucket which has no data? Just thinking aloud here, we
> have many places where don't quite handle error codes, but I tend to at
> least stick XXX in them and hopefully figure it out when we get first
> failures.
yeah, I wasn't quite sure what to do here. I didn't like the idea of
length() returning an $rv and populating a $length argument, since it seemed
more useful to simply have it return the length. but I'm deviating from the
API here, so I dunno.
as for sticking the rv in $@, I don't see that being useful, since it's just
an APR_ status code.
> In the test script you end up allocating memory for 300k instead of
> 200k, just to throw 100k of it right away. a big waste.
hmm, the length passed ends up being allocated? I was just trying to
simulate a user that used a big number since the size was unknown (and who
didn't know about length())
>
> I don't think flatten is effective, as it has to go through all the
> bucket twice - once because you need to figure out its length, second
> time to slurp data.
I was getting that feeling as I implemented it - the dependency on the
length made it seem less than optimal for a "slurp" mode.
>
> pflatten doesn't require that. It does all the work for you. You really
> want to use flatten only when you want to flatten only not more than X
> bytes, I think.
yeah, that's how it's used, I noticed.
>
> with pflatten you do:
>
> char *buffer;
> apr_size_t bufsiz = HUGE_STRING_LEN;
> rc = apr_brigade_pflatten(bb, &buffer, &bufsiz, pool);
>
> buffer is probably PVX(sv_buf), after you forced it into PV.
> and after that set the pv's length slot to bufsize.
> and then just stash that buffer into a new SV or pass.
yup, pretty much like the way I did it before (which was stolen from
APR::Socket:recv)
> More effecient
> and simpler for the end user (though requires the dreaded pool object)
yeah, that's the problem - requiring a pool is turning this all into less of
a shortcut. but just because it takes more effort doesn't mean we shouldn't
open them both up anyway.
so, I'll do them both and whip up a patch for the Apache::Request docs that
use pflatten() instead.
--Geoff
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: apr_brigade_flatten
Posted by Stas Bekman <st...@stason.org>.
Geoffrey Young wrote:
[...]
> Index: xs/APR/Brigade/APR__Brigade.h
> ===================================================================
> RCS file: /home/cvspublic/modperl-2.0/xs/APR/Brigade/APR__Brigade.h,v
> retrieving revision 1.4
> diff -u -r1.4 APR__Brigade.h
> --- xs/APR/Brigade/APR__Brigade.h 29 Mar 2002 16:16:43 -0000 1.4
> +++ xs/APR/Brigade/APR__Brigade.h 20 Jan 2004 18:54:10 -0000
> @@ -67,3 +67,35 @@
> return APR_BRIGADE_EMPTY(brigade);
> }
>
> +static MP_INLINE
> +SV *mpxs_APR__Brigade_length(pTHX_ apr_bucket_brigade *bb,
> + int read_all)
> +{
> + apr_off_t length;
> +
> + apr_status_t rv = apr_brigade_length(bb, read_all, &length);
> +
> + if (rv == APR_SUCCESS) {
> + return newSViv((int)length);
> + }
> +
> + return &PL_sv_undef;
> +}
won't it be better to die if it fails and stick rv into $@? e.g. if you try to
read a bucket which has no data? Just thinking aloud here, we have many places
where don't quite handle error codes, but I tend to at least stick XXX in them
and hopefully figure it out when we get first failures.
> +static MP_INLINE
> +apr_status_t mpxs_apr_brigade_flatten(pTHX_ apr_bucket_brigade *bb,
> + SV *sv_buf, SV *sv_len)
> +{
> + apr_status_t status;
> + apr_size_t len = mp_xs_sv2_apr_size_t(sv_len);
> +
> + mpxs_sv_grow(sv_buf, len);
> + status = apr_brigade_flatten(bb, SvPVX(sv_buf), &len);
> + mpxs_sv_cur_set(sv_buf, len);
> +
> + if (!SvREADONLY(sv_len)) {
> + sv_setiv(sv_len, len);
> + }
> +
> + return status;
> +}
In the test script you end up allocating memory for 300k instead of 200k, just
to throw 100k of it right away. a big waste.
I don't think flatten is effective, as it has to go through all the bucket
twice - once because you need to figure out its length, second time to slurp data.
pflatten doesn't require that. It does all the work for you. You really want
to use flatten only when you want to flatten only not more than X bytes, I think.
with pflatten you do:
char *buffer;
apr_size_t bufsiz = HUGE_STRING_LEN;
rc = apr_brigade_pflatten(bb, &buffer, &bufsiz, pool);
buffer is probably PVX(sv_buf), after you forced it into PV.
and after that set the pv's length slot to bufsize.
and then just stash that buffer into a new SV or pass. More effecient and
simpler for the end user (though requires the dreaded pool object)
__________________________________________________________________
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: apr_brigade_flatten
Posted by Geoffrey Young <ge...@modperlcookbook.org>.
Stas Bekman wrote:
> Geoffrey Young wrote:
>
>> hi all
>>
>> joe schaefer suggested on modperl@ that having access to
>> apr_brigade_flatten
>> might be of use.
> I certainly have no objections to opening up the entire APR API if
> someone needs it. And most of the APR::(Brigade|Bucket) API is needed in
> filters.
ok, here's a first pass. I opened up apr_brigade_length as well, since
apr_brigade_flatten takes a length parameter.
oh, and joe had asked:
> (hmm, does APR::Brigade have a DESTROY method?).
yes, apr_brigade_destroy is currently mapped as $bb->destroy, and is called
in a few of the filter tests.
--Geoff
Re: apr_brigade_flatten
Posted by Stas Bekman <st...@stason.org>.
Geoffrey Young wrote:
> hi all
>
> joe schaefer suggested on modperl@ that having access to apr_brigade_flatten
> might be of use. at least in the context of Apache::Request::Upload you
> could use
>
> my $upload_data = $req->upload('file')->bb->flatten;
>
> to essentially slurp up the file without iterating the brigade manually.
>
> I'm not sure about how this would fit into filters, since I guess there are
> multiple brigades, but I'm not sure.
>
> anyway, any arguments against opening it up? (with tests, of course :)
I certainly have no objections to opening up the entire APR API if someone
needs it. And most of the APR::(Brigade|Bucket) API is needed in filters.
I've already replied to Joe, that I think apreq2 shouldn't expose the bucket
brigade interface or at least have a simple alternative method which will do
all the work behind the scenes (which also removes the immediate need for
flatten, since there is the C API for that).
__________________________________________________________________
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