[MPlayer-dev-eng] [PATCH] Use posix_fadvise in stream_file if available

[Tobias Diedrich]

Reimar Döffinger wrote:
> On Sun, Nov 08, 2009 at 02:30:56PM +0100, Tobias Diedrich wrote:
> >  static int fill_buffer(stream_t *s, char* buffer, int max_len){
> >    int r = read(s->fd,buffer,max_len);
> > +#ifdef POSIX_FADV_WILLNEED
> > +  posix_fadvise(s->fd, s->pos + r, PREFETCH_LEN, POSIX_FADV_WILLNEED);
> > +#endif
> >    return (r <= 0) ? -1 : r;
> 
> Then you have to check for r <= 0 before the fadvise (I think that looks nicer anyway).
Well, it's not strictly necessary, it would only mean one useless
prefetch (if at all, if the read returns an error most likely
posix_fadvise will also return an error)
Hmm on second thought it would overwrite the errno, but this should
work without needing an additional check:

static int fill_buffer(stream_t *s, char* buffer, int max_len){
  int r;
#ifdef POSIX_FADV_WILLNEED
  posix_fadvise(s->fd, s->pos + max_len, PREFETCH_LEN,
POSIX_FADV_WILLNEED);
#endif
  r = read(s->fd,buffer,max_len);
  return (r <= 0) ? -1 : r;
}

It should be ok to use max_len since read should only return a short
read if it's either not a file or we are at the end of the file, in
both cases the posix_fadvise won't matter.
As the manpage says: "The advice applies to a (not necessarily
existent) region", so it's ok if the region goes beyond the end of
the file.

BTW, is there any system where read will return a value different
from -1 on error?
If not, the extra check on return is not necessary at all and this
would work just as well:

static int fill_buffer(stream_t *s, char* buffer, int max_len){
#ifdef POSIX_FADV_WILLNEED
  posix_fadvise(s->fd, s->pos + max_len, PREFETCH_LEN,
POSIX_FADV_WILLNEED);
#endif
  return read(s->fd,buffer,max_len);
}

-- 
Tobias						PGP: http://8ef7ddba.uguu.de