2010/2/27 Pádraig Brady <[email protected]> > Well as long as the reads are big enough, > read-process-read-process-... won't be significantly different than > read-read-process-process-... However in the case where the > storage and CPU are contended for by other processes the former > is more desirable. You could test that by spinning the CPU with > another process while doing the test. It still seems to me > that WILLNEED is not appropriate for the sequential case. > > Here are the results of sorting when two other processes are running at 100% each (dual core) for the duration of the sort: NORMAL : 279.03s, 12.0% CPU SEQUENTIAL: 230.92s, 14.5% CPU WILLNEED : 186.26s, 18.5% CPU
I suspect this trend may also repeat in situations where other processes are contending for both CPU and storage. Perhaps the priority of the WILLNEED read ahead is much higher than that of typical reads? Just a guess. > While we're looking at this it would also be worth considering > DONTNEED which could be used to drop the cache for temp files > when we're finished reading them. Note one shouldn't specify > a len of 0 until the file is completely read so as not to > invalidate any read ahead cache associated with the file. > Thanks for the suggestion. I have tested DONTNEED both in xfclose and in fillbuf but do not see any improvements. It may be that this will only improve performance in situations where resources are very tight. A patch containing both WILLNEED and DONTNEED is included below. To accommodate different advice flags I added some defines for the different flags. Would you recommend doing this any other way? Thanks, Joey >From bd934f42675739d064776bae4e631b0b2f02f7ab Mon Sep 17 00:00:00 2001 From: Joey Degges <[email protected]> Date: Thu, 25 Feb 2010 22:54:50 -0800 Subject: [PATCH] sort: use posix_fadvise to announce access patterns on file data --- configure.ac | 3 +++ src/sort.c | 39 ++++++++++++++++++++++++++++++++++++--- 2 files changed, 39 insertions(+), 3 deletions(-) diff --git a/configure.ac b/configure.ac index b07a52b..c07fbd4 100644 --- a/configure.ac +++ b/configure.ac @@ -344,6 +344,9 @@ if test "$elf_sys" = "yes" && \ gl_ADD_PROG([optional_pkglib_progs], [libstdbuf.so]) fi +# Check for fcntl.h/posix_fadvise +AC_CHECK_HEADERS(fcntl.h, [AC_CHECK_FUNCS(posix_fadvise)]) + ############################################################################ mk="$srcdir/src/Makefile.am" # Extract all literal names from the definition of $(EXTRA_PROGRAMS) diff --git a/src/sort.c b/src/sort.c index 481fdb8..caf4610 100644 --- a/src/sort.c +++ b/src/sort.c @@ -32,6 +32,7 @@ #include "filevercmp.h" #include "hard-locale.h" #include "hash.h" +#include "ignore-value.h" #include "md5.h" #include "physmem.h" #include "posixver.h" @@ -47,6 +48,15 @@ #include "xnanosleep.h" #include "xstrtol.h" +#if HAVE_FCNTL_H && HAVE_POSIX_FADVISE +# include <fcntl.h> +# define XFADV_DONTNEED POSIX_FADV_DONTNEED +# define XFADV_WILLNEED POSIX_FADV_WILLNEED +#else +# define XFADV_DONTNEED 0 +# define XFADV_WILLNEED 0 +#endif + #if HAVE_SYS_RESOURCE_H # include <sys/resource.h> #endif @@ -794,6 +804,17 @@ create_temp_file (int *pfd, bool survive_fd_exhaustion) return node; } +/* Wrapper for posix_fadvise. Used to predeclare an access pattern for file + data. Ignore any errors -- this is only advisory. */ + +static void +xfadvise (int fd, off_t offset, off_t len, int advice) +{ +#if HAVE_POSIX_FADVISE + ignore_value (posix_fadvise (fd, offset, len, advice)); +#endif +} + /* Return a stream for FILE, opened with mode HOW. A null FILE means standard output; HOW should be "w". When opening for input, "-" means standard input. To avoid confusion, do not return file @@ -805,10 +826,18 @@ stream_open (const char *file, const char *how) { if (!file) return stdout; - if (STREQ (file, "-") && *how == 'r') + if (*how == 'r') { - have_read_stdin = true; - return stdin; + FILE *fp; + if (STREQ (file, "-")) + { + have_read_stdin = true; + fp = stdin; + } + else + fp = fopen (file, how); + xfadvise (fileno (fp), 0, 0, XFADV_WILLNEED); + return fp; } return fopen (file, how); } @@ -845,6 +874,7 @@ xfclose (FILE *fp, char const *file) break; default: + xfadvise (fileno (fp), 0, 0, XFADV_DONTNEED); if (fclose (fp) != 0) die (_("close failed"), file); break; @@ -1044,6 +1074,7 @@ open_temp (const char *name, pid_t pid) break; } + xfadvise (fileno (fp), 0, 0, XFADV_WILLNEED); return fp; } @@ -1612,6 +1643,8 @@ fillbuf (struct buffer *buf, FILE *fp, char const *file) } } + xfadvise (fileno (fp), 0, ftell (fp), XFADV_DONTNEED); + /* Find and record each line in the just-read input. */ while ((p = memchr (ptr, eol, ptrlim - ptr))) { -- 1.6.3.3
