RFC optimisation of file.c read size

[Stephen R. van den Berg]

In the course of tracking down the system-call-overhead of the pgsql
driver, I encountered the dual read-buffer strategy in file.c.
Although it was/is functional as is, it could be polished a bit.

Any thoughts?

I tried to cater for both large reads from files as well as network
sized reads from arriving packets.

commit e44d08a94ebc9e38cd107177abd1fdfdae8a8e73
Author: Stephen R. van den Berg <[EMAIL PROTECTED]>
Date:   Tue Aug 19 10:53:08 2008 +0200

    file.c optimise buffer management, minimise read size/systemcalls

diff --git a/src/modules/files/file.c b/src/modules/files/file.c
index e69043a..7d4ef16 100644
--- a/src/modules/files/file.c
+++ b/src/modules/files/file.c
@@ -121,8 +121,6 @@
 #define FD (THIS->box.fd)
 #define ERRNO (THIS->my_errno)
 
-#define READ_BUFFER 8192
-
 /* Don't try to use socketpair() on AmigaOS, socketpair_ultra works better */
 #ifdef __amigaos__
 #undef HAVE_SOCKETPAIR
@@ -130,6 +128,10 @@
 
 /* #define SOCKETPAIR_DEBUG */
 
+#define READ_BUFFER    8192
+#define DIRECT_BUFSIZE (64*1024)
+#define SMALL_NETBUF   2048
+
 struct program *file_program;
 struct program *file_ref_program;
 
@@ -472,13 +474,16 @@ static struct pike_string *do_read(int fd,
 {
   ONERROR ebuf;
   ptrdiff_t bytes_read, i;
+  INT32 try_read;
   bytes_read=0;
   *err=0;
 
-  if(r <= 65536)
+  if(r <= DIRECT_BUFSIZE
+   || all && (r<<1) > r && (((r-1)|r)+1)!=(r<<1))   /* r<<1 != power of two */
   {
     struct pike_string *str;
 
+    i=r;
 
     str=begin_shared_string(r);
 
@@ -488,7 +493,9 @@ static struct pike_string *do_read(int fd,
       int fd=FD;
       int e;
       THREADS_ALLOW();
-      i = fd_read(fd, str->str+bytes_read, r);
+
+      try_read = i;
+      i = fd_read(fd, str->str+bytes_read, i);
       e=errno;
       THREADS_DISALLOW();
 
@@ -515,6 +522,24 @@ static struct pike_string *do_read(int fd,
        }
        break;
       }
+
+      /*
+       * Large reads cause the kernel to validate more memory before
+       * starting the actual read, and hence cause slowdowns.
+       * Ideally you read as much as you're going to receive.
+       * This buffer calculation algorithm tries to optimise the
+       * read length depending on past read chunksizes.
+       * For network reads this allows it to follow the packetsizes.
+       * The initial read will attempt the full buffer.
+       */
+      if( i < try_read )
+       i += SMALL_NETBUF;
+      else
+       i += (r-i)>>1;
+      if (i < SMALL_NETBUF)
+       i = SMALL_NETBUF;
+      if(i > r-SMALL_NETBUF)
+       i = r;
     }while(r);
 
     UNSET_ONERROR(ebuf);
@@ -533,18 +558,22 @@ static struct pike_string *do_read(int fd,
     /* For some reason, 8k seems to work faster than 64k.
      * (4k seems to be about 2% faster than 8k when using linux though)
      * /Hubbe (Per pointed it out to me..)
+     *
+     * The slowdowns most likely are because of memory validation
+     * done by the kernel for buffer space which is later unused
+     * (short read)
      */
-#define CHUNK ( 1024 * 8 )
-    INT32 try_read;
     dynamic_buffer b;
 
     b.s.str=0;
     initialize_buf(&b);
     SET_ONERROR(ebuf, free_dynamic_buffer, &b);
+    i = all && (r<<1)>r ? DIRECT_BUFSIZE : READ_BUFFER;
     do{
       int e;
       char *buf;
-      try_read=MINIMUM(CHUNK,r);
+
+      try_read = i;
 
       buf = low_make_buf_space(try_read, &b);
 
@@ -555,23 +584,13 @@ static struct pike_string *do_read(int fd,
 
       check_threads_etc();
 
-      if(i==try_read)
-      {
-       r-=i;
-       bytes_read+=i;
-       if(!all) break;
-      }
-      else if(i>0)
+      if(i>=0)
       {
        bytes_read+=i;
        r-=i;
-       low_make_buf_space(i - try_read, &b);
-       if(!all) break;
-      }
-      else if(i==0)
-      {
-       low_make_buf_space(-try_read, &b);
-       break;
+       if(try_read > i)
+         low_make_buf_space(i - try_read, &b);
+       if(!all || !i) break;
       }
       else
       {
@@ -588,6 +607,16 @@ static struct pike_string *do_read(int fd,
          break;
        }
       }
+
+      /* See explanation in previous loop above */
+      if( i < try_read )
+       i += SMALL_NETBUF;
+      else
+       i += (DIRECT_BUFSIZE-i)>>1;
+      if (i < SMALL_NETBUF)
+       i = SMALL_NETBUF;
+      if(i > r-SMALL_NETBUF)
+       i = r;
     }while(r);
 
     UNSET_ONERROR(ebuf);
@@ -596,7 +625,6 @@ static struct pike_string *do_read(int fd,
       ADD_FD_EVENTS (THIS, PIKE_BIT_FD_READ);
 
     return low_free_buf(&b);
-#undef CHUNK
   }
 }
 
@@ -2457,7 +2485,7 @@ static void file_listxattr(INT32 args)
   if( res<0 && errno==ERANGE )
   {
     /* Too little space in buffer.*/
-    int blen = 65536;
+    int blen = DIRECT_BUFSIZE;
     do_free = 1;
     ptr = xalloc( 1 );
     do {
@@ -2527,7 +2555,7 @@ static void file_getxattr(INT32 args)
   if( res<0 && errno==ERANGE )
   {
     /* Too little space in buffer.*/
-    int blen = 65536;
+    int blen = DIRECT_BUFSIZE;
     do_free = 1;
     ptr = xalloc( 1 );
     do {
-- 
Sincerely,
           Stephen R. van den Berg.
E-mails should be like a lady's skirt:
Long enough to cover the subject, and short enough to be interesting.