sox-devel@lists.sourceforge.net unofficial mirror
 help / color / mirror / code / Atom feed
* [PATCH] use posix_fadvise to increase readahead
@ 2015-09-07  1:10 Eric Wong
  2020-08-12  6:28 ` Eric Wong
  0 siblings, 1 reply; 5+ messages in thread
From: Eric Wong @ 2015-09-07  1:10 UTC (permalink / raw)
  To: sox-devel

All relevant audio file formats store data sequentially, so
give a hint to the kernel to perform more readahead.  In current
Linux, the readahead hint doubles readahead pages and can help
with playback on slow devices.
---
  If you prefer "git pull" instead of "git am":

  The following changes since commit 7e74b254b2a7c963be0bfce751fc5911fe681c12:

    Remove hepler script. It's mostly unmaintained, I don't know if anyone but me ever used it. In any case, those who want a custom Debian package should be capable of updating the debian/changelog entry on their own. (2015-02-26 22:48:40 -0500)

  are available in the git repository at:

    git://bogomips.org/sox fadvise

  for you to fetch changes up to 1c47376a04218838447e7bd49fb42156e9cb13b6:

    use posix_fadvise to increase readahead (2015-09-07 00:39:24 +0000)

  ----------------------------------------------------------------
  Eric Wong (1):
        use posix_fadvise to increase readahead

   configure.ac  |  1 +
   src/formats.c | 18 ++++++++++++++++--
   src/sox_i.h   |  4 ++++
   3 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/configure.ac b/configure.ac
index 23138a9..d9ae1ba 100644
--- a/configure.ac
+++ b/configure.ac
@@ -208,6 +208,7 @@ AC_CHECK_HEADERS(fcntl.h unistd.h byteswap.h sys/stat.h sys/time.h sys/timeb.h s
 
 dnl Checks for library functions.
 AC_CHECK_FUNCS(strcasecmp strdup popen vsnprintf gettimeofday mkstemp fmemopen)
+AC_CHECK_FUNCS(posix_fadvise)
 
 dnl Check if math library is needed.
 AC_SEARCH_LIBS([pow], [m])
diff --git a/src/formats.c b/src/formats.c
index 724a4cd..340b867 100644
--- a/src/formats.c
+++ b/src/formats.c
@@ -329,12 +329,26 @@ static void set_endiannesses(sox_format_t * ft)
 static sox_bool is_seekable(sox_format_t const * ft)
 {
   struct stat st;
+  int fd, seekable;
 
   assert(ft);
   if (!ft->fp)
     return sox_false;
-  fstat(fileno((FILE*)ft->fp), &st);
-  return ((st.st_mode & S_IFMT) == S_IFREG);
+  fd = fileno((FILE*)ft->fp);
+  if (fd < 0)
+     return 0;
+  fstat(fd, &st);
+  seekable = ((st.st_mode & S_IFMT) == S_IFREG);
+#if defined HAVE_POSIX_FADVISE
+  if (seekable) {
+    /*
+     * POSIX_FADV_NOREUSE can potentially be beneficial, too,
+     * but is a no-op as of Linux 4.2.  Not sure about other kernels.
+     */
+    (void)posix_fadvise(fd, (off_t)0, st.st_size, POSIX_FADV_SEQUENTIAL);
+  }
+#endif
+  return seekable;
 }
 
 /* check that all settings have been given */
diff --git a/src/sox_i.h b/src/sox_i.h
index 04bbe8c..2eec9d3 100644
--- a/src/sox_i.h
+++ b/src/sox_i.h
@@ -20,6 +20,10 @@
 #define _GNU_SOURCE
 #endif
 
+#if defined HAVE_POSIX_FADVISE
+#define _XOPEN_SOURCE 600
+#endif
+
 #include <errno.h>
 #include <stdio.h>
 #include <stdlib.h>
-- 
EW


------------------------------------------------------------------------------

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] use posix_fadvise to increase readahead
  2015-09-07  1:10 [PATCH] use posix_fadvise to increase readahead Eric Wong
@ 2020-08-12  6:28 ` Eric Wong
  2020-08-12  9:39   ` Måns Rullgård
  0 siblings, 1 reply; 5+ messages in thread
From: Eric Wong @ 2020-08-12  6:28 UTC (permalink / raw)
  To: Måns Rullgård; +Cc: sox-devel

Eric Wong <normalperson@yhbt.net> wrote:
> All relevant audio file formats store data sequentially, so
> give a hint to the kernel to perform more readahead.  In current
> Linux, the readahead hint doubles readahead pages and can help
> with playback on slow devices.

Btw, I've been running this for a few years, too; but pretty
much all on FLAC.

I don't know if there's audio formats we support which aren't
sequential.  Maybe there's some wacky audio format which
requires random read all over the place...

> ---
>   If you prefer "git pull" instead of "git am":
> 
>   The following changes since commit 7e74b254b2a7c963be0bfce751fc5911fe681c12:
> 
>     Remove hepler script. It's mostly unmaintained, I don't know if anyone but me ever used it. In any case, those who want a custom Debian package should be capable of updating the debian/changelog entry on their own. (2015-02-26 22:48:40 -0500)
> 
>   are available in the git repository at:
> 
>     git://bogomips.org/sox fadvise

Nowadays moved to:

     https://yhbt.net/sox.git ew/fadvise

The commit SHA-1 remains unchanged

>   for you to fetch changes up to 1c47376a04218838447e7bd49fb42156e9cb13b6:
> 
>     use posix_fadvise to increase readahead (2015-09-07 00:39:24 +0000)
> 
>   ----------------------------------------------------------------
>   Eric Wong (1):
>         use posix_fadvise to increase readahead
> 
>    configure.ac  |  1 +
>    src/formats.c | 18 ++++++++++++++++--
>    src/sox_i.h   |  4 ++++
>    3 files changed, 21 insertions(+), 2 deletions(-)
> 
> diff --git a/configure.ac b/configure.ac
> index 23138a9..d9ae1ba 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -208,6 +208,7 @@ AC_CHECK_HEADERS(fcntl.h unistd.h byteswap.h sys/stat.h sys/time.h sys/timeb.h s
>  
>  dnl Checks for library functions.
>  AC_CHECK_FUNCS(strcasecmp strdup popen vsnprintf gettimeofday mkstemp fmemopen)
> +AC_CHECK_FUNCS(posix_fadvise)
>  
>  dnl Check if math library is needed.
>  AC_SEARCH_LIBS([pow], [m])
> diff --git a/src/formats.c b/src/formats.c
> index 724a4cd..340b867 100644
> --- a/src/formats.c
> +++ b/src/formats.c
> @@ -329,12 +329,26 @@ static void set_endiannesses(sox_format_t * ft)
>  static sox_bool is_seekable(sox_format_t const * ft)
>  {
>    struct stat st;
> +  int fd, seekable;
>  
>    assert(ft);
>    if (!ft->fp)
>      return sox_false;
> -  fstat(fileno((FILE*)ft->fp), &st);
> -  return ((st.st_mode & S_IFMT) == S_IFREG);
> +  fd = fileno((FILE*)ft->fp);
> +  if (fd < 0)
> +     return 0;
> +  fstat(fd, &st);
> +  seekable = ((st.st_mode & S_IFMT) == S_IFREG);
> +#if defined HAVE_POSIX_FADVISE
> +  if (seekable) {
> +    /*
> +     * POSIX_FADV_NOREUSE can potentially be beneficial, too,
> +     * but is a no-op as of Linux 4.2.  Not sure about other kernels.

Still true as of Linux 5.8 :<

> +     */
> +    (void)posix_fadvise(fd, (off_t)0, st.st_size, POSIX_FADV_SEQUENTIAL);

Also, Linux has always ignored offset and size for
POSIX_FADV_SEQUENTIAL, so this won't do anything strange when
using the trim-as-first-effect to force seeking.

> +  }
> +#endif
> +  return seekable;
>  }
>  
>  /* check that all settings have been given */
> diff --git a/src/sox_i.h b/src/sox_i.h
> index 04bbe8c..2eec9d3 100644
> --- a/src/sox_i.h
> +++ b/src/sox_i.h
> @@ -20,6 +20,10 @@
>  #define _GNU_SOURCE
>  #endif
>  
> +#if defined HAVE_POSIX_FADVISE
> +#define _XOPEN_SOURCE 600
> +#endif
> +

I think _DEFAULT_SOURCE is needed, too, nowadays.

>  #include <errno.h>
>  #include <stdio.h>
>  #include <stdlib.h>


_______________________________________________
SoX-devel mailing list
SoX-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/sox-devel

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] use posix_fadvise to increase readahead
  2020-08-12  6:28 ` Eric Wong
