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:

[...]

>>>Therefore I think we have a choice of solutions: we can either change
>>>the pure virtual functions to be something lik e.g.:
>>>
>>>virtual Cyg_ErrNo write( const cyg_uint8 *buffer, cyg_ucount32
>>>buffer_length, cyg_ucount32 *bytes_written ) {
>>>  CYG_FAIL("write method called directly on Cyg_OutputStream");
>>>}
>>>
>>>However even that depends on assertion support being on, so is not
>>>ideal.
>>
>> Why not ideal? Either you want assertions and then you compile with
>> assertions on, or you don't. I think it's the best way to handle this
>> provided you don't want to bring C++ runtime to every application. [I
>> have my applications in C++, so I do battle with these issues anyway]
>
> Firstly, since this would be a fundamental problem, if it occurs with
> assertions off, it could be unnoticed.

The question is how is it different from any other occurrence of CYG_FAIL
in the eCos source base?

Please note that it's extremely unlikely that user will inherit from
this class and thus will introduce a bug by not overwriting one of those
functions, so even making them just empty is IMHO acceptable, while not
better then CYG_FAIL.

> Secondly (and this applies even with assertions off), I couldn't be
> sure that the compiler will always remove this base class virtual
> function from the image, even if it is unused in practice. That could
> be verified one way or the other admittedly. Nevertheless....

I think compiler won't remove the base class virtual function, unless it
can remove entire virtual table. So it does have an overhead of having
these 3 empty functions. What I don't get is why is it a problem for an
application that uses either Cyg_StdioStream or vfnprintf() that both
are rather fat anyway.

>>>At the same time, it adds virtual functions in each stream. It does
>>>take away the DEVIO_TABLE global, but the virtual functions are for
>>>_every_ stream, which in many cases will mean 36 bytes for
>>>functionality that may never be used (and since we're trying to fit
>>>eCos on some SoC with 16K RAM, we care!).
>>
>> How come 36 bytes? 4 bytes x 9 streams? Do you really have 9 streams on
>> SoC with 16K RAM?!
>>
>> On ARM I get 4 bytes * number of streams: pointer to virtual table for
>> every stream, plus 28 bytes * 3 for total 3 virtual tables, but the
>> latter doesn't depend on the number of streams.
>
> My miscalculation made it sound better than it was in fact. In the
> common case: three streams for stdin, stdout, stderr, give 12 bytes,
> add 28*3 to give 96 bytes.

[...]

> 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.

RAM use from vtables, as calculated above, is 12 bytes, as vtables
themselves are ROMable.

ROM: I've miscalculated it either. Direct penalty on ARM is 192 bytes
for all the virtual tables plus code for three empty functions from the
base class. Indirect code increase from calling virtual vs non-virtual
function in vfnprintf() raises the resulting overhead to about 400 bytes
of ROM (this is with gcc-2.95, more recent gcc has less overhead on
repeated virtual calls). 

That said, I'm still not convinced it needs a configuration option. In
general, there is size vs speed issue with almost any non-trivial code,
and we obviously aren't going to have at least two variants for every
such case and to put all of them under options?

For example, I do know how to easily reduce total overhead to about 200
bytes though it will cost another funcall for every write() operation
within vfnprintf(). Do we put it under yet another option?

-- Sergei.