[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:12:05 PDT 2015
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/1e678b4f/attachment.html>
More information about the llvm-commits
mailing list