@ 2020-08-12  9:39   ` Måns Rullgård
  2020-08-13  3:53     ` Eric Wong
  0 siblings, 1 reply; 5+ messages in thread
From: Måns Rullgård @ 2020-08-12  9:39 UTC (permalink / raw)
  To: Eric Wong; +Cc: sox-devel

Eric Wong <normalperson@yhbt.net> writes:

> Eric Wong <normalperson@yhbt.net> wrote:
>> All relevant audio file formats store data sequentially, so
>> give a hint to the kernel to perform more readahead.  In current
>> Linux, the readahead hint doubles readahead pages and can help
>> with playback on slow devices.
>
> Btw, I've been running this for a few years, too; but pretty
> much all on FLAC.
>
> I don't know if there's audio formats we support which aren't
> sequential.  Maybe there's some wacky audio format which
> requires random read all over the place...

I can't think of any format that isn't mostly sequential, certainly not
that's supported in SoX.  There might exist some format that separates
channel data such that reading sequentially from multiple starting
points is the best strategy.

What sort of improvement do you get from this anyway?  I'm not opposed
to the addition, just curious.

-- 
Måns Rullgård


_______________________________________________
SoX-devel mailing list
SoX-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/sox-devel

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] use posix_fadvise to increase readahead
  2020-08-12  9:39   ` Måns Rullgård
@ 2020-08-13  3:53     ` Eric Wong
  2020-08-13 11:32       ` Måns Rullgård
  0 siblings, 1 reply; 5+ messages in thread
From: Eric Wong @ 2020-08-13  3:53 UTC (permalink / raw)
  To: Måns Rullgård; +Cc: sox-devel

Måns Rullgård <mans@mansr.com> wrote:
> Eric Wong <normalperson@yhbt.net> writes:
> 
> > Eric Wong <normalperson@yhbt.net> wrote:
> >> All relevant audio file formats store data sequentially, so
> >> give a hint to the kernel to perform more readahead.  In current
> >> Linux, the readahead hint doubles readahead pages and can help
> >> with playback on slow devices.
> >
> > Btw, I've been running this for a few years, too; but pretty
> > much all on FLAC.
> >
> > I don't know if there's audio formats we support which aren't
> > sequential.  Maybe there's some wacky audio format which
> > requires random read all over the place...
> 
> I can't think of any format that isn't mostly sequential, certainly not
> that's supported in SoX.  There might exist some format that separates
> channel data such that reading sequentially from multiple starting
> points is the best strategy.

Yeah, I was wondering about something along those lines,
especially if exceeding 2 channels...

> What sort of improvement do you get from this anyway?  I'm not opposed
> to the addition, just curious.

I didn't measure before :x  It's more of a "it couldn't hurt"
thing and let the kernel (and configured per-device readahead)
decide.  It's only intended as a hint for the kernel, after all.

My 7200 RPM HDD has 256 MB cache on it; I couldn't measure any
difference even with 'echo 3 >/proc/sys/vm/drop_caches' (Linux)
and reading ~300 MB of stuff before testing in hopes it'd clear
the internal HDD cache.  Maybe slower mounts (optical storage,
4200 RPM HDDs, or network FSes) will see the benefit.

I used to play audio on a laptop using an sshfs mount back in
the day (with FUSE readahead, too), but haven't had the need or
ability to do that for years, now.


_______________________________________________
SoX-devel mailing list
SoX-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/sox-devel

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] use posix_fadvise to increase readahead
  2020-08-13  3:53     ` Eric Wong
