improve printf locality
Jeff Johnston
jjohnstn@redhat.com
Mon May 14 19:44:00 GMT 2007
Minor change. Please use another name for the cvt value type rather
than _DOUBLE (i.e. I don't like the idea of defining _DOUBLE as
_LONG_DOUBLE). Suggest something like _CVT_VALUE_FLOAT_TYPE. Other
than that, patch is fine.
-- Jeff J.
Eric Blake wrote:
> 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)
>
>
More information about the Newlib
mailing list