[PATCH] D14053: Optimize StringTableBuilder

[Rui Ueyama via llvm-commits]

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/6c26a17e/attachment.html>