@ 2020-08-13 11:32       ` Måns Rullgård
  0 siblings, 0 replies; 5+ messages in thread
From: Måns Rullgård @ 2020-08-13 11:32 UTC (permalink / raw)
  To: Eric Wong; +Cc: sox-devel

Eric Wong <normalperson@yhbt.net> writes:

> Måns Rullgård <mans@mansr.com> wrote:
>> Eric Wong <normalperson@yhbt.net> writes:
>> 
>> > Eric Wong <normalperson@yhbt.net> wrote:
>> >> All relevant audio file formats store data sequentially, so
>> >> give a hint to the kernel to perform more readahead.  In current
>> >> Linux, the readahead hint doubles readahead pages and can help
>> >> with playback on slow devices.
>> >
>> > Btw, I've been running this for a few years, too; but pretty
>> > much all on FLAC.
>> >
>> > I don't know if there's audio formats we support which aren't
>> > sequential.  Maybe there's some wacky audio format which
>> > requires random read all over the place...
>> 
>> I can't think of any format that isn't mostly sequential, certainly not
>> that's supported in SoX.  There might exist some format that separates
>> channel data such that reading sequentially from multiple starting
>> points is the best strategy.
>
> Yeah, I was wondering about something along those lines,
> especially if exceeding 2 channels...

I know of formats that interleave channel data in blocks of a few
kilobytes, but those don't require seeking.  Some MOV/MP4 video files
store the video and audio completely separately, but we don't care about
those.

>> What sort of improvement do you get from this anyway?  I'm not opposed
>> to the addition, just curious.
>
> I didn't measure before :x  It's more of a "it couldn't hurt"
> thing and let the kernel (and configured per-device readahead)
> decide.  It's only intended as a hint for the kernel, after all.
>
> My 7200 RPM HDD has 256 MB cache on it; I couldn't measure any
> difference even with 'echo 3 >/proc/sys/vm/drop_caches' (Linux)
> and reading ~300 MB of stuff before testing in hopes it'd clear
> the internal HDD cache.  Maybe slower mounts (optical storage,
> 4200 RPM HDDs, or network FSes) will see the benefit.
>
> I used to play audio on a laptop using an sshfs mount back in
> the day (with FUSE readahead, too), but haven't had the need or
> ability to do that for years, now.

Hmm, I prefer not to add code like this on a hunch.  Maybe I'll hook up
some horrible old laptop drive and see if it makes a difference there.
Then again, does anyone care?

-- 
Måns Rullgård


_______________________________________________
SoX-devel mailing list
SoX-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/sox-devel

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2020-08-13 11:32 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-09-07  1:10 [PATCH] use posix_fadvise to increase readahead Eric Wong
2020-08-12  6:28 ` Eric Wong
2020-08-12  9:39   ` Måns Rullgård
2020-08-13  3:53     ` Eric Wong
2020-08-13 11:32       ` Måns Rullgård

Code repositories for project(s) associated with this inbox:

	https://80x24.org/mirrors/sox.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).