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

[Uoti Urpala]

On Sat, 2009-11-07 at 20:37 +0100, Reimar Döffinger wrote:
> On Sat, Nov 07, 2009 at 09:22:38PM +0200, Uoti Urpala wrote:
> > On Sat, 2009-11-07 at 20:06 +0100, Reimar Döffinger wrote:
> > > Also I don't think this is supposed to be s->pos, like this you'll
> > > advise the kernel to read ahead the data it just read while almost
> > > certainly it would make more sense to tell the kernel to drop that part.
> > 
> > > posix_fadvise(s->fd, pos, max_len, POSIX_FADV_DONTNEED);
> > 
> > I think that's a bad idea. At least on Linux POSIX_FADV_DONTNEED seems
> > to make the kernel always drop the data from the cache instantly, even
> > if there is no memory pressure. So this would require filesystem reads
> > for every seek. It'd also completely prevent the file from being in
> > cache after playback, and even drop it from the cache if it was already
> > there.
> > 
> > If the behavior of DONTNEED was only "schedule this to be dropped from
> > cache first in case of memory pressure" then this could be beneficial.
> > But currently it isn't.
> 
> If it doesn't cause any serious issues I don't think implementation
> idiocies in the current Linux kernel are a good reason to make things
> harder for (possibly future) better implementations.

It does more harm than good at the moment. I think future improvements
should not be counted on if there's no idea when or if they would
happen.

> Though probably adding POSIX_FADV_NOREUSE is the better solution,
> for a "perfect" implementation it will have the same effect with less
> code and for the Linux kernel it will (AFAIK) have no effect.

Maybe better, though if it's not actually beneficial either on current
platforms then it's not certain whether it'll become beneficial or
harmful like DONTNEED first.