You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by Cliff Woolley <cl...@yahoo.com> on 2001/07/05 03:41:49 UTC

Re: cvs commit: apr-util/buckets apr_buckets_file.c

On 5 Jul 2001 rbb@apache.org wrote:

>   We need to ALWAYS do the seek if we are reading from the file.  This is
>   unfortunate from a performance perspective, but right now, I can have an
>   offset of 0 in the bucket, but be referring to a file that has been read
>   from.  If we don't seek before reading from the bucket, we get invalid
>   data.
>
>        /* Handle offset ... */
>   -    if (fileoffset) {
>            rv = apr_file_seek(f, APR_SET, &fileoffset);
>            if (rv != APR_SUCCESS) {
>                free(buf);
>                return rv;
>   -        }
>        }

Hoooo... good catch.  I don't know why I never thought of that before.
<sigh>  You're right that it sucks for performance, but then again if we
get to this point we've given up on mmaping the file so we're in the
"worst case" performance scenario anyway.

--Cliff


--------------------------------------------------------------
   Cliff Woolley
   cliffwoolley@yahoo.com
   Charlottesville, VA



Re: cvs commit: apr-util/buckets apr_buckets_file.c

Posted by Cliff Woolley <cl...@yahoo.com>.
On Wed, 4 Jul 2001 rbb@covalent.net wrote:

> > Hoooo... good catch.  I don't know why I never thought of that before.
> > <sigh>  You're right that it sucks for performance, but then again if we
> > get to this point we've given up on mmaping the file so we're in the
> > "worst case" performance scenario anyway.
>
> I only caught it because it exposed a bug in mod_pop3.  :-)

I'd imagine that we've never noticed it before because you'd only see it
if you took a file bucket, split and/or copied it, and read from a LATER
part of the file BEFORE reading from the beginning of the file, which
isn't common in current Apache modules.  Maybe we should come up with a
mod_scramble/mod_unscramble to test for weird stuff like this.  =-)

--Cliff


--------------------------------------------------------------
   Cliff Woolley
   cliffwoolley@yahoo.com
   Charlottesville, VA



Re: cvs commit: apr-util/buckets apr_buckets_file.c

Posted by rb...@covalent.net.
> >   We need to ALWAYS do the seek if we are reading from the file.  This is
> >   unfortunate from a performance perspective, but right now, I can have an
> >   offset of 0 in the bucket, but be referring to a file that has been read
> >   from.  If we don't seek before reading from the bucket, we get invalid
> >   data.
> >
> >        /* Handle offset ... */
> >   -    if (fileoffset) {
> >            rv = apr_file_seek(f, APR_SET, &fileoffset);
> >            if (rv != APR_SUCCESS) {
> >                free(buf);
> >                return rv;
> >   -        }
> >        }
>
> Hoooo... good catch.  I don't know why I never thought of that before.
> <sigh>  You're right that it sucks for performance, but then again if we
> get to this point we've given up on mmaping the file so we're in the
> "worst case" performance scenario anyway.

I only caught it because it exposed a bug in mod_pop3.  :-)

Ryan

_____________________________________________________________________________
Ryan Bloom                        	rbb@apache.org
Covalent Technologies			rbb@covalent.net
-----------------------------------------------------------------------------