Skip to content

Commit 2dafa1f

Browse files
jonsequiturjonsequitur
authored
Perf tuning (#1145)
* preserve original token order * exclude benchmarks from ncrunch * capture reference to RootCommandResult up front * for loops in SyntaxVisitor * SymbolResultVisitor * small EnsureAliasIndexIsCurrent optimization * move conversion methods to ArgumentConverter * fix IndexOutOfRangeException in IsMatch * specify count in list creation * optimize ArgumentConverter * set UseShellExecute to false for dotnet-suggest registration
1 parent c41a10c commit 2dafa1f

19 files changed

+359
-169
lines changed

‎src/System.CommandLine.Benchmarks/System.CommandLine.Benchmarks.v3.ncrunchproject

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
<ProjectConfiguration>
2+
<Settings>
3+
<IgnoreThisComponentCompletely>True</IgnoreThisComponentCompletely>
4+
</Settings>
5+
</ProjectConfiguration>

‎src/System.CommandLine.Tests/Binding/ModelBinderTests.cs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
using System.CommandLine.Parsing;
88
using System.IO;
99
using FluentAssertions;
10-
using System.Linq;
1110
using Xunit;
1211
using System.Reflection;
1312

‎src/System.CommandLine.Tests/Invocation/CommandHandlerTests.cs

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,37 @@ void Execute(string name, int age)
6262
boundAge.Should().Be(425);
6363
}
6464

65+
[Fact]
66+
public async Task Method_parameters_on_the_invoked_method_are_bound_to_matching_option_aliases()
67+
{
68+
string boundName = default;
69+
int boundAge = default;
70+
71+
void Execute(string n, int a)
72+
{
73+
boundName = n;
74+
boundAge = a;
75+
}
76+
77+
var command = new Command("command");
78+
command.AddOption(
79+
new Option("--name")
80+
{
81+
Argument = new Argument<string>()
82+
});
83+
command.AddOption(
84+
new Option("--age")
85+
{
86+
Argument = new Argument<int>()
87+
});
88+
command.Handler = CommandHandler.Create<string, int>(Execute);
89+
90+
await command.InvokeAsync("command --age 425 --name Gandalf", _console);
91+
92+
boundName.Should().Be("Gandalf");
93+
boundAge.Should().Be(425);
94+
}
95+
6596
[Fact]
6697
public async Task Method_parameters_on_the_invoked_method_can_be_bound_to_hyphenated_option_names()
6798
{

‎src/System.CommandLine.Tests/ParserTests.cs

Lines changed: 1 addition & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33

44
using System.Collections.Generic;
55
using System.CommandLine.Builder;
6-
using System.CommandLine.Invocation;
76
using System.CommandLine.Parsing;
87
using System.CommandLine.Tests.Utility;
98
using System.IO;
@@ -12,21 +11,13 @@
1211
using System.Linq;
1312
using FluentAssertions.Common;
1413
using Xunit;
15-
using Xunit.Abstractions;
1614
using System.ComponentModel;
1715
using System.Globalization;
1816

1917
namespace System.CommandLine.Tests
2018
{
2119
public partial class ParserTests
2220
{
23-
private readonly ITestOutputHelper _output;
24-
25-
public ParserTests(ITestOutputHelper output)
26-
{
27-
_output = output;
28-
}
29-
3021
[Fact]
3122
public void An_option_without_a_long_form_can_be_checked_for_using_a_prefix()
3223
{
@@ -1041,8 +1032,6 @@ public void When_child_option_will_not_accept_arg_then_parent_can()
10411032

10421033
var result = command.Parse("the-command -x the-argument");
10431034

1044-
_output.WriteLine(result.ToString());
1045-
10461035
result.FindResultFor(option).Tokens.Should().BeEmpty();
10471036
result.CommandResult.Tokens.Select(t => t.Value).Should().BeEquivalentTo("the-argument");
10481037
}
@@ -1195,8 +1184,6 @@ public void Options_only_apply_to_the_nearest_command()
11951184

11961185
var result = outer.Parse("outer inner -x one -x two");
11971186

1198-
_output.WriteLine(result.ToString());
1199-
12001187
result.RootCommandResult
12011188
.FindResultFor(outerOption)
12021189
.Should()
@@ -1451,12 +1438,9 @@ public void Command_default_argument_value_does_not_override_parsed_value()
14511438
Name = "the-arg"
14521439
}
14531440
};
1454-
command.Handler = CommandHandler.Create<DirectoryInfo>(arg => { } );
14551441

14561442
var result = command.Parse("the-directory");
14571443

1458-
_output.WriteLine(result.ToString());
1459-
14601444
result.CommandResult
14611445
.GetArgumentValueOrDefault<DirectoryInfo>("the-arg")
14621446
.Name
@@ -1710,8 +1694,6 @@ public void Boolean_options_with_no_argument_specified_do_not_match_subsequent_a
17101694

17111695
var result = command.Parse("-v an-argument");
17121696

1713-
_output.WriteLine(result.ToString());
1714-
17151697
result.ValueForOption("-v").Should().Be(true);
17161698
}
17171699

