Bypass GetNormalizedFile when not necessary...

[jingham]

Summary

Something in the changes in FileSpec::Equal and more particularly FileSpec::GetNormalizedPath over the past 3 or 4 months have made the file comparisons that happen when we're matching file & line breakpoints against loaded libraries much slower. I have reports of setting 50 breakpoints on a fairly complex program with ~10000 source files now takes many minutes, and the sample shows almost all the time spent in GetNormalizedFile.

I still have to figure out what got slow here, but in the near term, the FileSpec::Equals algorithm was computing the normalized path in many cases where it didn't need to. The only way two FileSpecs can be equal if their filenames are not equal is if one or the other filename is "." or "..".

This patch takes advantage of that observation to short cut all compares with unequal filenames that aren't "." or "..". Since almost all file compares done when setting breakpoints are for unequal filenames, this should reduce the time spent setting breakpoint.

[clayborg]

Since we are going for speed here, I added a few suggestions for making it quicker.

source/Utility/FileSpec.cpp

441–443

Add a return of false here and avoid the whole "last_component_is_dot" stuff below?

444

Can we look at remove_backups here and avoid the last_component_is_dot functionality if it is false?

[jingham]

How does this look?

[labath]

Seems legit.

I've added unit tests for this method when I last touched it, but it looks like there are no cases which test the "last component is dot" case. Would you mind adding one or two of these?

[clayborg]

Much better.

[jingham]

Closed by r298876

source/Utility/FileSpec.cpp

if (full || (a.m_directory && b.m_directory)) {
     if (result)
       return result;
   }
   return ConstString::Compare(a.m_filename, b.m_filename, case_sensitive);
 }
 
 bool FileSpec::Equal(const FileSpec &a, const FileSpec &b, bool full,
                      bool remove_backups) {
+  static ConstString g_dot_string(".");
+  static ConstString g_dot_dot_string("..");
+
   // case sensitivity of equality test
   const bool case_sensitive = a.IsCaseSensitive() || b.IsCaseSensitive();
   
+  bool filenames_equal = ConstString::Equals(a.m_filename, 
+                                             b.m_filename, 
+                                             case_sensitive);
+
+  // The only way two FileSpecs can be equal if their filenames are
+  // unequal is if we are removing backups and one or the other filename
+  // is a backup string:
+
+  if (!filenames_equal && !remove_backups)
+      return false;
+
+  bool last_component_is_dot = ConstString::Equals(a.m_filename, g_dot_string) 
+                               || ConstString::Equals(a.m_filename, 
+                                                      g_dot_dot_string)
+                               || ConstString::Equals(b.m_filename, 
+                                                      g_dot_string)
+                               || ConstString::Equals(b.m_filename, 
+                                                      g_dot_dot_string);
+
+  if (!filenames_equal && !last_component_is_dot)
+    return false;
+
   if (!full && (a.GetDirectory().IsEmpty() || b.GetDirectory().IsEmpty()))
-    return ConstString::Equals(a.m_filename, b.m_filename, case_sensitive);
+    return filenames_equal;
 
   if (remove_backups == false)
     return a == b;
 
   if (a == b)
     return true;
 
   return Equal(a.GetNormalizedPath(), b.GetNormalizedPath(), full, false);
 }
 
 FileSpec FileSpec::GetNormalizedPath() const {
   // Fast path. Do nothing if the path is not interesting.
   if (!m_directory.GetStringRef().contains(".") &&
       !m_directory.GetStringRef().contains("//") &&
       m_filename.GetStringRef() != ".." && m_filename.GetStringRef() != ".")
     return *this;
 
   llvm::SmallString<64> path, result;
   const bool normalize = false;