Skip to content

Commit e098c0d

Browse files
jonsequiturjonsequitur
authored
Misc. perf improvements (#1427)
* change IDirectiveCollection.TryGetValues signature: * consolidate GetEntryAssembly calls * simplify CommandLineConfiguration ctor * working on cleaning up Option ctors
1 parent 8a0f746 commit e098c0d

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

42 files changed

+604
-396
lines changed

‎src/System.CommandLine.Benchmarks/CommandLine/Perf_Parser_DefaultCtor.cs

Lines changed: 0 additions & 18 deletions
This file was deleted.

‎src/System.CommandLine.Benchmarks/CommandLine/Perf_Parser_Options_Bare.cs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
33

44
using System.Collections.Generic;
5+
using System.CommandLine.Benchmarks.Helpers;
56
using System.CommandLine.Parsing;
67
using System.Linq;
78
using BenchmarkDotNet.Attributes;
@@ -47,13 +48,16 @@ public void SetupTestOptions()
4748
}
4849

4950
[Benchmark]
50-
public Parser ParserFromOptions_Ctor() => new Parser(_testSymbols.ToArray());
51+
public Parser ParserFromOptions_Ctor()
52+
{
53+
return Utils.CreateParser(_testSymbols);
54+
}
5155

5256
[GlobalSetup(Target = nameof(ParserFromOptions_Parse))]
5357
public void SetupParserFromOptions_Parse()
5458
{
5559
var testSymbolsArr = GenerateTestOptions(TestSymbolsCount, ArgumentArity.Zero).ToArray();
56-
_testParser = new Parser(testSymbolsArr);
60+
_testParser = testSymbolsArr.CreateParser();
5761
_testSymbolsAsString = GenerateTestOptionsAsStringExpr(testSymbolsArr.Length);
5862
}
5963

‎src/System.CommandLine.Benchmarks/CommandLine/Perf_Parser_Options_With_Arguments.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
33

