ilya-biryukov added inline comments.
================
Comment at: clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp:53
+/// Combines occurrences of the same symbols across translation units.
+class SymbolMerger {
----------------
ilya-biryukov wrote:
> 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.
Looking at the traces, the threads spend ~6% of `mergeSymbols` time between
`Lock(Mut)` and `for(const Sym...`.
Improving it might be a good idea, but from my point of view it's not too bad
to commit it as is.
Doing it properly is a little complicated in the current setup, since we don't
have knobs to create per-thread structs and merge them later.
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
