[cfe-dev] Avoiding unnecessary calls to Decl::getASTContext

[Richard Smith via cfe-dev]

On Sat, 15 Dec 2018, 09:57 Bruno Ricci via cfe-dev <cfe-dev at lists.llvm.org
wrote:

> I have added the profile data to the first patch (
> https://reviews.llvm.org/D55658)
>
> The top 20 callers of getASTContext and getParentASTContext are:
> (with the number of calls to getTranslationUnitDecl, edited for
> readability)
>
> 5399586 : DeclContext::lookup
> 801684 : VarDecl::getMemberSpecializationInfo
> 551928 : FunctionDecl::getMinRequiredArguments
> 401464 : CXXRecordDecl::lookupInBases
> 338614 : Decl::getAttrs
> 311651 : DeclContext::makeDeclVisibleInContextImpl
> 173684 : Decl::getASTMutationListener
> 164264 : CXXConstructorDecl::isSpecializationCopyingObject
> 154066 : CXXMethodDecl::size_overridden_methods
> 141622 : CXXRecordDecl::getVisibleConversionFunctions
> 120440 : ClassTemplateSpecializationDecl::Profile
> 116303 : VarDecl::isThisDeclarationADefinition
> 106208 : CXXRecordDecl::conversion_begin
> 106208 : CXXRecordDecl::conversion_end
> 99075 : FunctionDecl::setParams
> 95910 : RedeclarableTemplateDecl::findSpecializationImpl
> 57043 : CXXRecordDecl::getDestructor
> 56668 : VarDecl::getDefinition
> 52140 : TagDecl::startDefinition
> 48266 : CXXConstructorDecl::isCopyOrMoveConstructor
> 37407 : FunctionDecl::getBody
>

Given your data that perhaps 1.5-2% of the time building boost is finding
the AST context, it definitely seems worthwhile to avoid doing so for
lookup. The next three seem reasonable to address too, if you feel so
inclined, but I'd not go any further than that -- the long tail seems
negligible in comparison.

Bruno
>
> On 15/12/2018 16:38, David Blaikie wrote:
> > Seems reasonable and some profile data would help
> motivate/support/justify which bits are worth changing and which aren't
> >
> > On Sat, Dec 15, 2018, 8:27 AM Bruno Ricci via cfe-dev <
> cfe-dev at lists.llvm.org <mailto:cfe-dev at lists.llvm.org> wrote:
> >
> >     Hi all,
> >
> >     I have been looking at Decl::getASTContext and
> DeclContext::getParentASTContext,
> >     and it turns out that they are not that cheap.
> >
> >     The reason for this is that they must walk back to the translation
> unit declaration
> >     along what is essentially a linked list. This is amplified by the
> fact that it does
> >     look like a simple getter and therefore is called extremely often
> (the most egregious
> >     case by far being DeclContext::lookup), even when most of the times
> the context is
> >     already easily available.
> >
> >     To put some numbers on this I experimented with removing about half
> of the iterations
> >     in Decl::getTranslationUnitDecl. This gives a speed up of about 1%
> when parsing all
> >     of Boost. Extrapolating from this I get that about 1.5%-2% of the
> time spent
> >     parsing Boost is spent getting the ASTContext, which seems a bit
> unfortunate.
> >
> >     Now to be clear 1.5%-2% is not that much, but the fix is simply to
> pass a ref to
> >     the ASTContext more often as a function parameter. The downside is
> that this would
> >     cause a some amount of churn in the C++ api. To be clear I am *not*
> suggesting
> >     removing all of the calls to Decl::getASTContext, but only removing
> the ones causing
> >     the most iterations in Decl::getTranslationUnitDecl (say the top 10
> or 20).
> >
> >     What do you think ?
> >
> >     Bruno
> >     _______________________________________________
> >     cfe-dev mailing list
> >     cfe-dev at lists.llvm.org <mailto:cfe-dev at lists.llvm.org>
> >     http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
> >
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20181216/64d2cfea/attachment.html>