Re: Reduce stack usage of _vfiprintf_r()

[Corinna Vinschen <vinschen at redhat dot com>]

On Oct 11 17:06, Federico Terraneo wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> On 10/11/2012 04:39 PM, Freddie Chopin wrote:
> > W dniu 2012-10-11 16:28, Claudio Lanconelli pisze:
> >> Hi, I use newlib for ARM Cortex-M3. I agree with Joel, a
> >> malloc()/free() call for each printf() is not acceptable for me,
> >> this would raise the risk of memory fragmentation.
> >> 
> >> Just my two cents.
> > 
> > Please - first read, then post!
> > 
> > Currently EACH entry to _vf[i]printf_r() costs you 1.3kB of stack.
> > When you use it with an unbuffered stream it will use twice that
> > (2.6kB).
> > 
> > With the change I suggested EACH entry to _vf[i]printf_r() costs
> > you 300B of stack. ALWAYS! There is NO additional memory allocation
> > unless you use an unbuffered stream. Malloc() is called ONLY if the
> > stream is unbuffered! There risk of memory fragmentation is very
> > low, because the buffer is freed before the function returns -
> > fragmentation would only happen in multithreaded application when
> > another task interrupts the stream processing and allocates
> > something more. No fragmentation is possible for a single threaded
> > application. Moreover memory fragmentation usually happens if you
> > allocate SMALL pieces of memory and free them in random order, not
> > a 1kB buffer which is freed right away...
> > 
> > Just my three cents. It's probably the third or fourth time I'm 
> > explaining... Nobody is suggesting to replace unconditional
> > allocation on stack with an uncoditional allocation on heap! Not
> > that it would be a wrong idea in some places...
> > 
> > BTW - printf() style functions use malloc all the time, especially
> > for floating point, so your disagreement is a bit late...
> > 
> > 4\/3!!
> 
> My two cents are to fix this by making sure __sbprintf() cannot be
> inlined. If I understand correctly, this would reduce the stack usage
> down to 300bytes for buffered streams, and 1.3KB for unbuffered ones.
> People still concerned with that 1.3KB may modify __BUFSIZ__ to a
> lower value, or simply avoid using unbuffered streams.
> 
> Advantages:
> - - it's a simple patch, can either be done by removing the static
>   qualifier or, if the compiler is GCC, with __attribute__((noinline))
> - - it introduces no performance penalty but the overhead of a function
>   call, which I don't think is a concern for any platform newlib
>   supports, so there's no need to enable this conditionally only for
>   small memory targets
> - - it optimizes for stack usage in the common case of buffered streams
> - - it does not introduce heap allocations

I tend to prefer this as well.  What about this patch?

Index: libc/stdio/vfprintf.c
===================================================================
RCS file: /cvs/src/src/newlib/libc/stdio/vfprintf.c,v
retrieving revision 1.82
diff -u -p -r1.82 vfprintf.c
--- libc/stdio/vfprintf.c	8 Aug 2012 11:04:17 -0000	1.82
+++ libc/stdio/vfprintf.c	11 Oct 2012 17:28:13 -0000
@@ -333,8 +333,17 @@ int __sprint_r (struct _reent *, FILE *,
  * Helper function for `fprintf to unbuffered unix file': creates a
  * temporary buffer.  We only work on write-only files; this avoids
  * worries about ungetc buffers and so forth.
+ *
+ * Make sure to avoid inlining when optimizing for size.
  */
-static int
+#ifndef __OPTIMIZE_SIZE__
+static
+#else
+#if defined (__GNUC__) && __GNUC__ > 3 || (__GNUC__ == 3 && __GNUC_MINOR__ >= 1)
+__attribute__ ((__noinline__)) static 
+#endif
+#endif /* __OPTIMIZE_SIZE__ */
+int
 _DEFUN(__sbprintf, (rptr, fp, fmt, ap),
        struct _reent *rptr _AND
        register FILE *fp   _AND


Corinna

-- 
Corinna Vinschen
Cygwin Project Co-Leader
Red Hat