[PATCH] D80202: [ASTMatchers] Performance optimization for memoization

sammccall added inline comments. Comment at: clang/lib/ASTMatchers/ASTMatchFinder.cpp:902 // Maps (matcher, node) -> the match result for memoization. - typedef std::map MemoizationMap; + typedef std::map> MemoizationMap; MemoizationMap ResultCache;

[PATCH] D80202: [ASTMatchers] Performance optimization for memoization

njames93 updated this revision to Diff 272470. njames93 added a comment. Fix checks failing Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80202/new/ https://reviews.llvm.org/D80202 Files: clang/lib/ASTMatchers/ASTMatchFinder.cpp Index:

[PATCH] D80202: [ASTMatchers] Performance optimization for memoization

klimek added a reviewer: sammccall. klimek added a comment. +Sam for additional sanity checking. Comment at: clang/lib/ASTMatchers/ASTMatchFinder.cpp:902 // Maps (matcher, node) -> the match result for memoization. - typedef std::map MemoizationMap; + typedef std::map>

[PATCH] D80202: [ASTMatchers] Performance optimization for memoization

njames93 updated this revision to Diff 272433. njames93 added a comment. rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80202/new/ https://reviews.llvm.org/D80202 Files: clang/lib/ASTMatchers/ASTMatchFinder.cpp Index:

[PATCH] D80202: [ASTMatchers] Performance optimization for memoization

njames93 added a comment. I decided to do some more stringent benchmarking, just focusing on the matching, not the boilerplate of running clang-tidy. =BeforePatch=== Matcher Timings: 116.0756 user 29.1397 system 145.2154

[PATCH] D80202: [ASTMatchers] Performance optimization for memoization

njames93 added a comment. Running most of the clang tidy checks on the clang-tidy folder yields these results =BeforePatch=== RUN1: 4045.17user 83.93system 11:28.80elapsed 599%CPU (0avgtext+0avgdata 534024maxresident)k

[PATCH] D80202: [ASTMatchers] Performance optimization for memoization

klimek added a comment. In D80202#2046321 , @njames93 wrote: > In D80202#2046266 , @klimek wrote: > > > Given the extra complexity I'd like to see that it matters - bound nodes > > tend to be small. > > > I put

[PATCH] D80202: [ASTMatchers] Performance optimization for memoization

njames93 added a comment. In D80202#2046266 , @klimek wrote: > Given the extra complexity I'd like to see that it matters - bound nodes tend > to be small. I put that in the description, but this is where i need help. whats the best way to benchmark

[PATCH] D80202: [ASTMatchers] Performance optimization for memoization

klimek added a comment. Given the extra complexity I'd like to see that it matters - bound nodes tend to be small. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80202/new/ https://reviews.llvm.org/D80202

[PATCH] D80202: [ASTMatchers] Performance optimization for memoization

grandinj added inline comments. Comment at: clang/lib/ASTMatchers/ASTMatchFinder.cpp:78 + const BoundNodesTreeBuilder + const TraversalKind Traversal; + note that making fields const tends to disable various move optimisations, so better to mark the whole

[PATCH] D80202: [ASTMatchers] Performance optimization for memoization

njames93 created this revision. njames93 added a reviewer: klimek. Herald added a project: clang. Herald added a subscriber: cfe-commits. A simple optimization to avoid unnecessary copies of BoundNodesTreeBuilders when querying the cache for memoized matchers. This hasn't been benchmarked as I'm