[PATCH] D82771: [ASTMatcher] Fix a performance regression: memorize the child match.

Sam McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Jun 30 04:18:58 PDT 2020


sammccall added a comment.

(I can't quite follow the original reason for disabling memoization in the non-recursive case, so this seems reasonable but going to leave that to Manuel who knew the argument)



================
Comment at: clang/lib/ASTMatchers/ASTMatchFinder.cpp:468
+    // Memoize result even doing a single-level match, it might be expensive.
+    Key.Type = MaxDepth == 1 ? MatchType::Child : MatchType::Descendants;
     MemoizationMap::iterator I = ResultCache.find(Key);
----------------
This line looks a lot like a cache-poisoning bug, until realizing that MaxDepth is never actually used as an integer, it's a boolean - 1 means non-recursive and 0 means recursive. (Note that the recursive call passes MaxDepth, not MaxDepth-1).
I guess we should just fix the type, actually bounding the depth doesn't seem likely to be done anytime soon.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82771/new/

https://reviews.llvm.org/D82771





More information about the cfe-commits mailing list