Re: [PATCH] Do not break buffers in fvwrite for unbuffered files

[Federico Terraneo <fede dot tft at hotmail dot it>]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 10/21/2013 05:36 PM, Corinna Vinschen wrote:
> On Oct 21 16:15, Federico wrote:
>> -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
>> 
>> On 10/21/2013 02:47 PM, Corinna Vinschen wrote:
>>> 
>>> Maye it makes sense to use the largest multiple  of BUFSIZ <= 
>>> MAX_INT. See below.
>>>> 
>>>> I'm wondering if it makes sense to avoid splitting writes
>>>> (and reads) also for buffered files. glibc appears to do it,
>>> 
>>> Buffered writes are already split into fp->_bf._size chunks. 
>>> Glibc basically uses the largest multiple of fp->_bf._size <=
>>> len instead. We could do the same.  Would you like to create a
>>> patch for that?
>> 
>> I'll have a look at what can be done in the next days.
> 
> Thanks!

Sorry for the delay, I've been quite busy.

Anyway, here's the patch that makes both unbuffered and fully buffered
writes faster. It writes in multiples of fp->_bf._size, and clamps at
INT_MAX, as requested.

Here's the changelog entry:

2013-10-31  Terraneo Federico  <fede.tft@hotmail.it>

	* libc/stdio/fvwrite.c: Allow writing in multiples of
	 fp->_bf._size for fully buffered and unbuffered files,
	 to improve write performance.

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQEcBAEBAgAGBQJScih7AAoJECkLFtN5Xr9fXQoIALOk290GUWA3YvT1b+UkSf+Y
RXEGK2kX0IGMrZu8Affq2VX2wbudKFKBktZFhTaUmjo3e9whmDz1S3+CQJhpndpu
FrJdy//v5Z1A/+nDy2oI1y7xiOO8c8DbhU+S5AlOh09uTXXxthW/Rw8yB4Qhj5/B
mu02savYNM38e7lgjr+Dly2Gyg768lpqUx3yMRVKfB6yLOpOOCObaIwqDEdYVHnO
QbvNMx4QeoglWEOo3mBTJa8eja16an4MVGbte2SmaMDODU337tPoe/Cv8TpKQfj2
THtU62CnNT6UVBPxaRcsoCnHqyNirZ/tvnofDkRidaBSbLi7oF4M7JcRe+Hx3p4=
=iRfH
-----END PGP SIGNATURE-----

diff -ruN a/newlib/libc/stdio/fvwrite.c b/newlib/libc/stdio/fvwrite.c
--- a/newlib/libc/stdio/fvwrite.c	2013-10-31 01:10:06.110679241 +0100
+++ b/newlib/libc/stdio/fvwrite.c	2013-10-31 09:25:04.543332198 +0100
@@ -21,6 +21,7 @@
 #include <string.h>
 #include <stdlib.h>
 #include <errno.h>
+#include <limits.h>
 #include "local.h"
 #include "fvwrite.h"
 
@@ -89,12 +90,16 @@
   if (fp->_flags & __SNBF)
     {
       /*
-       * Unbuffered: write up to BUFSIZ bytes at a time.
+       * Unbuffered: split the buffer in the larger chunk that
+       * is both less than INT_MAX, as some legacy code may
+       * expect int instead of size_t, and is a multiple of
+       * fp->_bf._size.
        */
       do
 	{
 	  GETIOV (;);
-	  w = fp->_write (ptr, fp->_cookie, p, MIN (len, BUFSIZ));
+	  w = fp->_write (ptr, fp->_cookie, p,
+	    ((int)MIN(len, INT_MAX)) / fp->_bf._size * fp->_bf._size);
 	  if (w <= 0)
 	    goto err;
 	  p += w;
@@ -177,30 +182,24 @@
 	      fp->_p += w;
 	      w = len;		/* but pretend copied all */
 	    }
-	  else if (fp->_p > fp->_bf._base && len > w)
+	  else if (fp->_p > fp->_bf._base || len < fp->_bf._size)
 	    {
-	      /* fill and flush */
+	      /* pass through the buffer */
+	      w = MIN(len, w);
 	      COPY (w);
-	      /* fp->_w -= w; *//* unneeded */
+	      fp->_w -= w;
 	      fp->_p += w;
-	      if (_fflush_r (ptr, fp))
+	      if(len > w) if(_fflush_r (ptr, fp))
 		goto err;
 	    }
-	  else if (len >= (w = fp->_bf._size))
+	  else
 	    {
 	      /* write directly */
+	      w = (int)MIN(len, INT_MAX) / fp->_bf._size * fp->_bf._size;
 	      w = fp->_write (ptr, fp->_cookie, p, w);
 	      if (w <= 0)
 		goto err;
 	    }
-	  else
-	    {
-	      /* fill and done */
-	      w = len;
-	      COPY (w);
-	      fp->_w -= w;
-	      fp->_p += w;
-	    }
 	  p += w;
 	  len -= w;
 	}

Attachment: fvwrite.patch.sig
Description: Binary data