44
using System.Collections.Generic;
5+
using System.CommandLine.Benchmarks.Helpers;
56
using System.CommandLine.Parsing;
67
using System.Linq;
78
using BenchmarkDotNet.Attributes;
@@ -52,7 +53,7 @@ private string GenerateTestOptionsWithArgumentsAsStringExpr(int optionsCount, in
5253
public void SetupParserFromOptionsWithArguments_Parse()
5354
{
5455
var testSymbolsArr = GenerateTestOptions(TestOptionsCount, ArgumentArity.OneOrMore).ToArray();
55-
_testParser = new Parser(testSymbolsArr);
56+
_testParser = testSymbolsArr.CreateParser();
5657
_testSymbolsAsString = GenerateTestOptionsWithArgumentsAsStringExpr(testSymbolsArr.Length, TestArgumentsCount);
5758
}
5859

‎src/System.CommandLine.Benchmarks/Helpers/Utils.cs

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
33

44
using System.Collections.Generic;
5+
using System.CommandLine.Parsing;
56
using System.IO;
67
using System.Linq;
78
using Microsoft.CodeAnalysis;
@@ -10,7 +11,7 @@
1011

1112
namespace System.CommandLine.Benchmarks.Helpers
1213
{
13-
static class Utils
14+
internal static class Utils
1415
{
1516
internal static string GetInputFullFilePath(string name)
1617
=> Path.Combine(Path.GetDirectoryName(typeof(Program).Assembly.Location), "Input", name);
@@ -42,5 +43,17 @@ internal static string CreateTestAssemblyInTempFileFromString(string sourceCode,
4243

4344
return pathToAssemblyFile;
4445
}
46+
47+
public static Parser CreateParser(this IEnumerable<Symbol> symbols)
48+
{
49+
var rootCommand = new RootCommand();
50+
51+
foreach (var symbol in symbols)
52+
{
53+
rootCommand.Add(symbol);
54+
}
55+
56+
return new Parser(rootCommand);
57+
}
4558
}
4659
}

‎src/System.CommandLine.Generator.Tests/GeneratedCommandHandlerTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212

1313
namespace System.CommandLine.Generator.Tests
1414
{
15-
public class CommandHandlerTests
15+
public class GeneratedCommandHandlerTests
1616
{
1717
private readonly TestConsole _console = new();
1818

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
using System.IO;
1010
using FluentAssertions;
1111
using Xunit;
12-
using System.Reflection;
1312
using System.Threading.Tasks;
1413

1514
namespace System.CommandLine.Tests.Binding
@@ -524,7 +523,7 @@ public void Explicit_model_binder_binds_only_to_configured_properties()
524523
{
525524
var intOption = new Option<int>("--int-property");
526525
var stringOption = new Option<string>("--string-property");
527-
var parser = new Parser(intOption, stringOption);
526+
var parser = new Parser(new RootCommand { intOption, stringOption });
528527

529528
var bindingContext = new BindingContext(parser.Parse("--int-property 42 --string-property Hello"));
530529
var binder = new ModelBinder<ClassWithMultiLetterSetters>

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

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -143,12 +143,9 @@ public void Directives_can_be_disabled()
143143
{
144144
var parser = new Parser(
145145
new CommandLineConfiguration(
146-
new[]
146+
new RootCommand
147147
{
148-
new RootCommand
149-
{
150-
new Argument<List<string>>()
151-
}
148+
new Argument<List<string>>()
152149
},
153150
enableDirectives: false));
154151

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,7 @@ public async Task ParseResult_can_be_replaced_by_middleware()
195195
{
196196
command
197197
})
198-
.UseMiddleware(async (context, next) =>
198+
.AddMiddleware(async (context, next) =>
199199
{
200200
var tokens = context.ParseResult
201201
.Tokens
@@ -230,7 +230,7 @@ public async Task Invocation_can_be_short_circuited_by_middleware_by_not_calling
230230
{
231231
command
232232
})
233-
.UseMiddleware(async (_, _) =>
233+
.AddMiddleware(async (_, _) =>
234234
{
235235
middlewareWasCalled = true;
236236
await Task.Yield();
@@ -260,7 +260,7 @@ public void Synchronous_invocation_can_be_short_circuited_by_async_middleware_by
260260
{
261261
command
262262
})
263-
.UseMiddleware(async (context, next) =>
263+
.AddMiddleware(async (context, next) =>
264264
{
265265
middlewareWasCalled = true;
266266
await Task.Yield();

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

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ public void An_option_can_be_checked_by_object_instance()
3131
{
3232
var option = new Option("--flag");
3333
var option2 = new Option("--flag2");
34-
var result = new Parser(option, option2)
34+
var result = new Parser(new RootCommand { option, option2 })
3535
.Parse("--flag");
3636

3737
result.HasOption(option).Should().BeTrue();
@@ -46,27 +46,17 @@ public void Two_options_are_parsed_correctly()
4646
var optionTwo = new Option(new[] { "-t", "--two" });
4747

4848
var result = new Parser(
49-
optionOne,
50-
optionTwo)
49+
new RootCommand
50+
{
51+
optionOne,
52+
optionTwo
53+
})
5154
.Parse("-o -t");
5255

5356
result.HasOption(optionOne).Should().BeTrue();
5457
result.HasOption(optionTwo).Should().BeTrue();
5558
}
5659

57-
[Fact]
58-
public void When_no_options_are_specified_then_an_error_is_returned()
59-
{
60-
Action create = () => new Parser(Array.Empty<Symbol>());
61-
62-
create.Should()
63-
.Throw<ArgumentException>()
64-
.Which
65-
.Message
66-
.Should()
67-
.Be("You must specify at least one option or command.");
68-
}
69-
7060
[Theory]
7161
[InlineData("-")]
7262
[InlineData("/")]
@@ -86,10 +76,14 @@ public void When_a_token_is_just_a_prefix_then_an_error_is_returned(string prefi
8676
public void Two_options_cannot_have_conflicting_aliases()
8777
{
8878
Action create = () =>
89-
new Parser(new Option(
90-
new[] { "-o", "--one" }),
91-
new Option(
92-
new[] { "-t", "--one" }));
79+
new Parser(
80+
new RootCommand
81+
{
82+
new Option(
83+
new[] { "-o", "--one" }),
84+
new Option(
85+
new[] { "-t", "--one" })
86+
});
9387

9488
create.Should()
9589
.Throw<ArgumentException>()
@@ -390,9 +384,11 @@ public void Parser_root_Options_can_be_specified_multiple_times_and_their_argume
390384
{
391385
var animalsOption = new Option(new[] { "-a", "--animals" }) { Arity = ArgumentArity.ZeroOrMore };
392386
var vegetablesOption = new Option(new[] { "-v", "--vegetables" }) { Arity = ArgumentArity.ZeroOrMore };
393-
var parser = new Parser(
387+
var parser = new RootCommand
388+
{
394389
animalsOption,
395-
vegetablesOption);
390+
vegetablesOption
391+
};
396392

397393
var result = parser.Parse("-a cat -v carrot -a dog");
398394

@@ -1152,7 +1148,11 @@ public void Options_can_have_the_same_alias_differentiated_only_by_prefix()
11521148
var option1 = new Option(new[] { "-a" });
11531149
var option2 = new Option(new[] { "--a" });
11541150

1155-
var parser = new Parser(option1, option2);
1151+
var parser = new RootCommand
1152+
{
1153+
option1,
1154+
option2
1155+
};
11561156

11571157
parser.Parse("-a").CommandResult
11581158
.Children
@@ -1460,7 +1460,7 @@ public void When_a_command_line_has_unmatched_tokens_they_are_not_applied_to_sub
14601460
[Fact]
14611461
public void Parse_can_be_called_with_null_args()
14621462
{
1463-
var parser = new Parser();
1463+
var parser = new Parser(new RootCommand());
14641464

14651465
var result = parser.Parse(null);
14661466

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -911,9 +911,11 @@ public void When_an_option_has_a_default_value_then_the_default_should_apply_if_
911911
var optionX = new Option<int>("-x", () => 123);
912912
var optionY = new Option<int>("-y", () => 456);
913913

914-
var parser = new Parser(
914+
var parser = new RootCommand
915+
{
915916
optionX,
916-
optionY);
917+
optionY
918+
};
917919

918920
var result = parser.Parse("");
919921

0 commit comments

Comments
 (0)