Sorry for not chiming in earlier. A few nits. On Wed, Oct 15, 2014 at 4:58 PM, Samuel Benzaquen <[email protected]> wrote:
> Author: sbenza > Date: Wed Oct 15 09:58:46 2014 > New Revision: 219792 > > URL: http://llvm.org/viewvc/llvm-project?rev=219792&view=rev > Log: > Speed up hasName() matcher. > > Summary: > Speed up hasName() matcher by skipping the expensive generation of the > fully qualified name unless we need it. > In the common case of matching an unqualified name, we don't need to > generate the full name. We might not even need to copy any string at > all. > This change speeds up our clang-tidy benchmark by ~10% > > Reviewers: klimek > > Subscribers: klimek, cfe-commits > > Differential Revision: http://reviews.llvm.org/D5776 > > Modified: > cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h > cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h > cfe/trunk/lib/ASTMatchers/ASTMatchersInternal.cpp > > Modified: cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h?rev=219792&r1=219791&r2=219792&view=diff > > ============================================================================== > --- cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h (original) > +++ cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h Wed Oct 15 09:58:46 > 2014 > @@ -1591,16 +1591,8 @@ inline internal::Matcher<Stmt> sizeOfExp > /// \code > /// namespace a { namespace b { class X; } } > /// \endcode > -AST_MATCHER_P(NamedDecl, hasName, std::string, Name) { > - assert(!Name.empty()); > - const std::string FullNameString = "::" + > Node.getQualifiedNameAsString(); > - const StringRef FullName = FullNameString; > - const StringRef Pattern = Name; > - if (Pattern.startswith("::")) { > - return FullName == Pattern; > - } else { > - return FullName.endswith(("::" + Pattern).str()); > - } > +inline internal::Matcher<NamedDecl> hasName(StringRef Name) { > + return internal::Matcher<NamedDecl>(new internal::HasNameMatcher(Name)); > } > > /// \brief Matches NamedDecl nodes whose fully qualified names contain > > Modified: cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h?rev=219792&r1=219791&r2=219792&view=diff > > ============================================================================== > --- cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h (original) > +++ cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h Wed Oct 15 > 09:58:46 2014 > @@ -574,6 +574,33 @@ private: > std::string Name; > }; > > +/// \brief Matches named declarations with a specific name. > +/// > +/// See \c hasName() in ASTMatchers.h for details. > +class HasNameMatcher : public SingleNodeMatcherInterface<NamedDecl> { > + public: > + explicit HasNameMatcher(StringRef Name); > + > + bool matchesNode(const NamedDecl &Node) const override; > + > + private: > + /// \brief Unqualified match routine. > + /// > + /// It is much faster than the full match, but it only works for > unqualified > + /// matches. > + bool matchesNodeUnqualified(const NamedDecl &Node) const; > + > + /// \brief Full match routine > + /// > + /// It generates the fully qualified name of the declaration (which is > + /// expensive) before trying to match. > + /// It is slower but simple and works on all cases. > + bool matchesNodeFull(const NamedDecl &Node) const; > + > + const bool UseUnqualifiedMatch; > + const std::string Name; > +}; > + > /// \brief Matches declarations for QualType and CallExpr. > /// > /// Type argument DeclMatcherT is required by > PolymorphicMatcherWithParam1 but > > Modified: cfe/trunk/lib/ASTMatchers/ASTMatchersInternal.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/ASTMatchers/ASTMatchersInternal.cpp?rev=219792&r1=219791&r2=219792&view=diff > > ============================================================================== > --- cfe/trunk/lib/ASTMatchers/ASTMatchersInternal.cpp (original) > +++ cfe/trunk/lib/ASTMatchers/ASTMatchersInternal.cpp Wed Oct 15 09:58:46 > 2014 > @@ -13,6 +13,7 @@ > > #include "clang/ASTMatchers/ASTMatchers.h" > #include "clang/ASTMatchers/ASTMatchersInternal.h" > +#include "llvm/ADT/SmallString.h" > #include "llvm/Support/ManagedStatic.h" > > namespace clang { > @@ -221,6 +222,51 @@ bool AnyOfVariadicOperator(const ast_typ > return false; > } > > +HasNameMatcher::HasNameMatcher(StringRef NameRef) > + : UseUnqualifiedMatch(NameRef.find("::") == NameRef.npos), > Name(NameRef) { > + assert(!Name.empty()); > +} > + > +bool HasNameMatcher::matchesNodeUnqualified(const NamedDecl &Node) const { > + assert(UseUnqualifiedMatch); > + if (Node.getIdentifier()) { > + // Simple name. > + return Name == Node.getName(); > + } else if (Node.getDeclName()) { > http://llvm.org/docs/CodingStandards.html#don-t-use-else-after-a-return > + // Name needs to be constructed. > + llvm::SmallString<128> NodeName; > + llvm::raw_svector_ostream OS(NodeName); > + Node.printName(OS); > + return Name == OS.str(); > + } else { > http://llvm.org/docs/CodingStandards.html#don-t-use-else-after-a-return > + return false; > + } > +} > + > +bool HasNameMatcher::matchesNodeFull(const NamedDecl &Node) const { > + llvm::SmallString<128> NodeName = StringRef("::"); > + llvm::raw_svector_ostream OS(NodeName); > + Node.printQualifiedName(OS); > + const StringRef FullName = OS.str(); > + const StringRef Pattern = Name; > + if (Pattern.startswith("::")) { > + return FullName == Pattern; > + } else { > http://llvm.org/docs/CodingStandards.html#don-t-use-else-after-a-return > + return FullName.endswith(("::" + Pattern).str()); > If this shows on the profile, we can also avoid creating a temporary string here: return FullName.endswith(Pattern) && FullName.drop_back(Pattern.size()).endswith("::"); > + } > +} > + > +bool HasNameMatcher::matchesNode(const NamedDecl &Node) const { > + // FIXME: There is still room for improvement, but it would require > copying a > + // lot of the logic from NamedDecl::printQualifiedName(). The > benchmarks do > + // not show like that extra complexity is needed right now. > + if (UseUnqualifiedMatch) { > + assert(matchesNodeUnqualified(Node) == matchesNodeFull(Node)); > + return matchesNodeUnqualified(Node); > + } > + return matchesNodeFull(Node); > +} > + > } // end namespace internal > } // end namespace ast_matchers > } // end namespace clang > > > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
