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]

Re: improve printf locality


Eric Blake <ebb9 <at> byu.net> writes:

> On cygwin, where newlib is configured with --enable-newlib-io-long-double,
> print consumed more than an entire page of virtual memory in stack space!
>  This causes unnecessary page faults, decreased cache locality, and other
> potential slowdowns.  It was because at one point in history, newlib used
> to stack allocate the worst case length for %Lf output (if you have a
> 128-bit long double with a 15 bit exponent, this can result in more than
> 4900 characters).  But now newlib uses struct _reent to manage %f output,
> so the storage claimed by buf[BUF] was 90% unused.

Resubmit, in light of other patches committed in the meantime.  OK to apply?


2007-05-10  Eric Blake  <ebb9@byu.net>

	* libc/stdio/vfprintf.c (_VFPRINTF_R): Fix use of decimal point
	in %f.  Avoid malloc when possible for %S.
	(BUF): Improve stack locality by using smaller size.
	(MAXEXPLEN): Define.
	(exponent): Use smaller stack size.

Index: libc/stdio/vfprintf.c
===================================================================
RCS file: /cvs/src/src/newlib/libc/stdio/vfprintf.c,v
retrieving revision 1.58
diff -u -p -r1.58 vfprintf.c
--- libc/stdio/vfprintf.c	11 May 2007 20:09:00 -0000	1.58
+++ libc/stdio/vfprintf.c	14 May 2007 18:08:41 -0000
@@ -224,43 +224,66 @@ _DEFUN(__sbprintf, (rptr, fp, fmt, ap),
 
 
 #ifdef FLOATING_POINT
-#include <locale.h>
-#include <math.h>
-#include "floatio.h"
+# include <locale.h>
+# include <math.h>
 
-#if ((MAXEXP+MAXFRACT+1) > MB_LEN_MAX)
-#  define BUF (MAXEXP+MAXFRACT+1) /* + decimal point */
-#else 
-#  define BUF MB_LEN_MAX
-#endif
+/* For %La, an exponent of 15 bits occupies the exponent character, a
+   sign, and up to 5 digits.  */
+# define MAXEXPLEN		7
+# define DEFPREC		6
 
-#define	DEFPREC		6
+# ifdef _NO_LONGDBL
+
+extern char *_dtoa_r _PARAMS((struct _reent *, double, int,
+			      int, int *, int *, char **));
+
+#  define _DOUBLE double
+#  define _DTOA_R _dtoa_r
+#  define FREXP frexp
+
+# else /* !_NO_LONGDBL */
+
+extern char *_ldtoa_r _PARAMS((struct _reent *, _LONG_DOUBLE, int,
+			      int, int *, int *, char **));
 
-#ifdef _NO_LONGDBL
-static char *
-_EXFUN(cvt, (struct _reent *, double, int, int, char *, int *, int, int *,
-	     char *));
-#else
-static char *
-_EXFUN(cvt, (struct _reent *, _LONG_DOUBLE, int, int, char *, int *, int,
-	     int *, char *));
 extern int _EXFUN(_ldcheck,(_LONG_DOUBLE *));
-#endif
 
-static int _EXFUN(exponent, (char *, int, int));
+#  define _DOUBLE _LONG_DOUBLE
+#  define _DTOA_R _ldtoa_r
+/* FIXME - frexpl is not yet supported; and cvt infloops if (double)f
+   converts a finite value into infinity.  */
+/* #  define FREXP frexpl */
+#  define FREXP(f,e) ((_LONG_DOUBLE) frexp ((double)f, e))
+# endif /* !_NO_LONGDBL */
 
-#else /* no FLOATING_POINT */
+static char *cvt(struct _reent *, _DOUBLE, int, int, char *, int *, int, int *,
+                 char *);
 
-#define	BUF		40
+static int exponent(char *, int, int);
 
 #endif /* FLOATING_POINT */
 
+/* BUF must be big enough for the maximum %#llo (assuming long long is
+   at most 64 bits, this would be 23 characters), the maximum
+   multibyte character %C, and the maximum precision of %La (assuming
+   long double is at most 128 bits with 113 bits of mantissa, this
+   would be 29 characters).  %e, %f, and %g use reentrant storage
+   shared with mprec.  All other formats that use buf get by with
+   fewer characters.  Making BUF slightly bigger reduces the need for
+   malloc in %a and %S, when large precision or long strings are
+   processed.  */
+#define	BUF		40
+#if defined _MB_CAPABLE && MB_LEN_MAX > BUF
+# undef BUF
+# define BUF MB_LEN_MAX
+#endif
+
 #ifndef _NO_LONGLONG
-#define quad_t long long
-#define u_quad_t unsigned long long
+# define quad_t long long
+# define u_quad_t unsigned long long
 #else
-#define quad_t long
-#define u_quad_t unsigned long
+# define quad_t long
+# define u_quad_t unsigned long
 #endif
 
 typedef quad_t * quad_ptr_t;
@@ -389,8 +412,8 @@ _DEFUN(_VFPRINTF_R, (data, fp, fmt0, ap)
 	int expt;		/* integer value of exponent */
 	int expsize = 0;	/* character count for expstr */
 	int ndig = 0;		/* actual number of digits returned by cvt */
-	char expstr[7];		/* buffer for exponent string */
-#endif
+	char expstr[MAXEXPLEN];	/* buffer for exponent string */
+#endif /* FLOATING_POINT */
 	u_quad_t _uquad;	/* integer arguments %[diouxX] */
 	enum { OCT, DEC, HEX } base;/* base for [diouxX] conversion */
 	int dprec;		/* a copy of prec if [diouxX], 0 otherwise */
@@ -400,7 +423,7 @@ _DEFUN(_VFPRINTF_R, (data, fp, fmt0, ap)
 #define NIOV 8
 	struct __suio uio;	/* output information: summary */
 	struct __siov iov[NIOV];/* ... and individual io vectors */
-	char buf[BUF];		/* space for %c, %[diouxX], %[eEfgG] */
+	char buf[BUF];		/* space for %c, %S, %[diouxX], %[aA] */
 	char ox[2];		/* space for 0x hex-prefix */
 #ifdef _MB_CAPABLE
 	wchar_t wc;
@@ -1030,8 +1053,8 @@ reswitch:	switch (ch) {
 					while (1) {
 						if (wcp[m] == L'\0')
 							break;
-						if ((n = (int)_wcrtomb_r (data, 
-                                                     buf, wcp[m], &ps)) == -1) 
{
+						if ((n = (int)_wcrtomb_r (data,
+						     buf, wcp[m], &ps)) == -1) {
 							fp->_flags |= __SERR;
 							goto error;
 						}
@@ -1044,31 +1067,35 @@ reswitch:	switch (ch) {
 					}
 				}
 				else {
-					if ((size = (int)_wcsrtombs_r (data, 
-                                                   NULL, &wcp, 0, &ps)) == -1) 
{
+					if ((size = (int)_wcsrtombs_r (data,
+						   NULL, &wcp, 0, &ps)) == -1) {
 						fp->_flags |= __SERR;
 						goto error;
 					}
 					wcp = (_CONST wchar_t *)cp;
 				}
- 
+
 				if (size == 0)
 					break;
- 
-				if ((malloc_buf = 
-				    (char *)_malloc_r (data, size + 1)) == 
NULL) {
-					fp->_flags |= __SERR;
-					goto error;
-				}
-                             
+
+				if (size >= BUF) {
+					if ((malloc_buf =
+					     (char *)_malloc_r (data, size + 1))
+					    == NULL) {
+						fp->_flags |= __SERR;
+						goto error;
+					}
+					cp = malloc_buf;
+				} else
+					cp = buf;
+
 				/* Convert widechar string to multibyte string. 
*/
 				memset ((_PTR)&ps, '\0', sizeof (mbstate_t));
-				if (_wcsrtombs_r (data, malloc_buf, 
-                                                 &wcp, size, &ps) != size) {
+				if (_wcsrtombs_r (data, malloc_buf,
+						 &wcp, size, &ps) != size) {
 					fp->_flags |= __SERR;
 					goto error;
 				}
-				cp = malloc_buf;
 				cp[size] = '\0';
 			}
 #endif /* _MB_CAPABLE */
@@ -1254,11 +1281,11 @@ number:			if ((dprec = prec) >= 0)
 					PRINT (cp, ndig);
 					PAD (expt - ndig, zeroes);
 					if (flags & ALT)
-						PRINT (".", 1);
+						PRINT (decimal_point, 1);
 				} else {
 					PRINT (cp, expt);
 					cp += expt;
-					PRINT (".", 1);
+					PRINT (decimal_point, 1);
 					PRINT (cp, ndig - expt);
 				}
 			} else {	/* 'a', 'A', 'e', or 'E' */
@@ -1305,21 +1332,6 @@ error:
 
 #ifdef FLOATING_POINT
 
-# ifdef _NO_LONGDBL
-extern char *_dtoa_r _PARAMS((struct _reent *, double, int,
-			      int, int *, int *, char **));
-#  define _DTOA_R _dtoa_r
-#  define FREXP frexp
-# else
-extern char *_ldtoa_r _PARAMS((struct _reent *, _LONG_DOUBLE, int,
-			      int, int *, int *, char **));
-#  define _DTOA_R _ldtoa_r
-/* FIXME - frexpl is not yet supported; and cvt infloops if (double)f
-   converts a finite value into infinity.  */
-/* #  define FREXP frexpl */
-#  define FREXP(f,e) ((_LONG_DOUBLE) frexp ((double)f, e))
-# endif
-
 /* Using reentrant DATA, convert finite VALUE into a string of digits
    with no decimal point, using NDIGITS precision and FLAGS as guides
    to whether trailing zeros must be included.  Set *SIGN to nonzero
@@ -1327,31 +1339,9 @@ extern char *_ldtoa_r _PARAMS((struct _r
    *LENGTH to the length of the returned string.  CH must be one of
    [aAeEfFgG]; if it is [aA], then the return string lives in BUF,
    otherwise the return value shares the mprec reentrant storage.  */
-# ifdef _NO_LONGDBL
 static char *
-_DEFUN(cvt, (data, value, ndigits, flags, sign, decpt, ch, length, buf),
-       struct _reent *data _AND
-       double value _AND
-       int ndigits  _AND
-       int flags    _AND
-       char *sign   _AND
-       int *decpt   _AND
-       int ch       _AND
-       int *length  _AND
-       char *buf)
-# else
-static char *
-_DEFUN(cvt, (data, value, ndigits, flags, sign, decpt, ch, length, buf),
-       struct _reent *data _AND
-       _LONG_DOUBLE value  _AND
-       int ndigits         _AND
-       int flags           _AND
-       char *sign          _AND
-       int *decpt          _AND
-       int ch              _AND
-       int *length         _AND
-       char *buf)
-# endif
+cvt(struct _reent *data, _DOUBLE value, int ndigits, int flags, char *sign,
+    int *decpt, int ch, int *length, char *buf)
 {
 	int mode, dsgn;
 	char *digits, *bp, *rve;
@@ -1444,13 +1434,10 @@ _DEFUN(cvt, (data, value, ndigits, flags
 }
 
 static int
-_DEFUN(exponent, (p0, exp, fmtch),
-       char *p0 _AND
-       int exp  _AND
-       int fmtch)
+exponent(char *p0, int exp, int fmtch)
 {
 	register char *p, *t;
-	char expbuf[10];
+	char expbuf[MAXEXPLEN];
 # ifdef _WANT_IO_C99_FORMATS
 	int isa = fmtch == 'a' || fmtch == 'A';
 # else
@@ -1465,13 +1452,13 @@ _DEFUN(exponent, (p0, exp, fmtch),
 	}
 	else
 		*p++ = '+';
-	t = expbuf + 10;
+	t = expbuf + MAXEXPLEN;
 	if (exp > 9) {
 		do {
 			*--t = to_char (exp % 10);
 		} while ((exp /= 10) > 9);
 		*--t = to_char (exp);
-		for (; t < expbuf + 10; *p++ = *t++);
+		for (; t < expbuf + MAXEXPLEN; *p++ = *t++);
 	}
 	else {
 		if (!isa)



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]