> On Apr 25, 2018, at 12:54 AM, Pavel Labath via Phabricator 
> <revi...@reviews.llvm.org> wrote:
> 
> labath added a comment.
> 
> This code itself looks fine, I have just two minor comments.
> 
> However, I do have a question about performance. I remember us being very 
> worried about performance in the past, so we ended up putting in this like 
> r298876. This removes the normalization step during FileSpec comparison, but 
> it introduces mandatory normalization upon every FileSpec creation, so it's 
> not obvious to me what will this do to performance. Should we try to 
> benchmark this somehow?

I will try debugging LLDB with clang and llvm with debug info and benchmark 
setting setting a file and line breakpoint using a base name to see if there is 
a regression. This will cause all line tables to be created for all compile 
units and all compile unit files in the DWARF line table. I'll let you know 
what I found.
> 
> Jim, you seem to have encountered a case when the normalization was a 
> bottleneck (r298876). Do you remember what situation was that in?

r298876 was for making sure two paths compare correctly and each time you set a 
breakpoint it would normalize off the trailing ".". We will do this once now 
and of course this code form this patch is removed since the normalization will 
take care of it. Setting breakpoints should be faster if I had to guess since 
we are removing the need to check for and normalize a path if it is relative.
> 
> 
> ================
> Comment at: source/Utility/FileSpec.cpp:245
> 
> +  llvm::StringRef resolve_path_ref2(resolved.c_str());
> +
> ----------------
> It looks like this is unused.
> 
> 
> ================
> Comment at: source/Utility/FileSpec.cpp:269-270
> +  else {
> +    // Make sure we don't end up with "." in both the directory and filename.
> +    // If we do, clear the directory. 
> +    m_filename.SetString(".");
> ----------------
> `remove_dots` should never produce a path like this, so we should be able to 
> revert this now.
> 
> 
> https://reviews.llvm.org/D45977
> 
> 
> 

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to