Re: [boost] patch - performance problem in boost string algorithms

From: Pavol Droba (droba_at_[hidden])
Date: 2006-07-20 15:22:39

Hi,

First of all, I'd like to thank you for the nice benchmarks.

Now my comments to your patches:

1.) const Predicate& Pred
Although this change can bring a performance benefits, it has one big
drawback. You cannot pass an ordinary C function as predicate.
At least not in VC++ 7.1

Following code will not work:

int func(int a)
{
        return a+2;
}

template<typename TFunc>
int sum(TFunc f, unsigned int a)
{
        int acum=0;
        for(unsigned int i=0; i<a; i++)
        {
                acum+=f(i);
        }
        
        return acum;
}

...

sum(func, 1000);

I consider this is to be very important functionality, therefor it is
implemented the way it is.

Your investigation however pushed me to investigate this matter once
more.It seems, that gcc have no problem compiling it even in ansi mode.
I'm not sure what is the correct Standard explanation.

So I'll think about it and try to come up with some improvements here.

2.) std::locale()

I'm not a big fan of global objects when they are not necessary. Correct
locale implementation should give only a very small footprint when
instantiating std::locale(), since is all should be reference based.
As seen from your benchmarks, there is great difference between g++ and
msvc. On the other hand, the difference on the vc++ is so large, that
it is worth considering.

3.) replacing member with reference in the classification predicated.

This patch is definitely not acceptable due to reasons that were
partialy explained by Sebastian. Classisfication predicates could
be instantiated elsewhere and the lifetime of the predicate is in
no way bounded to the lifeting of the locales object. So there
is very good chance for "dangling reference" problems.

So that's it for now from my side. Once again thanks for the good work
on the benchmarks. I will definitely try to use the information you have
provided to improve the library.

Best regards,
Pavol

David Manura wrote:
> Janek Kozicki <janek_listy <at> wp.pl> writes:
>> wow. It's a good programming practice to use const& everywhere where
>> possible. T'd love to see this patch applied. Thanks!
> 
> The below second patch provides much further improvement.  I realize the second
> patch is somewhat of a hack, but it illustrates where the rest of the slowdown
> is coming from and may suggest a better way of fixing it.
> 
> Before second patch:
> 
>  MSVC++2005: 1: 2406 2: 2359 3: 1500 4: 297   (run times)
>  G++/Intel:  1:  703 2:  703 3:  188 4: 172
> 
> After second patch:
> 
>  MSVC++2005: 1: 235 2: 250 3: 251 4: 282   (run times)
>  G++/Intel:  1: 141 2: 141 3: 109 4: 172
> 
> There's likely other places in the string algorithms code that could benefit
> from similar changes.
> 
> --davidm
>