@@ -1995,8 +1977,6 @@ public void Tokens_are_not_split_if_the_part_before_the_delimiter_is_not_an_opti
19951977
rootCommand.Add(new Option<string>("url"));
19961978
var result = rootCommand.Parse("jdbc url \"jdbc:sqlserver://10.0.0.2;databaseName=main\"");
19971979

1998-
_output.WriteLine(result.ToString());
1999-
20001980
result.Tokens
20011981
.Select(t => t.Value)
20021982
.Should()
@@ -2025,7 +2005,7 @@ public void A_subcommand_wont_overflow_when_checking_maximum_argument_capcity()
20252005
argument2
20262006
};
20272007

2028-
var rootCommand = new RootCommand()
2008+
var rootCommand = new RootCommand
20292009
{
20302010
command
20312011
};

‎src/System.CommandLine/Argument.cs

Lines changed: 4 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -65,54 +65,25 @@ internal TryConvertArgument? ConvertArguments
6565
{
6666
get
6767
{
68-
if (_convertArguments == null)
68+
if (_convertArguments is null)
6969
{
7070
if (ArgumentType.CanBeBoundFromScalarValue())
7171
{
7272
if (Arity.MaximumNumberOfValues == 1 &&
7373
ArgumentType == typeof(bool))
7474
{
75-
_convertArguments = (ArgumentResult symbol, out object? value) =>
76-
{
77-
value = ArgumentConverter.ConvertObject(
78-
this,
79-
typeof(bool),
80-
symbol.Tokens.SingleOrDefault()?.Value ?? bool.TrueString);
81-
82-
return value is SuccessfulArgumentConversionResult;
83-
};
75+
_convertArguments = ArgumentConverter.TryConvertBoolArgument;
8476
}
8577
else
8678
{
87-
_convertArguments = DefaultConvert;
79+
_convertArguments = ArgumentConverter.TryConvertArgument;
8880
}
8981
}
9082
}
9183

9284
return _convertArguments;
9385

94-
bool DefaultConvert(ArgumentResult argumentResult, out object value)
95-
{
96-
switch (Arity.MaximumNumberOfValues)
97-
{
98-
case 1:
99-
value = ArgumentConverter.ConvertObject(
100-
this,
101-
ArgumentType,
102-
argumentResult.Tokens.Select(t => t.Value).SingleOrDefault());
103-
break;
104-
105-
default:
106-
value = ArgumentConverter.ConvertStrings(
107-
this,
108-
ArgumentType,
109-
argumentResult.Tokens.Select(t => t.Value).ToArray(),
110-
argumentResult);
111-
break;
112-
}
113-
114-
return value is SuccessfulArgumentConversionResult;
115-
}
86+
11687
}
11788
set => _convertArguments = value;
11889
}

‎src/System.CommandLine/Binding/ArgumentConverter.cs

Lines changed: 90 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
using System.Collections;
55
using System.Collections.Generic;
66
using System.CommandLine.Parsing;
7-
using System.ComponentModel;
7+
using System.ComponentModel;
88
using System.Diagnostics.CodeAnalysis;
99
using System.IO;
1010
using System.Linq;
@@ -51,7 +51,7 @@ internal static ArgumentConversionResult ConvertObject(
5151
return ConvertString(argument, type, singleValue);
5252
}
5353

54-
case IReadOnlyCollection<string> manyValues:
54+
case IReadOnlyList<string> manyValues:
5555
return ConvertStrings(argument, type, manyValues);
5656
}
5757

