bruno added a comment.
Thanks for working on this Troy, nice speed up.
A bit more context for reviewers: we're probably unique on how we use hmaps, we
have tons of them in a single compiler invocation. The other users of hmaps
I've seen don't use more than a handful. This means that general users
shouldn't expect much disruption here.
================
Comment at: clang/include/clang/Lex/HeaderMap.h:52
+ Optional<StringRef> Key = getString(B.Key);
+ if (Key) {
+ Callback(Key.value());
----------------
Please remove the curly braces for this `if` and similar ones in the rest of
the patch (if present).
================
Comment at: clang/include/clang/Lex/HeaderSearch.h:255
+ /// lets us avoid scanning them all to find a match.
+ llvm::StringMap<unsigned, llvm::BumpPtrAllocator> SearchDirHeaderMapIndex;
+
----------------
Did you end up getting data on the memory footprint of this map? It seems to me
that it should be fine if header maps aren't much populated.
================
Comment at: clang/lib/Lex/HeaderSearch.cpp:391
+ auto Callback = [&](StringRef Filename) { Index.try_emplace(Filename, i);
};
+
+ Dir.getHeaderMap()->forEachKey(Callback);
----------------
Remove this empty line.
================
Comment at: clang/lib/Lex/HeaderSearch.cpp:392
+
+ Dir.getHeaderMap()->forEachKey(Callback);
+ }
----------------
What happens if different header maps contain the same header name?
================
Comment at: clang/lib/Lex/HeaderSearch.cpp:1035
} else {
- // Otherwise, this is the first query, or the previous query didn't match
- // our search start. We will fill in our found location below, so prime
the
- // start point value.
CacheLookup.reset(/*NewStartIt=*/NextIt);
}
----------------
Looks like we could have only one call to `CacheLookup.reset` instead?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D135801/new/
https://reviews.llvm.org/D135801
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
