troyj added inline comments.
================
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;
+
----------------
bruno wrote:
> 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.
Right, with ~15000 header maps, this map has ~92000 entries, so each map
contributes about 6 unique keys. The patch does not increase the peak memory
usage of Clang.
================
Comment at: clang/lib/Lex/HeaderSearch.cpp:392
+
+ Dir.getHeaderMap()->forEachKey(Callback);
+ }
----------------
bruno wrote:
> What happens if different header maps contain the same header name?
>
The first one sticks because try_emplace doesn't insert if the key already
exists. I'll add a comment.
================
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);
}
----------------
bruno wrote:
> Looks like we could have only one call to `CacheLookup.reset` instead?
The reset call has a side effect of setting CacheLookup.MappedName to nullptr,
so we can't call it unconditionally because the Hit case should not call it.
The unpatched code combined the !SkipCache check and the Hit check, so it still
called reset when SkipCache == true. I preserved that behavior because I think
it's necessary. SkipCache seems to imply "don't consult the cache for this
call" instead of "don't cache anything" so it still caches the result for a
future LookupFile call that passes SkipCache == false.
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
