[base::FilePath] Optimize FilePath::ReferencesParent

base/files/file_path.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/files/file_path.h"
 
 #include <string.h>
 #include <algorithm>
 
 #include "base/logging.h"
@@ skipping 542 matching lines @@
 }
 
 FilePath FilePath::StripTrailingSeparators() const {
   FilePath new_path(path_);
   new_path.StripTrailingSeparatorsInternal();
 
   return new_path;
 }
 
 bool FilePath::ReferencesParent() const {
+  if (path_.find(kParentDirectory) == StringType::npos) {
+    // GetComponents is quite expensive, so avoid calling it in the majority
+    // of cases where there isn't a kParentDirectory anywhere in the path.
+    return false;
+  }
+
   std::vector<StringType> components;

[danakj, 2016/11/11 19:17:40]

This is great!

Question tho: We should probably use StringPiece instead StringType here too, so
we don't have to do all this mallocing and copying to do GetComponents. Would
that give the same wins in your case but not need an extra walk over the string?

[jkarlin, 2016/11/11 19:38:24]

Are you suggesting that GetComponents should take a vector<StringPieceType> and
fill it accordingly? That would be better, but it's non-trivial as GetComponents
depends on BaseName() and DirName() which both return FilePath types.

We could also rewrite this function to just walk down the path scanning for
'..'s and ignoring the whitespace on either side. That would result in a fairly
fast function, but I bet you that average case this function is still faster. I
don't think we're going to see '..' in paths very often.

[danakj, 2016/11/11 19:57:27]

Ah ok, it's not trivial at all np. This LGTM!
For most users I think you're right. Maybe someone will have a .. in their
XDG_CONFIG_PATH or something and then they'll hit it all the time. But it seems
corner-casey.

   GetComponents(&components);
 
   std::vector<StringType>::const_iterator it = components.begin();
   for (; it != components.end(); ++it) {
     const StringType& component = *it;
     // Windows has odd, undocumented behavior with path components containing
     // only whitespace and . characters. So, if all we see is . and
     // whitespace, then we treat any .. sequence as referencing parent.
     // For simplicity we enforce this on all platforms.
     if (component.find_first_not_of(FILE_PATH_LITERAL(". \n\r\t")) ==
@@ skipping 753 matching lines @@
 #endif
 }
 
 #if defined(OS_ANDROID)
 bool FilePath::IsContentUri() const {
   return StartsWith(path_, "content://", base::CompareCase::INSENSITIVE_ASCII);
 }
 #endif
 
 }  // namespace base