ilya-biryukov added a comment.
Herald added a subscriber: jkorous.
In https://reviews.llvm.org/D45478#1064027, @sammccall wrote:
> Is this patch still relevant after haojian's string deduplication?
Apparently it does. It has an advantage of distributing the work more evenly
between the program runs.
Currently the tool reports progress based on the number of files it parsed. But
then it takes a lot of time to actually merge all the symbols at the end and
the progress is not reported during that time.
The perfect behavior would be to remove duplicates, not just intern them :-)
That shouldn't be too hard and it probably even makes sense to have a
ToolExecutor that does that.
================
Comment at: clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp:53
+/// Combines occurrences of the same symbols across translation units.
+class SymbolMerger {
----------------
sammccall wrote:
> Seems reasonably likely we would actually have contention here? merging
> per-thread (combiner) and then globally at the end (reducer) might be the way
> to go (might be significantly faster). But not sure how big the impact is.
I haven't seen any noticeable performance degradation after this patch.
There should be contention, but it doesn't seem to matter much, as parsing
still takes way more time.
Still makes sense to rewrite the code in order to avoid contention, will do
that.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D45478
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
