[llvm] r251353 - Move imported entities into DwarfCompilationUnit to speed up LTO linking.
Ivan Krasin via llvm-commits
llvm-commits at lists.llvm.org
Mon Oct 26 15:38:05 PDT 2015
Indentationi fixed in r251364.
On Mon, Oct 26, 2015 at 3:12 PM, Ivan Krasin <krasin at google.com> wrote:
> I think there's a possibility that it could be done faster than it's now,
> but the profile shows the majority of the time is now spent somewhere else:
> https://code.google.com/p/chromium/issues/detail?id=542426#c36
> I might return to optimizing this particular spot, but it's not the top(1)
> offender anymore, there's no huge gains here.
>
> re: indents. On it. Will submit a patch without a review, as it's a
> whitespace change.
>
> On Mon, Oct 26, 2015 at 3:00 PM, David Blaikie <dblaikie at gmail.com> wrote:
>
>>
>>
>> On Mon, Oct 26, 2015 at 2:54 PM, Ivan Krasin <krasin at google.com> wrote:
>>
>>>
>>>
>>> On Mon, Oct 26, 2015 at 2:48 PM, David Blaikie <dblaikie at gmail.com>
>>> wrote:
>>>
>>>>
>>>>
>>>> On Mon, Oct 26, 2015 at 2:36 PM, Ivan Krasin via llvm-commits <
>>>> llvm-commits at lists.llvm.org> wrote:
>>>>
>>>>> Author: krasin
>>>>> Date: Mon Oct 26 16:36:35 2015
>>>>> New Revision: 251353
>>>>>
>>>>> URL: http://llvm.org/viewvc/llvm-project?rev=251353&view=rev
>>>>> Log:
>>>>> Move imported entities into DwarfCompilationUnit to speed up LTO
>>>>> linking.
>>>>>
>>>>> Summary:
>>>>> In particular, this CL speeds up the official Chrome linking with LTO
>>>>> by
>>>>> 1.8x.
>>>>>
>>>>> See more details in https://crbug.com/542426
>>>>>
>>>>> Reviewers: dblaikie
>>>>>
>>>>> Subscribers: jevinskie
>>>>>
>>>>> Differential Revision: http://reviews.llvm.org/D13918
>>>>>
>>>>> Modified:
>>>>> llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
>>>>> llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h
>>>>> llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
>>>>> llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h
>>>>>
>>>>> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
>>>>> URL:
>>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp?rev=251353&r1=251352&r2=251353&view=diff
>>>>>
>>>>> ==============================================================================
>>>>> --- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp (original)
>>>>> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp Mon Oct 26
>>>>> 16:36:35 2015
>>>>> @@ -342,9 +342,9 @@ void DwarfCompileUnit::constructScopeDIE
>>>>> // Skip imported directives in gmlt-like data.
>>>>> if (!includeMinimalInlineScopes()) {
>>>>> // There is no need to emit empty lexical block DIE.
>>>>> - for (const auto &E : DD->findImportedEntitiesForScope(DS))
>>>>> - Children.push_back(
>>>>> -
>>>>> constructImportedEntityDIE(cast<DIImportedEntity>(E.second)));
>>>>> + for (const auto *IE : ImportedEntities[DS])
>>>>> + Children.push_back(
>>>>>
>>>>
>>>> Looks like an extra indent happened on the line above (seems like the
>>>> body of the for loop is indented 4 spaces instead of 2)
>>>>
>>> Looking...
>>>
>>>>
>>>>
>>>>> + constructImportedEntityDIE(cast<DIImportedEntity>(IE)));
>>>>> }
>>>>>
>>>>> // If there are only other scopes as children, put them directly
>>>>> in the
>>>>>
>>>>> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h
>>>>> URL:
>>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h?rev=251353&r1=251352&r2=251353&view=diff
>>>>>
>>>>> ==============================================================================
>>>>> --- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h (original)
>>>>> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h Mon Oct 26
>>>>> 16:36:35 2015
>>>>> @@ -39,6 +39,12 @@ class DwarfCompileUnit : public DwarfUni
>>>>> /// The start of the unit within its section.
>>>>> MCSymbol *LabelBegin;
>>>>>
>>>>> + typedef llvm::SmallVector<const MDNode *, 8> ImportedEntityList;
>>>>> + typedef llvm::DenseMap<const MDNode *, ImportedEntityList>
>>>>> + ImportedEntityMap;
>>>>> +
>>>>> + ImportedEntityMap ImportedEntities;
>>>>>
>>>>
>>>> Are you going to experiment with switching this back to a sorted
>>>> sequence? Might be a nice memory savings over the overhead of
>>>> densemap+vectors? Possibly a runtime savings too in terms of building the
>>>> structures (building, sorting, then searching can be cheaper than
>>>> incremental building maintaining the sorted invariant) Or was this actually
>>>> better performing than the original data structure choice? (even once moved
>>>> to a per-CU level)
>>>>
>>> I tried the sorted sequence. It was either slower, or ugly.
>>>
>>
>> Curious, but fair enough. (yeah, I suppose the ugly part would be needing
>> an explicit hook into DwarfCompileUnit to say "I've finished adding all the
>> entities, sort them now" (but make sure you don't use them until then,
>> because the sorted invariant won't be available))
>>
>> What if we built the collection directly in the population code, sorted
>> it, then just had "DwarfCompileUnit::setImportedEntities" (& pass the
>> collection by value/move, which should be cheap/free)? Or is that the sort
>> of thing you found to be ugly?
>>
>>
>>> Depends on the variant I tried. As for memory consumption, we'll see if
>>> this change make the situation much worse.
>>> For the reference, linking Chrome with LTO (this patch included)
>>> requires ~32 GB of RAM.
>>>
>>
>> Fair enough
>>
>>
>>>
>>>
>>>>
>>>>
>>>>> +
>>>>> /// GlobalNames - A map of globally visible named entities for this
>>>>> unit.
>>>>> StringMap<const DIE *> GlobalNames;
>>>>>
>>>>> @@ -98,6 +104,10 @@ public:
>>>>>
>>>>> unsigned getOrCreateSourceID(StringRef FileName, StringRef DirName)
>>>>> override;
>>>>>
>>>>> + void addImportedEntity(const DIImportedEntity* IE) {
>>>>> + ImportedEntities[IE->getScope()].push_back(IE);
>>>>> + }
>>>>> +
>>>>> /// addRange - Add an address range to the list of ranges for this
>>>>> unit.
>>>>> void addRange(RangeSpan Range);
>>>>>
>>>>>
>>>>> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
>>>>> URL:
>>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp?rev=251353&r1=251352&r2=251353&view=diff
>>>>>
>>>>> ==============================================================================
>>>>> --- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp (original)
>>>>> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp Mon Oct 26
>>>>> 16:36:35 2015
>>>>> @@ -489,12 +489,7 @@ void DwarfDebug::beginModule() {
>>>>> auto *CUNode = cast<DICompileUnit>(N);
>>>>> DwarfCompileUnit &CU = constructDwarfCompileUnit(CUNode);
>>>>> for (auto *IE : CUNode->getImportedEntities())
>>>>> -
>>>>> ScopesWithImportedEntities.push_back(std::make_pair(IE->getScope(), IE));
>>>>> - // Stable sort to preserve the order of appearance of imported
>>>>> entities.
>>>>> - // This is to avoid out-of-order processing of interdependent
>>>>> declarations
>>>>> - // within the same scope, e.g. { namespace A = base; namespace B
>>>>> = A; }
>>>>> - std::stable_sort(ScopesWithImportedEntities.begin(),
>>>>> - ScopesWithImportedEntities.end(), less_first());
>>>>> + CU.addImportedEntity(IE);
>>>>> for (auto *GV : CUNode->getGlobalVariables())
>>>>> CU.getOrCreateGlobalVariableDIE(GV);
>>>>> for (auto *SP : CUNode->getSubprograms())
>>>>>
>>>>> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h
>>>>> URL:
>>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h?rev=251353&r1=251352&r2=251353&view=diff
>>>>>
>>>>> ==============================================================================
>>>>> --- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h (original)
>>>>> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h Mon Oct 26 16:36:35
>>>>> 2015
>>>>> @@ -289,11 +289,6 @@ class DwarfDebug : public AsmPrinterHand
>>>>> /// Holders for the various debug information flags that we might
>>>>> need to
>>>>> /// have exposed. See accessor functions below for description.
>>>>>
>>>>> - /// Holder for imported entities.
>>>>> - typedef SmallVector<std::pair<const MDNode *, const MDNode *>, 32>
>>>>> - ImportedEntityMap;
>>>>> - ImportedEntityMap ScopesWithImportedEntities;
>>>>> -
>>>>> /// Map from MDNodes for user-defined types to the type units that
>>>>> /// describe them.
>>>>> DenseMap<const MDNode *, const DwarfTypeUnit *> DwarfTypeUnits;
>>>>> @@ -626,14 +621,6 @@ public:
>>>>>
>>>>> const MachineFunction *getCurrentFunction() const { return CurFn; }
>>>>>
>>>>> - iterator_range<ImportedEntityMap::const_iterator>
>>>>> - findImportedEntitiesForScope(const MDNode *Scope) const {
>>>>> - return make_range(std::equal_range(
>>>>> - ScopesWithImportedEntities.begin(),
>>>>> ScopesWithImportedEntities.end(),
>>>>> - std::pair<const MDNode *, const MDNode *>(Scope, nullptr),
>>>>> - less_first()));
>>>>> - }
>>>>> -
>>>>> /// A helper function to check whether the DIE for a given Scope is
>>>>> /// going to be null.
>>>>> bool isLexicalScopeDIENull(LexicalScope *Scope);
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> llvm-commits mailing list
>>>>> llvm-commits at lists.llvm.org
>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>>>>
>>>>
>>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151026/eec60269/attachment.html>
More information about the llvm-commits
mailing list