This is the mail archive of the newlib@sourceware.org mailing list for the newlib project.
| Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
|---|---|---|
| Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |
| Other format: | [Raw text] | |
On Oct 21 10:36, Federico wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> While hunting a performance bottleneck related to writing files on a
> microcontroller I noticed an inconsistency in newlib for unbuffered files.
>
> While doing an fread() with a large buffer results in a call to read()
> with the same buffer and size, a call to fwrite() breaks the buffer in
> multiple calls to write() each up to BUFSIZ in size.
>
> This may cause performance issues, and for reference glibc seems to even
> try to pass large buffers untouched also if the files are fully
> buffered, at least that's what a simple test using strace on Linux has
> shown.
>
> This simple patch makes the behaviour of fwrite() consistent with fread().
>
>
> 2013-10-21 Terraneo Federico <...?
>
> * libc/stdio/fvwrite.c: Do not break buffers for unbuffered
> files
> [...]
> - w = fp->_write (ptr, fp->_cookie, p, MIN (len, BUFSIZ));
> + w = fp->_write (ptr, fp->_cookie, p, len);
This code is straight from BSD code, which you'll find is still the same
in FreeBSD, NetBSD and OpenBSD. AFAIK, the idea here is to write always
in chunks of BUFSIZ since it's meant to be a multiple of the underlying
blocksize of the usual devices.
On the other hand, a quick test using a fread/fwrite loop to copy a file
to and fro an NFS share under Cygwin, using 1 Meg reads/writes, results
in an excessive perfomance gain, which is why I think your patch should
go in.
But there's another problem. len is size_t. The third parameter to
the fp->_write, as well as w, the return value from fp->_write, are int.
This is hopelessly outdated and therefore wrong.
The _read/_write size parameter should be size_t per POSIX. w should be
ssize_t or better _READ_WRITE_RETURN_TYPE. As it is, len could be too
big (sizeof (size_t) > sizeof (int)).
This has to be fixed first.
Here's my Proposal:
We introduce a new define called _READ_WRITE_BUFSIZE_TYPE. It is used
as the type of the `count' parameter for the _read and _write methods in
struct __sFILE. It's set to int by default for backward compatibility.
Interested targets can set it to the more correct size_t in
include/sys/config.h.
Here's a patch. Please check if that's ok for your targets. In theory,
it shouldnt change anything for existing targets, unless you define your
own _READ_WRITE_BUFSIZE_TYPE. I'm going to do so for Cygwin, if the
patch is ok.
Thanks,
Corinna
* libc/include/stdio.h (funopen): Change prototype of
__readfn and __writefn parameter to match new definition of
FILE's _read and _write methods.
(_funopen_r): Ditto.
(funopen): Ditto.
(_funopen_r): Ditto.
* libc/include/sys/config.h (_READ_WRITE_BUFSIZE_TYPE) Define
as type int if not already defined. Add comment to explain.
* libc/include/sys/reent.h: Include stddef.h.
(struct __sFILE): Change type of last parameter in declaration
of _read and _write methods to _READ_WRITE_BUFSIZE_TYPE.
(struct __sFILE64): Ditto.
* libc/stdio/local.h (__sread): Declare with last parameter set
to _READ_WRITE_BUFSIZE_TYPE.
(__seofread): Ditto.
(__swrite): Ditto.
(__swrite64): Ditto.
* libc/stdio/fvwrite.c (__sfvwrite_r): Change type of local
variables w and s to _READ_WRITE_BUFSIZE_TYPE.
* libc/stdio/fmemopen.c (fmemreader): Align to above change.
(fmemwriter): Ditto.
* libc/stdio/fopencookie.c (fcreader): Ditto.
(fcwriter): Ditto.
* libc/stdio/funopen.c (funread): Ditto.
(funwrite): Ditto.
(funreader): Ditto.
(funwriter): Ditto.
* libc/stdio/open_memstream.c (memwriter): Ditto.
* libc/stdio/stdio.c (__sread): Ditto.
(__seofread): Ditto.
(__swrite): Ditto.
* libc/stdio64/stdio64.c (__swrite64): Ditto.
Index: libc/include/stdio.h
===================================================================
RCS file: /cvs/src/src/newlib/libc/include/stdio.h,v
retrieving revision 1.64
diff -u -p -r1.64 stdio.h
--- libc/include/stdio.h 26 Jun 2013 21:34:16 -0000 1.64
+++ libc/include/stdio.h 21 Oct 2013 11:30:14 -0000
@@ -516,24 +516,32 @@ int _EXFUN(__swbuf_r, (struct _reent *,
#ifndef __STRICT_ANSI__
# ifdef __LARGE64_FILES
FILE *_EXFUN(funopen,(const _PTR __cookie,
- int (*__readfn)(_PTR __c, char *__buf, int __n),
- int (*__writefn)(_PTR __c, const char *__buf, int __n),
+ int (*__readfn)(_PTR __c, char *__buf,
+ _READ_WRITE_BUFSIZE_TYPE __n),
+ int (*__writefn)(_PTR __c, const char *__buf,
+ _READ_WRITE_BUFSIZE_TYPE __n),
_fpos64_t (*__seekfn)(_PTR __c, _fpos64_t __off, int __whence),
int (*__closefn)(_PTR __c)));
FILE *_EXFUN(_funopen_r,(struct _reent *, const _PTR __cookie,
- int (*__readfn)(_PTR __c, char *__buf, int __n),
- int (*__writefn)(_PTR __c, const char *__buf, int __n),
+ int (*__readfn)(_PTR __c, char *__buf,
+ _READ_WRITE_BUFSIZE_TYPE __n),
+ int (*__writefn)(_PTR __c, const char *__buf,
+ _READ_WRITE_BUFSIZE_TYPE __n),
_fpos64_t (*__seekfn)(_PTR __c, _fpos64_t __off, int __whence),
int (*__closefn)(_PTR __c)));
# else
FILE *_EXFUN(funopen,(const _PTR __cookie,
- int (*__readfn)(_PTR __cookie, char *__buf, int __n),
- int (*__writefn)(_PTR __cookie, const char *__buf, int __n),
+ int (*__readfn)(_PTR __cookie, char *__buf,
+ _READ_WRITE_BUFSIZE_TYPE __n),
+ int (*__writefn)(_PTR __cookie, const char *__buf
+ _READ_WRITE_BUFSIZE_TYPE __n),
fpos_t (*__seekfn)(_PTR __cookie, fpos_t __off, int __whence),
int (*__closefn)(_PTR __cookie)));
FILE *_EXFUN(_funopen_r,(struct _reent *, const _PTR __cookie,
- int (*__readfn)(_PTR __cookie, char *__buf, int __n),
- int (*__writefn)(_PTR __cookie, const char *__buf, int __n),
+ int (*__readfn)(_PTR __cookie, char *__buf,
+ _READ_WRITE_BUFSIZE_TYPE __n),
+ int (*__writefn)(_PTR __cookie, const char *__buf,
+ _READ_WRITE_BUFSIZE_TYPE __n),
fpos_t (*__seekfn)(_PTR __cookie, fpos_t __off, int __whence),
int (*__closefn)(_PTR __cookie)));
# endif /* !__LARGE64_FILES */
Index: libc/include/sys/config.h
===================================================================
RCS file: /cvs/src/src/newlib/libc/include/sys/config.h,v
retrieving revision 1.64
diff -u -p -r1.64 config.h
--- libc/include/sys/config.h 9 Jul 2013 13:14:31 -0000 1.64
+++ libc/include/sys/config.h 21 Oct 2013 11:30:14 -0000
@@ -250,6 +250,12 @@
#ifndef _READ_WRITE_RETURN_TYPE
#define _READ_WRITE_RETURN_TYPE int
#endif
+/* Define `count' parameter of read/write routines. In POSIX, the `count'
+ parameter is "size_t" but legacy newlib code has been using "int" for some
+ time. If not specified, "int" is defaulted. */
+#ifndef _READ_WRITE_BUFSIZE_TYPE
+#define _READ_WRITE_BUFSIZE_TYPE int
+#endif
#ifndef __WCHAR_MAX__
#if __INT_MAX__ == 32767 || defined (_WIN32)
Index: libc/include/sys/reent.h
===================================================================
RCS file: /cvs/src/src/newlib/libc/include/sys/reent.h,v
retrieving revision 1.56
diff -u -p -r1.56 reent.h
--- libc/include/sys/reent.h 2 Jul 2013 19:26:20 -0000 1.56
+++ libc/include/sys/reent.h 21 Oct 2013 11:30:14 -0000
@@ -11,6 +11,7 @@ extern "C" {
#define _SYS_REENT_H_
#include <_ansi.h>
+#include <stddef.h>
#include <sys/_types.h>
#define _NULL 0
@@ -192,9 +193,10 @@ struct __sFILE {
_PTR _cookie; /* cookie passed to io functions */
_READ_WRITE_RETURN_TYPE _EXFNPTR(_read, (struct _reent *, _PTR,
- char *, int));
+ char *, _READ_WRITE_BUFSIZE_TYPE));
_READ_WRITE_RETURN_TYPE _EXFNPTR(_write, (struct _reent *, _PTR,
- const char *, int));
+ const char *,
+ _READ_WRITE_BUFSIZE_TYPE));
_fpos_t _EXFNPTR(_seek, (struct _reent *, _PTR, _fpos_t, int));
int _EXFNPTR(_close, (struct _reent *, _PTR));
@@ -247,9 +249,10 @@ struct __sFILE64 {
_PTR _cookie; /* cookie passed to io functions */
_READ_WRITE_RETURN_TYPE _EXFNPTR(_read, (struct _reent *, _PTR,
- char *, int));
+ char *, _READ_WRITE_BUFSIZE_TYPE));
_READ_WRITE_RETURN_TYPE _EXFNPTR(_write, (struct _reent *, _PTR,
- const char *, int));
+ const char *,
+ _READ_WRITE_BUFSIZE_TYPE));
_fpos_t _EXFNPTR(_seek, (struct _reent *, _PTR, _fpos_t, int));
int _EXFNPTR(_close, (struct _reent *, _PTR));
Index: libc/stdio/fmemopen.c
===================================================================
RCS file: /cvs/src/src/newlib/libc/stdio/fmemopen.c,v
retrieving revision 1.3
diff -u -p -r1.3 fmemopen.c
--- libc/stdio/fmemopen.c 30 May 2012 08:58:42 -0000 1.3
+++ libc/stdio/fmemopen.c 21 Oct 2013 11:30:15 -0000
@@ -87,7 +87,7 @@ _DEFUN(fmemreader, (ptr, cookie, buf, n)
struct _reent *ptr _AND
void *cookie _AND
char *buf _AND
- int n)
+ _READ_WRITE_BUFSIZE_TYPE n)
{
fmemcookie *c = (fmemcookie *) cookie;
/* Can't read beyond current size, but EOF condition is not an error. */
@@ -107,7 +107,7 @@ _DEFUN(fmemwriter, (ptr, cookie, buf, n)
struct _reent *ptr _AND
void *cookie _AND
const char *buf _AND
- int n)
+ _READ_WRITE_BUFSIZE_TYPE n)
{
fmemcookie *c = (fmemcookie *) cookie;
int adjust = 0; /* true if at EOF, but still need to write NUL. */
Index: libc/stdio/fopencookie.c
===================================================================
RCS file: /cvs/src/src/newlib/libc/stdio/fopencookie.c,v
retrieving revision 1.5
diff -u -p -r1.5 fopencookie.c
--- libc/stdio/fopencookie.c 30 May 2012 08:58:42 -0000 1.5
+++ libc/stdio/fopencookie.c 21 Oct 2013 11:30:15 -0000
@@ -101,7 +101,7 @@ _DEFUN(fcreader, (ptr, cookie, buf, n),
struct _reent *ptr _AND
void *cookie _AND
char *buf _AND
- int n)
+ _READ_WRITE_BUFSIZE_TYPE n)
{
int result;
fccookie *c = (fccookie *) cookie;
@@ -116,7 +116,7 @@ _DEFUN(fcwriter, (ptr, cookie, buf, n),
struct _reent *ptr _AND
void *cookie _AND
const char *buf _AND
- int n)
+ _READ_WRITE_BUFSIZE_TYPE n)
{
int result;
fccookie *c = (fccookie *) cookie;
Index: libc/stdio/funopen.c
===================================================================
RCS file: /cvs/src/src/newlib/libc/stdio/funopen.c,v
retrieving revision 1.2
diff -u -p -r1.2 funopen.c
--- libc/stdio/funopen.c 30 May 2012 08:58:42 -0000 1.2
+++ libc/stdio/funopen.c 21 Oct 2013 11:30:15 -0000
@@ -85,8 +85,9 @@ Supporting OS subroutines required: <<sb
#include <sys/lock.h>
#include "local.h"
-typedef int (*funread)(void *_cookie, char *_buf, int _n);
-typedef int (*funwrite)(void *_cookie, const char *_buf, int _n);
+typedef int (*funread)(void *_cookie, char *_buf, _READ_WRITE_BUFSIZE_TYPE _n);
+typedef int (*funwrite)(void *_cookie, const char *_buf,
+ _READ_WRITE_BUFSIZE_TYPE _n);
#ifdef __LARGE64_FILES
typedef _fpos64_t (*funseek)(void *_cookie, _fpos64_t _off, int _whence);
#else
@@ -107,7 +108,7 @@ _DEFUN(funreader, (ptr, cookie, buf, n),
struct _reent *ptr _AND
void *cookie _AND
char *buf _AND
- int n)
+ _READ_WRITE_BUFSIZE_TYPE n)
{
int result;
funcookie *c = (funcookie *) cookie;
@@ -122,7 +123,7 @@ _DEFUN(funwriter, (ptr, cookie, buf, n),
struct _reent *ptr _AND
void *cookie _AND
const char *buf _AND
- int n)
+ _READ_WRITE_BUFSIZE_TYPE n)
{
int result;
funcookie *c = (funcookie *) cookie;
Index: libc/stdio/fvwrite.c
===================================================================
RCS file: /cvs/src/src/newlib/libc/stdio/fvwrite.c,v
retrieving revision 1.16
diff -u -p -r1.16 fvwrite.c
--- libc/stdio/fvwrite.c 20 Dec 2011 09:06:58 -0000 1.16
+++ libc/stdio/fvwrite.c 21 Oct 2013 11:30:15 -0000
@@ -52,7 +52,7 @@ _DEFUN(__sfvwrite_r, (ptr, fp, uio),
register size_t len;
register _CONST char *p = NULL;
register struct __siov *iov;
- register int w, s;
+ register _READ_WRITE_RETURN_TYPE w, s;
char *nl;
int nlknown, nldist;
Index: libc/stdio/local.h
===================================================================
RCS file: /cvs/src/src/newlib/libc/stdio/local.h,v
retrieving revision 1.37
diff -u -p -r1.37 local.h
--- libc/stdio/local.h 29 Apr 2013 21:06:23 -0000 1.37
+++ libc/stdio/local.h 21 Oct 2013 11:30:15 -0000
@@ -149,11 +149,13 @@ extern int _EXFUN(__sflags,(struct _r
extern int _EXFUN(__sflush_r,(struct _reent *,FILE *));
extern int _EXFUN(__srefill_r,(struct _reent *,FILE *));
extern _READ_WRITE_RETURN_TYPE _EXFUN(__sread,(struct _reent *, void *, char *,
- int));
+ _READ_WRITE_BUFSIZE_TYPE));
extern _READ_WRITE_RETURN_TYPE _EXFUN(__seofread,(struct _reent *, void *,
- char *, int));
+ char *,
+ _READ_WRITE_BUFSIZE_TYPE));
extern _READ_WRITE_RETURN_TYPE _EXFUN(__swrite,(struct _reent *, void *,
- const char *, int));
+ const char *,
+ _READ_WRITE_BUFSIZE_TYPE));
extern _fpos_t _EXFUN(__sseek,(struct _reent *, void *, _fpos_t, int));
extern int _EXFUN(__sclose,(struct _reent *, void *));
extern int _EXFUN(__stextmode,(int));
@@ -168,7 +170,8 @@ extern int _EXFUN(__submore, (struct _re
#ifdef __LARGE64_FILES
extern _fpos64_t _EXFUN(__sseek64,(struct _reent *, void *, _fpos64_t, int));
extern _READ_WRITE_RETURN_TYPE _EXFUN(__swrite64,(struct _reent *, void *,
- const char *, int));
+ const char *,
+ _READ_WRITE_BUFSIZE_TYPE));
#endif
/* Called by the main entry point fns to ensure stdio has been initialized. */
Index: libc/stdio/open_memstream.c
===================================================================
RCS file: /cvs/src/src/newlib/libc/stdio/open_memstream.c,v
retrieving revision 1.7
diff -u -p -r1.7 open_memstream.c
--- libc/stdio/open_memstream.c 30 May 2012 08:58:42 -0000 1.7
+++ libc/stdio/open_memstream.c 21 Oct 2013 11:30:15 -0000
@@ -97,7 +97,7 @@ _DEFUN(memwriter, (ptr, cookie, buf, n),
struct _reent *ptr _AND
void *cookie _AND
const char *buf _AND
- int n)
+ _READ_WRITE_BUFSIZE_TYPE n)
{
memstream *c = (memstream *) cookie;
char *cbuf = *c->pbuf;
Index: libc/stdio/stdio.c
===================================================================
RCS file: /cvs/src/src/newlib/libc/stdio/stdio.c,v
retrieving revision 1.9
diff -u -p -r1.9 stdio.c
--- libc/stdio/stdio.c 11 Mar 2009 11:53:22 -0000 1.9
+++ libc/stdio/stdio.c 21 Oct 2013 11:30:15 -0000
@@ -34,10 +34,10 @@ _DEFUN(__sread, (ptr, cookie, buf, n),
struct _reent *ptr _AND
void *cookie _AND
char *buf _AND
- int n)
+ _READ_WRITE_BUFSIZE_TYPE n)
{
register FILE *fp = (FILE *) cookie;
- register int ret;
+ register ssize_t ret;
#ifdef __SCLE
int oldmode = 0;
@@ -67,7 +67,7 @@ _DEFUN(__seofread, (ptr, cookie, buf, le
struct _reent *_ptr _AND
_PTR cookie _AND
char *buf _AND
- int len)
+ _READ_WRITE_BUFSIZE_TYPE len)
{
return 0;
}
@@ -77,10 +77,10 @@ _DEFUN(__swrite, (ptr, cookie, buf, n),
struct _reent *ptr _AND
void *cookie _AND
char const *buf _AND
- int n)
+ _READ_WRITE_BUFSIZE_TYPE n)
{
register FILE *fp = (FILE *) cookie;
- int w;
+ ssize_t w;
#ifdef __SCLE
int oldmode=0;
#endif
Index: libc/stdio64/stdio64.c
===================================================================
RCS file: /cvs/src/src/newlib/libc/stdio64/stdio64.c,v
retrieving revision 1.3
diff -u -p -r1.3 stdio64.c
--- libc/stdio64/stdio64.c 4 Jun 2007 18:10:17 -0000 1.3
+++ libc/stdio64/stdio64.c 21 Oct 2013 11:30:15 -0000
@@ -51,10 +51,10 @@ _DEFUN(__swrite64, (ptr, cookie, buf, n)
struct _reent *ptr _AND
void *cookie _AND
char const *buf _AND
- int n)
+ _READ_WRITE_BUFSIZE_TYPE n)
{
register FILE *fp = (FILE *) cookie;
- int w;
+ _READ_WRITE_RETURN_TYPE w;
#ifdef __SCLE
int oldmode=0;
#endif
--
Corinna Vinschen
Cygwin Maintainer
Red Hat
Attachment:
pgpa2QExh11MZ.pgp
Description: PGP signature
| Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
|---|---|---|
| Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |