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

[David Blaikie]

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.

Huh. Curious.

>> & 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".

*nod

(more generally, if we're going to tune these, it might be useful for
your patches/tools for doing that tuning go somewhere so we can repeat
the experiment every now & then)

>> (& 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.

Perhaps - maybe I'll get around to testing this one day (my initial
thought was just to alias-template SmallVector to std::vector to see
what happens... but turned out to be a bit more than an evening's
hackery so I haven't got around to finishing it).

- David

>
>  - 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