<html><head><meta http-equiv="Content-Type" content="text/html; charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class=""><br class=""><div><br class=""><blockquote type="cite" class=""><div class="">On Apr 25, 2018, at 12:54 AM, Pavel Labath via Phabricator <<a href="mailto:reviews@reviews.llvm.org" class="">reviews@reviews.llvm.org</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div class="">labath added a comment.<br class=""><br class="">This code itself looks fine, I have just two minor comments.<br class=""><br class="">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?<br class=""></div></div></blockquote><div><br class=""></div>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.<br class=""><blockquote type="cite" class=""><div class=""><div class=""><br class="">Jim, you seem to have encountered a case when the normalization was a bottleneck (r298876). Do you remember what situation was that in?<br class=""></div></div></blockquote><div><br class=""></div><font face="Menlo-Regular" class="">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.</font><br class=""><blockquote type="cite" class=""><div class=""><div class=""><br class=""><br class="">================<br class="">Comment at: source/Utility/FileSpec.cpp:245<br class=""><br class="">+ llvm::StringRef resolve_path_ref2(resolved.c_str());<br class="">+<br class="">----------------<br class="">It looks like this is unused.<br class=""><br class=""><br class="">================<br class="">Comment at: source/Utility/FileSpec.cpp:269-270<br class="">+ else {<br class="">+ // Make sure we don't end up with "." in both the directory and filename.<br class="">+ // If we do, clear the directory. <br class="">+ m_filename.SetString(".");<br class="">----------------<br class="">`remove_dots` should never produce a path like this, so we should be able to revert this now.<br class=""><br class=""><br class=""><a href="https://reviews.llvm.org/D45977" class="">https://reviews.llvm.org/D45977</a><br class=""><br class=""><br class=""><br class=""></div></div></blockquote></div><br class=""></body></html>