Merge pull request #2268 from stephentoub/pathcombinethree

Remove string allocation/copy from Path.Combine

Author: stephentoub

src/System.Runtime.Extensions/src/System/IO/Path.cs

@@ -311,7 +311,7 @@ public static string Combine(string path1, string path2, string path3)
             CheckInvalidPathChars(path2);
             CheckInvalidPathChars(path3);
 
-            return CombineNoChecks(CombineNoChecks(path1, path2), path3);
+            return CombineNoChecks(path1, path2, path3);
         }
 
         public static string Combine(params string[] paths)
@@ -402,6 +402,49 @@ private static string CombineNoChecks(string path1, string path2)
                 path1 + DirectorySeparatorCharAsString + path2;
         }
 
+        private static string CombineNoChecks(string path1, string path2, string path3)
+        {
+            if (path1.Length == 0)
+                return CombineNoChecks(path2, path3);
+            if (path2.Length == 0)
+                return CombineNoChecks(path1, path3);
+            if (path3.Length == 0)
+                return CombineNoChecks(path1, path2);
+
+            if (IsPathRooted(path3))
+                return path3;
+            if (IsPathRooted(path2))
+                return CombineNoChecks(path2, path3);
+
+            bool hasSep1 = IsDirectoryOrVolumeSeparator(path1[path1.Length - 1]);
+            bool hasSep2 = IsDirectoryOrVolumeSeparator(path2[path2.Length - 1]);
+
+            if (hasSep1 && hasSep2)
+            {
+                return path1 + path2 + path3;
+            }
+            else if (hasSep1)
+            {
+                return path1 + path2 + DirectorySeparatorCharAsString + path3;
+            }
+            else if (hasSep2)
+            {
+                return path1 + DirectorySeparatorCharAsString + path2 + path3;
+            }
+            else
+            {
+                // string.Concat only has string-based overloads up to four arguments; after that requires allocating
+                // a params string[].  Instead, try to use a cached StringBuilder.
+                StringBuilder sb = StringBuilderCache.Acquire(path1.Length + path2.Length + path3.Length + 2);
+                sb.Append(path1)
+                  .Append(DirectorySeparatorChar)
+                  .Append(path2)
+                  .Append(DirectorySeparatorChar)
+                  .Append(path3);
+                return StringBuilderCache.GetStringAndRelease(sb);
+            }
+        }
+
         private static readonly char[] s_Base32Char = {
                 'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h',
                 'i', 'j', 'k', 'l', 'm', 'n', 'o', 'p',

src/System.Runtime.Extensions/tests/System/IO/Path.Combine.cs

@@ -51,8 +51,15 @@ public static void PathIsNullWihtoutRootedAfterArgumentNull()
     }
 
     [Fact]
-    [PlatformSpecific(PlatformID.Windows)]
     public static void ContainsInvalidCharWithoutRootedAfterArgumentNull()
+    {
+        //any path contains invalid character without rooted after (AE)
+        CommonCasesException<ArgumentException>("ab\0cd");
+    }
+
+    [Fact]
+    [PlatformSpecific(PlatformID.Windows)]
+    public static void ContainsInvalidCharWithoutRootedAfterArgumentNull_Windows()
     {
         //any path contains invalid character without rooted after (AE)
         CommonCasesException<ArgumentException>("ab\"cd");
@@ -66,16 +73,23 @@ public static void ContainsInvalidCharWithoutRootedAfterArgumentNull()
     }
 
     [Fact]
-    [PlatformSpecific(PlatformID.Windows)]
     public static void ContainsInvalidCharWithRootedAfterArgumentNull()
+    {
+        //any path contains invalid character with rooted after (AE)
+        CommonCasesException<ArgumentException>("ab\0cd", s_separator + "abc");
+    }
+
+
+    [Fact]
+    [PlatformSpecific(PlatformID.Windows)]
+    public static void ContainsInvalidCharWithRootedAfterArgumentNull_Windows()
     {
         //any path contains invalid character with rooted after (AE)
         CommonCasesException<ArgumentException>("ab\"cd", s_separator + "abc");
         CommonCasesException<ArgumentException>("ab<cd", s_separator + "abc");
         CommonCasesException<ArgumentException>("ab>cd", s_separator + "abc");
         CommonCasesException<ArgumentException>("ab|cd", s_separator + "abc");
         CommonCasesException<ArgumentException>("ab\bcd", s_separator + "abc");
-        CommonCasesException<ArgumentException>("ab\0cd", s_separator + "abc");
         CommonCasesException<ArgumentException>("ab\tcd", s_separator + "abc");
     }
 
@@ -110,6 +124,7 @@ public static void PathIsSingleElement()
     {
         //any path is single element
         CommonCases("abc");
+        CommonCases("abc" + s_separator);
     }
 
     [Fact]
@@ -119,10 +134,13 @@ public static void PathIsMultipleElements()
         CommonCases(Path.Combine("abc", Path.Combine("def", "ghi")));
     }
 
-    public static void NoPathIsRooted()
+    [Fact]
+    public static void PathElementsAllSeparated()
     {
-        //no path is rooted
-        CommonCases("abc");
+        Verify(new string[] { "abc" + s_separator, "def" + s_separator });
+        Verify(new string[] { "abc" + s_separator, "def" + s_separator, "ghi" + s_separator });
+        Verify(new string[] { "abc" + s_separator, "def" + s_separator, "ghi" + s_separator, "jkl" + s_separator });
+        Verify(new string[] { "abc" + s_separator, "def" + s_separator, "ghi" + s_separator, "jkl" + s_separator, "mno" + s_separator });
     }
 
     private static void Verify(string[] paths)