@@ -106,59 +106,79 @@ private static ArgumentConversionResult ConvertString(
106106
public static ArgumentConversionResult ConvertStrings(
107107
IArgument argument,
108108
Type type,
109-
IReadOnlyCollection<string> tokens,
109+
IReadOnlyList<string> tokens,
110110
ArgumentResult? argumentResult = null)
111111
{
112-
if (type is null)
113-
{
114-
throw new ArgumentNullException(nameof(type));
115-
}
116-
117-
if (tokens is null)
118-
{
119-
throw new ArgumentNullException(nameof(tokens));
120-
}
121-
122112
var itemType = type == typeof(string)
123113
? typeof(string)
124114
: GetItemTypeIfEnumerable(type);
125115

126-
var parseResults = tokens
127-
.Select(arg => ConvertString(argument, itemType, arg))
128-
.ToArray();
116+
var (values, isArray) = type.IsArray
117+
? (CreateArray(itemType!, tokens.Count), true)
118+
: (CreateList(itemType!, tokens.Count), false);
129119

130-
var list = (IList) Activator.CreateInstance(typeof(List<>).MakeGenericType(itemType));
131-
132-
for (var i = 0; i < parseResults.Length; i++)
120+
for (var i = 0; i < tokens.Count; i++)
133121
{
134-
var result = parseResults[i];
122+
var token = tokens[i];
123+
124+
var result = ConvertString(argument, itemType, token);
135125

136126
switch (result)
137127
{
138128
case FailedArgumentTypeConversionResult _:
139129
case FailedArgumentConversionResult _:
140130
if (argumentResult is { })
141-
{
131+
{
142132
argumentResult.OnlyTake(i);
143-
133+
144134
// exit the for loop
145-
i = parseResults.Length;
135+
i = tokens.Count;
146136
break;
147137
}
148138

149139
return result;
150-
140+
151141
case SuccessfulArgumentConversionResult success:
152-
list.Add(success.Value);
142+
if (isArray)
143+
{
144+
values[i] = success.Value;
145+
}
146+
else
147+
{
148+
values.Add(success.Value);
149+
}
150+
153151
break;
154152
}
155153
}
156154

157-
var value = type.IsArray
158-
? (object) Enumerable.ToArray((dynamic) list)
159-
: list;
155+
return Success(argument, values);
156+
157+
static IList CreateList(Type itemType, int capacity)
158+
{
159+
if (itemType == typeof(string))
160+
{
161+
return new List<string>(capacity);
162+
}
163+
else
164+
{
165+
return (IList) Activator.CreateInstance(
166+
typeof(List<>).MakeGenericType(itemType),
167+
capacity);
168+
}
169+
}
160170

161-
return Success(argument, value);
171+
static IList CreateArray(Type itemType, int capacity)
172+
{
173+
if (itemType == typeof(string))
174+
{
175+
return new string[capacity];
176+
}
177+
else
178+
{
179+
return Array.CreateInstance(itemType, capacity);
180+
}
181+
}
162182
}
163183

164184
private static Type? GetItemTypeIfEnumerable(Type type)
@@ -188,8 +208,7 @@ internal static bool IsEnumerable(this Type type)
188208
return
189209
type.IsArray
190210
||
191-
(type.IsGenericType &&
192-
type.GetGenericTypeDefinition() == typeof(IEnumerable<>));
211+
typeof(IEnumerable).IsAssignableFrom(type);
193212
}
194213

195214
private static bool HasStringTypeConverter(this Type type)
@@ -323,5 +342,45 @@ internal static T GetValueOrDefault<T>(this ArgumentConversionResult result)
323342
return default!;
324343
}
325344
}
345+
346+
public static bool TryConvertBoolArgument(ArgumentResult argumentResult, out object? value)
347+
{
348+
if (argumentResult.Tokens.Count == 0)
349+
{
350+
value = true;
351+
return true;
352+
}
353+
else
354+
{
355+
var success = bool.TryParse(argumentResult.Tokens[0].Value, out var parsed);
356+
value = parsed;
357+
return success;
358+
}
359+
}
360+
361+
public static bool TryConvertArgument(ArgumentResult argumentResult, out object? value)
362+
{
363+
var argument = argumentResult.Argument;
364+
365+
switch (argument.Arity.MaximumNumberOfValues)
366+
{
367+
case 1:
368+
value = ConvertObject(
369+
argument,
370+
argument.ValueType,
371+
argumentResult.Tokens[0].Value);
372+
break;
373+
374+
default:
375+
value = ConvertStrings(
376+
argument,
377+
argument.ValueType,
378+
argumentResult.Tokens.Select(t => t.Value).ToArray(),
379+
argumentResult);
380+
break;
381+
}
382+
383+
return value is SuccessfulArgumentConversionResult;
384+
}
326385
}
327386
}

0 commit comments

Comments
 (0)