<div dir="auto"><div><div class="gmail_quote"><div dir="ltr">On Sun, 16 Dec 2018, 14:27 Bruno Ricci via cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org">cfe-dev@lists.llvm.org</a> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On 16/12/2018 18:55, Richard Smith wrote:<br>
> On Sat, 15 Dec 2018, 09:57 Bruno Ricci via cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org" target="_blank" rel="noreferrer">cfe-dev@lists.llvm.org</a> <mailto:<a href="mailto:cfe-dev@lists.llvm.org" target="_blank" rel="noreferrer">cfe-dev@lists.llvm.org</a>> wrote:<br>
> <br>
>     I have added the profile data to the first patch (<a href="https://reviews.llvm.org/D55658" rel="noreferrer noreferrer" target="_blank">https://reviews.llvm.org/D55658</a>)<br>
> <br>
>     The top 20 callers of getASTContext and getParentASTContext are:<br>
>     (with the number of calls to getTranslationUnitDecl, edited for readability)<br>
> <br>
>     5399586 : DeclContext::lookup<br>
>     801684 : VarDecl::getMemberSpecializationInfo<br>
>     551928 : FunctionDecl::getMinRequiredArguments<br>
>     401464 : CXXRecordDecl::lookupInBases<br>
>     338614 : Decl::getAttrs<br>
>     311651 : DeclContext::makeDeclVisibleInContextImpl<br>
>     173684 : Decl::getASTMutationListener<br>
>     164264 : CXXConstructorDecl::isSpecializationCopyingObject<br>
>     154066 : CXXMethodDecl::size_overridden_methods<br>
>     141622 : CXXRecordDecl::getVisibleConversionFunctions<br>
>     120440 : ClassTemplateSpecializationDecl::Profile<br>
>     116303 : VarDecl::isThisDeclarationADefinition<br>
>     106208 : CXXRecordDecl::conversion_begin<br>
>     106208 : CXXRecordDecl::conversion_end<br>
>     99075 : FunctionDecl::setParams<br>
>     95910 : RedeclarableTemplateDecl::findSpecializationImpl<br>
>     57043 : CXXRecordDecl::getDestructor<br>
>     56668 : VarDecl::getDefinition<br>
>     52140 : TagDecl::startDefinition<br>
>     48266 : CXXConstructorDecl::isCopyOrMoveConstructor<br>
>     37407 : FunctionDecl::getBody<br>
> <br>
> <br>
> 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.<br>
<br>
Just to clarify, the 1.5%-2% figure is an estimation for the time spent obtaining the<br>
AST context when parsing all of Boost without doing any code generation (ie: -fsyntax-only).<br>
<br>
I generally agree about not bothering too much with the long tail, with perhaps two exceptions:<br>
<br>
1.) Some of these are really easy to fix. For example FunctionDecl::setParams is almost only<br>
    called in Sema and so the context is already easily available.<br></blockquote></div></div><div dir="auto"><br></div><div dir="auto">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.)</div><div dir="auto"><br></div><div dir="auto"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
2.) The numbers shown above are the number of calls to getTranslationUnitDecl. A better metric<br>
    would be the number of iterations needed to reach the translation unit declaration. With<br>
    that in mind it would also make sense to pass the AST context to some of the above where<br>
    the declaration is likely to occur nested deep in the declaration context chain<br>
    (VarDecls for example).<br>
<br>
If there are no objections I will submit some patches next week to do this change, hopefully<br>
in small increments. In any case this will require small (but straightforward) changes to at<br>
least clang-tools-extra and lldb. (Am I forgetting any project here ?)<br></blockquote></div></div><div dir="auto"><br></div><div dir="auto">I think it's just those two. Thank you!</div><div dir="auto"><br></div><div dir="auto"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Bruno<br>
_______________________________________________<br>
cfe-dev mailing list<br>
<a href="mailto:cfe-dev@lists.llvm.org" target="_blank" rel="noreferrer">cfe-dev@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev" rel="noreferrer noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev</a><br>
</blockquote></div></div></div>