Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Commit 8e225ce

Browse files
committed
Remove string allocation/copy from Path.Combine
Path.Combine(string, string, string) is essentially implemented as Path.Combine(Path.Combine(string, string), string), resulting in the temporary string from combining the first two strings and then another string from combining that with the third string. This commit adds a new overload of CommitNoCheck that does combining for three paths without the intermediate allocation/copy.
1 parent 3384c2a commit 8e225ce

File tree

2 files changed

+68
-7
lines changed

2 files changed

+68
-7
lines changed

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

Lines changed: 44 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -311,7 +311,7 @@ public static string Combine(string path1, string path2, string path3)
311311
CheckInvalidPathChars(path2);
312312
CheckInvalidPathChars(path3);
313313

314-
return CombineNoChecks(CombineNoChecks(path1, path2), path3);
314+
return CombineNoChecks(path1, path2, path3);
315315
}
316316

317317
public static string Combine(params string[] paths)
@@ -402,6 +402,49 @@ private static string CombineNoChecks(string path1, string path2)
402402
path1 + DirectorySeparatorCharAsString + path2;
403403
}
404404

405+
private static string CombineNoChecks(string path1, string path2, string path3)
406+
{
407+
if (path1.Length == 0)
408+
return CombineNoChecks(path2, path3);
409+
if (path2.Length == 0)
410+
return CombineNoChecks(path1, path3);
411+
if (path3.Length == 0)
412+
return CombineNoChecks(path1, path2);
413+
414+
if (IsPathRooted(path3))
415+
return path3;
416+
if (IsPathRooted(path2))
417+
return CombineNoChecks(path2, path3);
418+
419+
bool hasSep1 = IsDirectoryOrVolumeSeparator(path1[path1.Length - 1]);
420+
bool hasSep2 = IsDirectoryOrVolumeSeparator(path2[path2.Length - 1]);
421+
422+
if (hasSep1 && hasSep2)
423+
{
424+
return path1 + path2 + path3;
425+
}
426+
else if (hasSep1)
427+
{
428+
return path1 + path2 + DirectorySeparatorCharAsString + path3;
429+
}
430+
else if (hasSep2)
431+
{
432+
return path1 + DirectorySeparatorCharAsString + path2 + path3;
433+
}
434+
else
435+
{
436+
// string.Concat only has string-based overloads up to four arguments; after that requires allocating
437+
// a params string[]. Instead, try to use a cached StringBuilder.
438+
StringBuilder sb = StringBuilderCache.Acquire(path1.Length + path2.Length + path3.Length + 2);
439+
sb.Append(path1)
440+
.Append(DirectorySeparatorChar)
441+
.Append(path2)
442+
.Append(DirectorySeparatorChar)
443+
.Append(path3);
444+
return StringBuilderCache.GetStringAndRelease(sb);
445+
}
446+
}
447+
405448
private static readonly char[] s_Base32Char = {
406449
'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h',
407450
'i', 'j', 'k', 'l', 'm', 'n', 'o', 'p',

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

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,15 @@ public static void PathIsNullWihtoutRootedAfterArgumentNull()
5151
}
5252

5353
[Fact]
54-
[PlatformSpecific(PlatformID.Windows)]
5554
public static void ContainsInvalidCharWithoutRootedAfterArgumentNull()
55+
{
56+
//any path contains invalid character without rooted after (AE)
57+
CommonCasesException<ArgumentException>("ab\0cd");
58+
}
59+
60+
[Fact]
61+
[PlatformSpecific(PlatformID.Windows)]
62+
public static void ContainsInvalidCharWithoutRootedAfterArgumentNull_Windows()
5663
{
5764
//any path contains invalid character without rooted after (AE)
5865
CommonCasesException<ArgumentException>("ab\"cd");
@@ -66,16 +73,23 @@ public static void ContainsInvalidCharWithoutRootedAfterArgumentNull()
6673
}
6774

6875
[Fact]
69-
[PlatformSpecific(PlatformID.Windows)]
7076
public static void ContainsInvalidCharWithRootedAfterArgumentNull()
77+
{
78+
//any path contains invalid character with rooted after (AE)
79+
CommonCasesException<ArgumentException>("ab\0cd", s_separator + "abc");
80+
}
81+
82+
83+
[Fact]
84+
[PlatformSpecific(PlatformID.Windows)]
85+
public static void ContainsInvalidCharWithRootedAfterArgumentNull_Windows()
7186
{
7287
//any path contains invalid character with rooted after (AE)
7388
CommonCasesException<ArgumentException>("ab\"cd", s_separator + "abc");
7489
CommonCasesException<ArgumentException>("ab<cd", s_separator + "abc");
7590
CommonCasesException<ArgumentException>("ab>cd", s_separator + "abc");
7691
CommonCasesException<ArgumentException>("ab|cd", s_separator + "abc");
7792
CommonCasesException<ArgumentException>("ab\bcd", s_separator + "abc");
78-
CommonCasesException<ArgumentException>("ab\0cd", s_separator + "abc");
7993
CommonCasesException<ArgumentException>("ab\tcd", s_separator + "abc");
8094
}
8195

@@ -110,6 +124,7 @@ public static void PathIsSingleElement()
110124
{
111125
//any path is single element
112126
CommonCases("abc");
127+
CommonCases("abc" + s_separator);
113128
}
114129

115130
[Fact]
@@ -119,10 +134,13 @@ public static void PathIsMultipleElements()
119134
CommonCases(Path.Combine("abc", Path.Combine("def", "ghi")));
120135
}
121136

122-
public static void NoPathIsRooted()
137+
[Fact]
138+
public static void PathElementsAllSeparated()
123139
{
124-
//no path is rooted
125-
CommonCases("abc");
140+
Verify(new string[] { "abc" + s_separator, "def" + s_separator });
141+
Verify(new string[] { "abc" + s_separator, "def" + s_separator, "ghi" + s_separator });
142+
Verify(new string[] { "abc" + s_separator, "def" + s_separator, "ghi" + s_separator, "jkl" + s_separator });
143+
Verify(new string[] { "abc" + s_separator, "def" + s_separator, "ghi" + s_separator, "jkl" + s_separator, "mno" + s_separator });
126144
}
127145

128146
private static void Verify(string[] paths)

0 commit comments

Comments
 (0)