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

[Jan Korous via cfe-dev]

Hi,

Sorry for being late to the discussion.

Do I understand it right that you’d like to use a local DeclContext* variable and make sure it’s identical to S.getASTContext() result (or similar for Parent)?

What do you think about caching ASTContext and/or ParentASTContext in Sema?

Jan

> On Dec 17, 2018, at 9:51 AM, Richard Smith via cfe-dev <cfe-dev at lists.llvm.org> wrote:
> 
> On Sun, 16 Dec 2018, 14:27 Bruno Ricci via cfe-dev <cfe-dev at lists.llvm.org <mailto:cfe-dev at lists.llvm.org> wrote:
> On 16/12/2018 18:55, Richard Smith wrote:
> > On Sat, 15 Dec 2018, 09:57 Bruno Ricci via cfe-dev <cfe-dev at lists.llvm.org <mailto:cfe-dev at lists.llvm.org> <mailto:cfe-dev at lists.llvm.org <mailto:cfe-dev at lists.llvm.org>> wrote:
> > 
> >     I have added the profile data to the first patch (https://reviews.llvm.org/D55658 <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.
> 
> Just to clarify, the 1.5%-2% figure is an estimation for the time spent obtaining the
> AST context when parsing all of Boost without doing any code generation (ie: -fsyntax-only).
> 
> I generally agree about not bothering too much with the long tail, with perhaps two exceptions:
> 
> 1.) Some of these are really easy to fix. For example FunctionDecl::setParams is almost only
>     called in Sema and so the context is already easily available.
> 
> Sure, non-disruptive cleanups for this seem good where possible. (But for example, getAttrs is called from a large number of places and it'd be really awkward to require an ASTContext, so I'd leave that one.)
> 
> 2.) The numbers shown above are the number of calls to getTranslationUnitDecl. A better metric
>     would be the number of iterations needed to reach the translation unit declaration. With
>     that in mind it would also make sense to pass the AST context to some of the above where
>     the declaration is likely to occur nested deep in the declaration context chain
>     (VarDecls for example).
> 
> If there are no objections I will submit some patches next week to do this change, hopefully
> in small increments. In any case this will require small (but straightforward) changes to at
> least clang-tools-extra and lldb. (Am I forgetting any project here ?)
> 
> I think it's just those two. Thank you!
> 
> 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 <http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev>
> _______________________________________________
> 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 <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/20190102/3abc9639/attachment.html>