<html><head><meta http-equiv="Content-Type" content="text/html; charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class=""><div class="">Hi,</div><div class=""><br class=""></div>Sorry for being late to the discussion.<div class=""><br class=""></div><div class="">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)?</div><div class=""><br class=""></div><div class="">What do you think about caching ASTContext and/or ParentASTContext in Sema?</div><div class=""><br class=""></div><div class="">Jan</div><div class=""><br class=""></div><div class=""><div><blockquote type="cite" class=""><div class="">On Dec 17, 2018, at 9:51 AM, Richard Smith via cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org" class="">cfe-dev@lists.llvm.org</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="auto" style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 13px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><div class=""><div class="gmail_quote"><div dir="ltr" class="">On Sun, 16 Dec 2018, 14:27 Bruno Ricci via cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org" class="">cfe-dev@lists.llvm.org</a><span class="Apple-converted-space"> </span>wrote:<br class=""></div><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-style: solid; border-left-color: rgb(204, 204, 204); padding-left: 1ex;">On 16/12/2018 18:55, Richard Smith wrote:<br class="">> On Sat, 15 Dec 2018, 09:57 Bruno Ricci via cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org" target="_blank" rel="noreferrer" class="">cfe-dev@lists.llvm.org</a><span class="Apple-converted-space"> </span><mailto:<a href="mailto:cfe-dev@lists.llvm.org" target="_blank" rel="noreferrer" class="">cfe-dev@lists.llvm.org</a>> wrote:<br class="">><span class="Apple-converted-space"> </span><br class="">> I have added the profile data to the first patch (<a href="https://reviews.llvm.org/D55658" rel="noreferrer noreferrer" target="_blank" class="">https://reviews.llvm.org/D55658</a>)<br class="">><span class="Apple-converted-space"> </span><br class="">> The top 20 callers of getASTContext and getParentASTContext are:<br class="">> (with the number of calls to getTranslationUnitDecl, edited for readability)<br class="">><span class="Apple-converted-space"> </span><br class="">> 5399586 : DeclContext::lookup<br class="">> 801684 : VarDecl::getMemberSpecializationInfo<br class="">> 551928 : FunctionDecl::getMinRequiredArguments<br class="">> 401464 : CXXRecordDecl::lookupInBases<br class="">> 338614 : Decl::getAttrs<br class="">> 311651 : DeclContext::makeDeclVisibleInContextImpl<br class="">> 173684 : Decl::getASTMutationListener<br class="">> 164264 : CXXConstructorDecl::isSpecializationCopyingObject<br class="">> 154066 : CXXMethodDecl::size_overridden_methods<br class="">> 141622 : CXXRecordDecl::getVisibleConversionFunctions<br class="">> 120440 : ClassTemplateSpecializationDecl::Profile<br class="">> 116303 : VarDecl::isThisDeclarationADefinition<br class="">> 106208 : CXXRecordDecl::conversion_begin<br class="">> 106208 : CXXRecordDecl::conversion_end<br class="">> 99075 : FunctionDecl::setParams<br class="">> 95910 : RedeclarableTemplateDecl::findSpecializationImpl<br class="">> 57043 : CXXRecordDecl::getDestructor<br class="">> 56668 : VarDecl::getDefinition<br class="">> 52140 : TagDecl::startDefinition<br class="">> 48266 : CXXConstructorDecl::isCopyOrMoveConstructor<br class="">> 37407 : FunctionDecl::getBody<br class="">><span class="Apple-converted-space"> </span><br class="">><span class="Apple-converted-space"> </span><br class="">> 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 class=""><br class="">Just to clarify, the 1.5%-2% figure is an estimation for the time spent obtaining the<br class="">AST context when parsing all of Boost without doing any code generation (ie: -fsyntax-only).<br class=""><br class="">I generally agree about not bothering too much with the long tail, with perhaps two exceptions:<br class=""><br class="">1.) Some of these are really easy to fix. For example FunctionDecl::setParams is almost only<br class=""> <span class="Apple-converted-space"> </span>called in Sema and so the context is already easily available.<br class=""></blockquote></div></div><div dir="auto" class=""><br class=""></div><div dir="auto" class="">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" class=""><br class=""></div><div dir="auto" class=""><div class="gmail_quote"><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-style: solid; border-left-color: rgb(204, 204, 204); padding-left: 1ex;">2.) The numbers shown above are the number of calls to getTranslationUnitDecl. A better metric<br class=""> <span class="Apple-converted-space"> </span>would be the number of iterations needed to reach the translation unit declaration. With<br class=""> <span class="Apple-converted-space"> </span>that in mind it would also make sense to pass the AST context to some of the above where<br class=""> <span class="Apple-converted-space"> </span>the declaration is likely to occur nested deep in the declaration context chain<br class=""> <span class="Apple-converted-space"> </span>(VarDecls for example).<br class=""><br class="">If there are no objections I will submit some patches next week to do this change, hopefully<br class="">in small increments. In any case this will require small (but straightforward) changes to at<br class="">least clang-tools-extra and lldb. (Am I forgetting any project here ?)<br class=""></blockquote></div></div><div dir="auto" class=""><br class=""></div><div dir="auto" class="">I think it's just those two. Thank you!</div><div dir="auto" class=""><br class=""></div><div dir="auto" class=""><div class="gmail_quote"><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-style: solid; border-left-color: rgb(204, 204, 204); padding-left: 1ex;">Bruno<br class="">_______________________________________________<br class="">cfe-dev mailing list<br class=""><a href="mailto:cfe-dev@lists.llvm.org" target="_blank" rel="noreferrer" class="">cfe-dev@lists.llvm.org</a><br class=""><a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev" rel="noreferrer noreferrer" target="_blank" class="">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev</a><br class=""></blockquote></div></div></div><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 13px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">_______________________________________________</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 13px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 13px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">cfe-dev mailing list</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 13px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><a href="mailto:cfe-dev@lists.llvm.org" style="font-family: Helvetica; font-size: 13px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;" class="">cfe-dev@lists.llvm.org</a><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 13px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev" style="font-family: Helvetica; font-size: 13px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;" class="">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev</a></div></blockquote></div><br class=""></div></body></html>