scott.smith marked an inline comment as done. scott.smith added a comment. In https://reviews.llvm.org/D32820#747309, @clayborg wrote:
> What are the measured improvements here? We can't slow things down on any > platforms. I know MacOS didn't respond well to making demangling run in > parallel. I want to see some numbers here. And please don't quote numbers > with tcmalloc or any special allocator unless you have a patch in LLDB > already to make this a build option. Without tcmalloc, on Ubuntu 14.04, 40 core VM: 13% With tcmalloc, on Ubuntu 14.04, 40 core VM: 24% (built using cmake ... -DCMAKE_EXE_LINKER_FLAGS=-ltcmalloc_minimal, which amazingly only works when building with clang, not gcc...) I don't have access to a Mac, and of course YMMV depending on the application. so yeah, it's a bigger improvement with tcmalloc. Interestingly, I tried adding back the demangler improvements I had queued up (which reduced memory allocations), and they didn't matter much, which makes me think this is contention allocating const strings. I could be wrong though. By far the biggest performance improvement I have queued up is loading the shared libraries in parallel (https://reviews.llvm.org/D32597), but that's waiting on pulling parallel_for_each from LLD into LLVM (I think). If you're really leery of this change, I could just make the structural changes to allow parallelization, and then keep a small patch internally to enable it. Or enabling it could be platform dependent. Or ... ? ================ Comment at: source/Symbol/Symtab.cpp:233-239 + // Don't let trampolines get into the lookup by name map + // If we ever need the trampoline symbols to be searchable by name + // we can remove this and then possibly add a new bool to any of the + // Symtab functions that lookup symbols by name to indicate if they + // want trampolines. + if (symbol.IsTrampoline()) + return; ---------------- clayborg wrote: > Not being able to search for symbols by name when they are trampolines? If > you lookup symbols by name I would expect things not to fail and I would > expect that I would get all the answers, not just ones that are omitted for > performance reasons. I would also not expect to have to specify extra flags. > Please remove This is just moved code, not new code. You can use the phabricator history tool above to diff baseline against #97908, and see that the only change I made was continue->return, due to changing it from a loop to a lambda (now a separate function). This is why I pubished D32708 separately - I wanted to separate the functional change from the structural change. Repository: rL LLVM https://reviews.llvm.org/D32820 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
