[PATCH] D14053: Optimize StringTableBuilder

[Rafael Espíndola via llvm-commits]

Lgtm
On Oct 26, 2015 5:47 AM, "Rui Ueyama" <ruiu at google.com> wrote:

> On Mon, Oct 26, 2015 at 4:08 AM, Rafael Espíndola <
> rafael.espindola at gmail.com> wrote:
>
>> > -O0
>> > real    0m0.455s
>> > real    0m0.430s (5.5% faster)
>> >
>> > -O3
>> > real    0m0.487s
>> > real    0m0.452s (7.2% faster)
>>
>> Nice!
>>
>> > +static void qsort(StringPair **Begin, StringPair **End, int Pos) {
>> > +  if (End - Begin < 2)
>> > +    return;
>> > +
>> > +  // Safeguard for pathetic input.
>> > +  if (Pos > 10000)
>> > +    return;
>>
>> We get here if two strings have a lot of characters in common, right?
>> This if then avoids running out of stack.
>
>
> Right.
>
>
>>
>> > +  // Partition items. Items in [Begin, P) are less than the pivot, [P,
>> Q)
>> > +  // are the same as the pivot, and [Q, End) are greater than the
>> pivot.
>>
>> We are actually sorting the other way, no? Things larger than the
>> pivot go to be start of the array.
>
>
> Fixed.
>
>
>>
>> > +  int Pivot = charTailAt(*Begin, Pos);
>> > +  StringPair **P = Begin;
>> > +  StringPair **Q = End;
>> > +  for (StringPair **R = Begin + 1; R < Q;) {
>> > +    int C = charTailAt(*R, Pos);
>> > +    if (C > Pivot)
>> > +      std::swap(*P++, *R++);
>> > +    else if (C < Pivot)
>> > +      std::swap(*--Q, *R);
>> > +    else
>> > +      R++;
>> >    }
>> > -  return SizeB - SizeA;
>> > +  qsort(Begin, P, Pos);
>> > +  if (Pivot != -1)
>> > +    qsort(P, Q, Pos + 1);
>>
>> This is the recursive call that can causes us to run out of stack. If
>> you write this as
>>
>> qsort(Begin, P, Pos);
>> qsort(Q, End, Pos);
>> if (Pivot != -1)
>>  loop back to stort P, Q, Pos + 1.
>>
>> We should be able to remove the earlier check for Pos being too large.
>>
>
> Funny you should mention that. I thought at that time that might be a bit
> too much, but since you asked too, we probably should do. Please take a
> look at the new patch. (If you don't like goto, I can use a for(;;)
> instead.)
>
>
>> Thanks you so much for ding this, it should help MC too!
>>
>> Cheers,
>> Rafael
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151026/c982725f/attachment.html>