Re: [PATCH] Speed-up sprintf() family of functions.

[Sergei Organov <osv at javad dot com>]

Jonathan Larmour <jifl@eCosCentric.com> writes:
> Sergei Organov wrote:
>> Jonathan Larmour <jifl@eCosCentric.com> writes:

[...]

>> I don't think so. Please read a comment in the original vsnprintf.cxx
>> file:
>>
>>     // I suspect most strings passed to vsnprintf will be relatively short,
>>     // so we just take the simple approach rather than have the overhead
>>     // of calling memcpy etc.
>
> eCos has now (for better or worse) been around quite a long time, and
> it may be that that comment precedes the compiler's ability to
> optimise memcpy() with a builtin. Either way, it is now wrong -
> calling memcpy() is a good thing.
>
>>     // simply copy string until we run out of user space
>>
>> and the implementation of memcpy() there. Also please notice that the
>> old implementation was really ugly, preventing almost all loop
>> optimizations due to possible aliasing between pointers.
>
> Oh yes, I know there's a lot of things I would do very differently
> given the chance and the benefit of 10 more years of experience. There
> are many, many places the code could be improved (or rewritten!), and
> that is one.

I think I need to experiment on this then. Hopefully this eCos part is
usually compiled without -fno-builtins?

[...]

>> Currently I think that simple CYG_FAIL is optimal.
>
> Due to the increased RAM use from vtables, and code from the functions
> becoming virtual and thereby always being included even when the code
> is not used, I think this has to be an option.

Well, I think I need to think more about this then.

>
>>> I cannot find a record of a copyright assignment. Did you fill one
>>> in? I know you have made several contributions in the past, and I
>>> thought your one to the kernel would have been large enough to warrant
>>> one.
>>
>>
>> No, I didn't fill any.
>
> Hmm, that kernel change did seem big enough to need one in my view. I
> think someone may have dropped the ball there. We would want an
> assignment for that, irrespective of this libc stdio change.

What kernel change are you talking about?

Happy New Year!

-- Sergei.