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

Reimar Döffinger Reimar.Doeffinger at gmx.de
Tue Nov 10 12:03:16 CET 2009


On Tue, Nov 10, 2009 at 07:21:31AM +0100, Tobias Diedrich wrote:
> With the patch, the first prefetch wants the whole 1MB block, so
> this gets merged nicely before going to sshfs and cause the
> high-speed peak at the beginning.
> After that fill_buffer requests additional 2KB at a time which
> effectively translates to one Page at a time in the kernel.
> 
> Since posix_fadvise has no way to give a deadline, the kernel
> assumes 'as soon as possible' and passes the 4KB request down to
> sshfs, which has no mechanism for request merging apparently, thus
> the slowdown. (I assume you would not see this on a LAN, since you
> would not have the big 300ms rtt)

Ok, I'd propose to merge the current patch with the first one.
Though I'd suggest two configurable defines:
PREFETCH_DISTANCE
(which determines when we will call the next fadvise, namely when we
have prefetched less than this ahead by the previous calls) and
PREFETCH_MIN_SIZE
which determines how much will be prefetched in one batch at least
(I'd suggest to prefetch at least max_size if it is more, though possibly rounded
up to 4kB).
However don't rename stream_priv_s, this is named like this because it
is indeed supposed to be stored in stream->priv.
So add whatever you need there, and change in open
>  m_struct_free(&stream_opts,opts);
to
> stream->priv = opts;
and add a close function that does
> m_struct_free(&stream_opts, stream->priv);
(see also stream_ftp.c if something about this is unclear).

What do you think? Seems nicer and more flexible than your increased
buffer size patch to me.



More information about the MPlayer-dev-eng mailing list