Skip to content

Commit c87e204

Browse files
authored
Targeted perf changes to CommandLineParser (#78446)
CommandLineParser shows up as surprisingly expensive in the speedometer traces I've been looking at for solution load. This PR attempts a couple small, targeted fixes for a slight improvement. IsOptionName was doing either 1 or 2 passes in the standard ascii case. Instead, do a length check upfront so that we commonly do 0 passes, and if the lengths match, do 1 pass in the ascii case. In the non-ascii case, fall back to using the ReadOnlySpan's Equals as the code did before. This is a small CPU win reflected by the first two images below. TryParseOption's calls to IndexOf are showing up in the trace. We can limit the span we search significantly as colon typically is found towards the beginning of arg and arg can be quite long. Small CPU win again, reflected by the 3rd and 4th images below. ParseSeparatedStrings is a very commonly tread codepath. Small micro-optimizations in the loop to not do the IndexOf call when c is a quote and to check the common condition first. See PR for performance details from speedometer runs.
1 parent d7903f9 commit c87e204

File tree

1 file changed

+30
-21
lines changed

1 file changed

+30
-21
lines changed

src/Compilers/Core/Portable/CommandLine/CommandLineParser.cs

Lines changed: 30 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -98,32 +98,39 @@ internal static bool IsOptionName(string shortOptionName, string longOptionName,
9898
/// </remarks>
9999
internal static bool IsOptionName(string optionName, ReadOnlySpan<char> value)
100100
{
101-
Debug.Assert(isAllAscii(optionName.AsSpan()));
102-
if (isAllAscii(value))
101+
assertAllAscii(optionName.AsSpan());
102+
103+
if (optionName.Length != value.Length)
104+
return false;
105+
106+
for (int i = 0; i < optionName.Length; i++)
103107
{
104-
if (optionName.Length != value.Length)
105-
return false;
108+
char ch = value[i];
109+
if (ch > 127)
110+
{
111+
// If a non-ascii character is encountered, do an InvariantCultureIgnoreCase comparison
112+
return optionName.AsSpan().Equals(value, StringComparison.InvariantCultureIgnoreCase);
113+
}
106114

107-
for (int i = 0; i < optionName.Length; i++)
115+
if (optionName[i] != char.ToLowerInvariant(ch))
108116
{
109-
if (optionName[i] != char.ToLowerInvariant(value[i]))
110-
{
111-
return false;
112-
}
117+
return false;
113118
}
114-
return true;
115119
}
116120

117-
return optionName.AsSpan().Equals(value, StringComparison.InvariantCultureIgnoreCase);
121+
return true;
118122

119-
static bool isAllAscii(ReadOnlySpan<char> span)
123+
[Conditional("DEBUG")]
124+
static void assertAllAscii(ReadOnlySpan<char> span)
120125
{
121126
foreach (char ch in span)
122127
{
123128
if (ch > 127)
124-
return false;
129+
{
130+
Debug.Assert(false);
131+
break;
132+
}
125133
}
126-
return true;
127134
}
128135
}
129136

@@ -167,16 +174,19 @@ internal static bool TryParseOption(string arg, out ReadOnlyMemory<char> name, o
167174
return true;
168175
}
169176

170-
int colon = arg.IndexOf(':');
177+
int colon = arg.IndexOf(':', 1);
171178

172179
// temporary heuristic to detect Unix-style rooted paths
173180
// pattern /goo/* or //* will not be treated as a compiler option
174181
//
175182
// TODO: consider introducing "/s:path" to disambiguate paths starting with /
176183
if (arg.Length > 1 && arg[0] != '-')
177184
{
178-
int separator = arg.IndexOf('/', 1);
179-
if (separator > 0 && (colon < 0 || separator < colon))
185+
int separator = colon < 0
186+
? arg.IndexOf('/', 1)
187+
: arg.IndexOf('/', 1, colon - 1);
188+
189+
if (separator > 0)
180190
{
181191
// "/goo/
182192
// "//
@@ -1064,11 +1074,10 @@ internal static void ParseSeparatedStrings(ReadOnlyMemory<char>? strMemory, char
10641074
{
10651075
inQuotes = !inQuotes;
10661076
}
1067-
1068-
if (!inQuotes && separators.IndexOf(c) >= 0)
1077+
else if (!inQuotes && separators.Contains(c))
10691078
{
10701079
var current = memory.Slice(nextPiece, i - nextPiece);
1071-
if (!removeEmptyEntries || current.Length > 0)
1080+
if (current.Length > 0 || !removeEmptyEntries)
10721081
{
10731082
builder.Add(current);
10741083
}
@@ -1078,7 +1087,7 @@ internal static void ParseSeparatedStrings(ReadOnlyMemory<char>? strMemory, char
10781087
}
10791088

10801089
var last = memory.Slice(nextPiece);
1081-
if (!removeEmptyEntries || last.Length > 0)
1090+
if (last.Length > 0 || !removeEmptyEntries)
10821091
{
10831092
builder.Add(last);
10841093
}

0 commit comments

Comments
 (0)