r215360 - Increase SmallVector<> size in DiagnosticsEngine::setSeverityForGroup

[Richard Smith]

On Mon, Aug 11, 2014 at 1:54 PM, Hans Wennborg <hans at chromium.org> wrote:

> On Mon, Aug 11, 2014 at 1:28 PM, David Blaikie <dblaikie at gmail.com> wrote:
> > On Mon, Aug 11, 2014 at 9:05 AM, Hans Wennborg <hans at hanshq.net> wrote:
> >> Author: hans
> >> Date: Mon Aug 11 11:05:54 2014
> >> New Revision: 215360
> >>
> >> URL: http://llvm.org/viewvc/llvm-project?rev=215360&view=rev
> >> Log:
> >> Increase SmallVector<> size in DiagnosticsEngine::setSeverityForGroup
> >>
> >> In a Clang bootstrap, this vector always held 129 elements.
> >
> > Presumably this is because the largest diagnostic group has 129
> > elements. When you say "always held 129 elements" - you mean the
> > maximum for a given compilation was 129? Or on /every/ call it was
> > 129? I'd be surprised if it was the same count on every call.
>
> It was 129 on every call. Maybe it was only getting called for a
> specific diagnostics group.


At a guess, your build had only one -W flag, and that group has 129
diagnostics. Maybe it's -Wall?


> > & are we going to update this whenever the number of diagnostics in
> > the largest group changes?
>
> I bumped it to 256 thinking that that "should be enough for everyone".
>
> > (& my personal leaning being "if performance here doesn't matter,
> > perhaps we should have a default size or drop the small optimization
> > entirely so we don't have to wonder if it's right/needs updating in
> > the future")
>
> I got samples from about 2000 small-optimized containers in my
> bootstrap. The vast majority of those are probably not important for
> performance, but if we switched all of them to non-small optimized
> containers I'm sure the impact would be visible.
>
>  - Hans
>
> >> Modified: cfe/trunk/lib/Basic/Diagnostic.cpp
> >> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Diagnostic.cpp?rev=215360&r1=215359&r2=215360&view=diff
> >>
> ==============================================================================
> >> --- cfe/trunk/lib/Basic/Diagnostic.cpp (original)
> >> +++ cfe/trunk/lib/Basic/Diagnostic.cpp Mon Aug 11 11:05:54 2014
> >> @@ -232,13 +232,13 @@ bool DiagnosticsEngine::setSeverityForGr
> >>                                              StringRef Group,
> diag::Severity Map,
> >>                                              SourceLocation Loc) {
> >>    // Get the diagnostics in this group.
> >> -  SmallVector<diag::kind, 8> GroupDiags;
> >> +  SmallVector<diag::kind, 256> GroupDiags;
> >>    if (Diags->getDiagnosticsInGroup(Flavor, Group, GroupDiags))
> >>      return true;
> >>
> >>    // Set the mapping.
> >> -  for (unsigned i = 0, e = GroupDiags.size(); i != e; ++i)
> >> -    setSeverity(GroupDiags[i], Map, Loc);
> >> +  for (diag::kind Diag : GroupDiags)
> >> +    setSeverity(Diag, Map, Loc);
> >>
> >>    return false;
> >>  }
> >>
> >>
> >> _______________________________________________
> >> cfe-commits mailing list
> >> cfe-commits at cs.uiuc.edu
> >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
> > _______________________________________________
> > cfe-commits mailing list
> > cfe-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140811/7a301b92/attachment.html>