Re: [boost] [locale] Review

Subject: Re: [boost] [locale] Review
From: Mathias Gaunard (mathias.gaunard_at_[hidden])
Date: 2011-04-21 09:24:58

On 20/04/2011 16:30, Artyom wrote:
>> From: Mathias Gaunard<mathias.gaunard_at_[hidden]>
>>
>> Generating very small functions that keep jumping and doing
>> indirections between each other is even worse than duplicating
>> a bit of binary  code.
>> Inlining gives very good performance benefits.
>
> Some people (performance freaks would disagree with you)
> see Linux kernel coding style.
>
> But this is other story.

High-performance computing is my job, but whatever.

> I see real libraries, like Boost.Asio and I see
> their endless compilation times and enormous
> executable sizes it creates and I'm not
> happy with this.
>
> That is a reality. Not a myth.

The problem with Asio is not templates, it's that the author likes it to 
be header-only to avoid having to set up a proper build environment.
Some code should definitely be in its own TU.

Compilation time is usually much longer when using templates, but that's 
not really a problem.

> I agree, template metaprogramming is
> very important part of C++ but everything
> has its own place.

Why do you insist on confusing templates with template meta-programming?
Template meta-programming is some very specific usage of templates.

> Sometimes dynamic polymorphism
> is better then static and sometimes
> other way around.

Its only advantage is being dynamic.

But maybe you don't need that dynamism?

> In order to have ranges just use break_iterator that
> returns boundary points. That it was designed for.

break_iterator is an iterator of iterators, not ranges.
And a poor one, but that's another issue.

> However token_iterator designed to create tokens.
>
> Note, if token iterator was returning a range
> you wouldn't be able to call
>
>     std::string s = *it;

Is that a problem? Why would you want to do such a thing in the first 
place? You already have the data, why copy it?

In any case, it's still possible by adding a conversion operator to the 
range type in question.

Maybe one of the Boost.Range maintainers could comment on why 
iterator_range doesn't have such a conversion operator?

Anyway, the whole boundary part of the library has a horrible design in 
my opinion. The more I look at it the less I like it.
It changing completely is the main condition for a future acceptance 
from me.

> I see, but sometimes I don't really agree that
> the interface you suggest simplifies things,
> sometimes it may but not always.
>
> But this is my point of view.

I don't see how that kind of simplification can be subject to points or 
view.

Not requiring users to convert to your format is simpler than requiring 
them to do so, it's a fact.

If I wanted to use your library, it would be ironic if the first thing I 
had to do is write a wrapper around it. I thought it was a library that 
aimed at being a wrapper of ICU with a nice C++ interface.

> You assume that "whatever" facet is stateful, and it is not
> it is const and stateless, unless I provide some specific
> state as additional parameter.

No, I assumed it was stateless.

>
> So this way or other I'll have to put something to buffer,
> or state or whatever, on stack if it is small and on
> heap if it is not so basically it should be.

Don't allocate anything but small automatic variables.
The algorithm itself should be separate from the buffer management.

>
>     allocate on stack small input buffer
>
>     try to copy the range to input buffer?
>     succeed
>       call to_upper
>         if(output buffer is too small
>           allocate one and call to_upper again
>         else
>           copy output buffer to output stream
>     else
>       allocate input buffer on heap.
>       copy data from temporary buffer
>       to the buffer on heap
>       update it with remaining part of data
>       allocate output buffer
>       run to_upper
>          if output buffer too small, reallocate it
>            run to_upper again
>       copy output buffer to output iterator...

Your algorithm seems wrong.
If the output buffer is too small, simply don't process the data that 
would require a larger buffer.

Or you could design the code so that the output buffer is always 
statically guaranteed to be big enough.

And don't copy the whole input range to a contiguous buffer (that would 
need to be heap-allocated), copy it in bits to an automatic buffer.

> But this is really "Dikumware STL" problem because other
> normal libraries do it in big chunks as they should.

If your library requires on codecvt facets to be efficient and the most 
popular implementation isn't, then it becomes your library's problem.