Re: Reduce stack usage of _vfiprintf_r()

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

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

On 10/11/2012 07:55 PM, Freddie Chopin wrote:
> W dniu 2012-10-11 19:29, Corinna Vinschen pisze:
>> I tend to prefer this as well.
> 
> I don't know why you're all against dynamic allocation - it is
> basically at the core of I/O streams...
> 
> Consider this scenario - there are 5 threads that do unbuffered I/O
> in emergency situations (let's say it's stderr). With allocation on
> stack, each thread has to have additional 1kB of stack, even if it
> will never be used. With dynamic allocation, in the worst case
> scenario (all threads doing unbuffered I/O at the exact same time,
> kernel preempting each of them in favor of another) some additional
> memory will be wasted (maybe 20B for thread. In "normal" situation
> overall usage of RAM will be smaller, and the fear of fragmentation
> is IMHO pointless - the buffer is freed right after the function
> ends, the risk of fragmentation is minimal and only in
> multithreaded environment - fragmentation is not possible with
> single thread.
> 
> sbrk() and malloc() will be pulled to executable in both cases, as
> they are used in other parts of stdio.
> 
>> 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
> 
> But it's not optimization for size, but optimization for memory use
> - in all places around newlib __OPTIMIZE_SIZE__ is for reducing
> code. Anyway - why make this fix conditional - there is virtually
> noone that would benefit from current situation...

I agree with this statement. If I recall correctly, __OPTIMIZE_SIZE__
is defined if newlib is configured with --enable-target-optspace. Some
time ago i made a benchmark with and without that option and came to
the conclusion that it significantly reduces the speed of some core
functions, strcpy to name one, that it makes the library almost
unusable when running on microcontrollers with low clock frequencies.
Therefore I am no longer using this option.

This patch does not add any significant slowdown, just a function
call, imho it would be better applied unconditionally (ok,
conditionally to the compiler being GCC).

> 
> If you're agains malloc(), then I guess there's nothing more I can
> do to convince you, so this patch is OK but only if unconditional
> (not only when __OPTIMIZE_SIZE__) - there's absolutely no point in
> leaving current behavior in any case.
> 
> 4\/3!!
> 
> 

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

iQEcBAEBAgAGBQJQdw53AAoJECkLFtN5Xr9ffuQIAI4yY0uIP6WQfRVr1LbT8tbN
3sB5TZW0djw2UiKP3qtvxbn3Tkorij9pL1G/n7J5nRGo5S7+4aFq8qZ3IRHtY+qQ
gUJx5hKt/xdHSMQK8E9HSko5g9zUI2WDAJCdY8dX5LwyVOVK81Io5SIx1N28ZMAd
PS/NpZNxTvVq5hnlmlUHQFUTL7ePpnwFz62m0Hu7lx6BiYv6KR3bimG8wdkSeLx6
+PyRh/+6AkBTVz9zOZ/L1Sjba3YyM6R5mY0htBz4alS/SPFf6TF6YQNgMf/Y8EL0
sL6/VZ2LDVqtUcwePU7cOUFVKRxmItjd8LkOnmoCSXcWYdlCR6c2KppmFhSNoWA=
=60jh
-----END PGP SIGNATURE-----