Skip to content

Commit 3cb430c

Browse files
adamsitnikadamsitnik
authored
Perf improvements part 6: startup time (#1654)
* extend ICommandHandler with synchronous Invoke method * don't build invocation chain if there is no middleware * _executablePath.Value already uses Environment.GetCommandLineArgs()[0] * Command.Validators: allocate when needed * reduce and simplify ValidateCommandResult size to JIT less code in most common scenarios * reduce ParseResultVisitor.Stop size, saves 1 ms of JIT time for simple scenarios * make Token a class again, so JIT does not need to compile specialized methods for it. -3ms for startup! * refactor ParseDirectives to JIT less code when directives are not provided (0.4 ms of JIT time) * refactor GetValueForHandlerParameter to JIT less code for most common case * remove static fields from StringExtensions (they need to be compiled and initialized) * optimize getting Middleware by using Tuple (class) instead of ValueTuple (struct). -4 JIT compilaitons * use Nullable.GetUnderlyingType instead of custom implementation (and compile less) * don't use Lazy, -4 compilations * reduce the number of types derived from ArgumentConversionResult to reduce the number of compilations and type loading * remove RootCommandNode to reduce the number of types * remove another Lazy (the code it uses needed to be compiled the first time ArgumentConverter was used, no matter if we wanted to create a list or not) * add net6.0 to benchmarks project * address code review feedback
1 parent 05ba968 commit 3cb430c

File tree

43 files changed

+408
-456
lines changed

Some content is hidden

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

43 files changed

+408
-456
lines changed

‎src/System.CommandLine.ApiCompatibility.Tests/ApiCompatibilityApprovalTests.System_CommandLine_NamingConventionBinder_api_is_not_changed.approved.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,7 @@
9898
public class ModelBindingCommandHandler, System.CommandLine.Invocation.ICommandHandler
9999
public System.Void BindParameter(System.Reflection.ParameterInfo param, System.CommandLine.Argument argument)
100100
public System.Void BindParameter(System.Reflection.ParameterInfo param, System.CommandLine.Option option)
101+
public System.Int32 Invoke(System.CommandLine.Invocation.InvocationContext context)
101102
public System.Threading.Tasks.Task<System.Int32> InvokeAsync(System.CommandLine.Invocation.InvocationContext context)
102103
public class ModelDescriptor
103104
public static ModelDescriptor FromType<T>()

‎src/System.CommandLine.ApiCompatibility.Tests/ApiCompatibilityApprovalTests.System_CommandLine_api_is_not_changed.approved.txt

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -356,6 +356,7 @@ System.CommandLine.Help
356356
public System.Int32 GetHashCode()
357357
System.CommandLine.Invocation
358358
public interface ICommandHandler
359+
public System.Int32 Invoke(InvocationContext context)
359360
public System.Threading.Tasks.Task<System.Int32> InvokeAsync(InvocationContext context)
360361
public interface IInvocationResult
361362
public System.Void Apply(InvocationContext context)
@@ -498,7 +499,7 @@ System.CommandLine.Parsing
498499
public T GetValueForOption<T>(Option<T> option)
499500
public System.Object GetValueForOption(System.CommandLine.Option option)
500501
public System.String ToString()
501-
public struct Token : System.ValueType, System.IEquatable<Token>
502+
public class Token, System.IEquatable<Token>
502503
public static System.Boolean op_Equality(Token left, Token right)
503504
public static System.Boolean op_Inequality(Token left, Token right)
504505
.ctor(System.String value, TokenType type, System.CommandLine.Symbol symbol)

‎src/System.CommandLine.Benchmarks/System.CommandLine.Benchmarks.csproj

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,8 @@
99
<UseSharedCompilation>false</UseSharedCompilation>
1010

1111
<!-- Supported target frameworks -->
12-
<TargetFrameworks Condition="'$(TargetFrameworks)' == '' AND '$(OS)' == 'Windows_NT'">net461;net5.0;</TargetFrameworks>
13-
<TargetFrameworks Condition="'$(TargetFrameworks)' == ''">net5.0;</TargetFrameworks>
12+
<TargetFrameworks Condition="'$(TargetFrameworks)' == '' AND '$(OS)' == 'Windows_NT'">net461;net5.0;net6.0;</TargetFrameworks>
13+
<TargetFrameworks Condition="'$(TargetFrameworks)' == ''">net5.0;net6.0;</TargetFrameworks>
1414

1515
<!-- This repo does not produce any libraries, therefore generating docs is disabled -->
1616
<GenerateDocumentationFile>False</GenerateDocumentationFile>

‎src/System.CommandLine.Generator/CommandHandlerSourceGenerator.cs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,9 @@ private class GeneratedHandler_{handlerCount} : {ICommandHandlerType}
114114
{propertyDeclaration}");
115115
}
116116

117+
builder.Append($@"
118+
public int Invoke(global::System.CommandLine.Invocation.InvocationContext context) => InvokeAsync(context).GetAwaiter().GetResult();");
119+
117120
builder.Append($@"
118121
public async global::System.Threading.Tasks.Task<int> InvokeAsync(global::System.CommandLine.Invocation.InvocationContext context)
119122
{{");

‎src/System.CommandLine.Hosting.Tests/HostingHandlerTest.cs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,12 @@ public MyHandler(MyService service)
132132
public int IntOption { get; set; } // bound from option
133133
public IConsole Console { get; set; } // bound from DI
134134

135+
public int Invoke(InvocationContext context)
136+
{
137+
service.Value = IntOption;
138+
return IntOption;
139+
}
140+
135141
public Task<int> InvokeAsync(InvocationContext context)
136142
{
137143
service.Value = IntOption;
@@ -162,6 +168,8 @@ public MyHandler(MyService service)
162168

163169
public string One { get; set; }
164170

171+
public int Invoke(InvocationContext context) => InvokeAsync(context).GetAwaiter().GetResult();
172+
165173
public Task<int> InvokeAsync(InvocationContext context)
166174
{
167175
service.Value = IntOption;

‎src/System.CommandLine.NamingConventionBinder.Tests/ParameterBindingTests.cs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -446,6 +446,8 @@ public abstract class AbstractTestCommandHandler : ICommandHandler
446446
{
447447
public abstract Task<int> DoJobAsync();
448448

449+
public int Invoke(InvocationContext context) => InvokeAsync(context).GetAwaiter().GetResult();
450+
449451
public Task<int> InvokeAsync(InvocationContext context)
450452
=> DoJobAsync();
451453
}
@@ -458,6 +460,8 @@ public override Task<int> DoJobAsync()
458460

459461
public class VirtualTestCommandHandler : ICommandHandler
460462
{
463+
public int Invoke(InvocationContext context) => 42;
464+
461465
public virtual Task<int> InvokeAsync(InvocationContext context)
462466
=> Task.FromResult(42);
463467
}

‎src/System.CommandLine.NamingConventionBinder/ModelBindingCommandHandler.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,4 +128,6 @@ private void BindValueSource(ParameterInfo param, IValueSource valueSource)
128128
: _methodDescriptor.ParameterDescriptors
129129
.FirstOrDefault(x => x.ValueName == param.Name &&
130130
x.ValueType == param.ParameterType);
131+
132+
public int Invoke(InvocationContext context) => InvokeAsync(context).GetAwaiter().GetResult();
131133
}

‎src/System.CommandLine/ArgumentArity.cs

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ public bool Equals(ArgumentArity other) =>
7272
public override int GetHashCode()
7373
=> MaximumNumberOfValues ^ MinimumNumberOfValues ^ IsNonDefault.GetHashCode();
7474

75-
internal static FailedArgumentConversionArityResult? Validate(
75+
internal static ArgumentConversionResult? Validate(
7676
SymbolResult symbolResult,
7777
Argument argument,
7878
int minimumNumberOfValues,
@@ -93,9 +93,10 @@ public override int GetHashCode()
9393
return null;
9494
}
9595

96-
return new MissingArgumentConversionResult(
96+
return ArgumentConversionResult.Failure(
9797
argument,
98-
symbolResult.LocalizationResources.RequiredArgumentMissing(symbolResult));
98+
symbolResult.LocalizationResources.RequiredArgumentMissing(symbolResult),
99+
ArgumentConversionResultType.FailedMissingArgument);
99100
}
100101

101102
if (tokenCount > maximumNumberOfValues)
@@ -104,9 +105,10 @@ public override int GetHashCode()
104105
{
105106
if (!optionResult.Option.AllowMultipleArgumentsPerToken)
106107
{
107-
return new TooManyArgumentsConversionResult(
108+
return ArgumentConversionResult.Failure(
108109
argument,
109-
symbolResult!.LocalizationResources.ExpectsOneArgument(symbolResult));
110+
symbolResult!.LocalizationResources.ExpectsOneArgument(symbolResult),
111+
ArgumentConversionResultType.FailedTooManyArguments);
110112
}
111113
}
112114
}

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

Lines changed: 57 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,73 @@
11
// Copyright (c) .NET Foundation and contributors. All rights reserved.
22
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
33

4+
using System.Linq;
5+
46
namespace System.CommandLine.Binding
57
{
6-
internal abstract class ArgumentConversionResult
8+
internal sealed class ArgumentConversionResult
79
{
8-
private protected ArgumentConversionResult(Argument argument)
10+
internal readonly Argument Argument;
11+
internal readonly object? Value;
12+
internal readonly string? ErrorMessage;
13+
internal ArgumentConversionResultType Result;
14+
15+
private ArgumentConversionResult(Argument argument, string error, ArgumentConversionResultType failure)
16+
{
17+
Argument = argument ?? throw new ArgumentNullException(nameof(argument));
18+
ErrorMessage = error ?? throw new ArgumentNullException(nameof(error));
19+
Result = failure;
20+
}
21+
22+
private ArgumentConversionResult(Argument argument, object? value)
23+
{
24+
Argument = argument ?? throw new ArgumentNullException(nameof(argument));
25+
Value = value;
26+
Result = ArgumentConversionResultType.Successful;
27+
}
28+
29+
private ArgumentConversionResult(Argument argument)
930
{
1031
Argument = argument ?? throw new ArgumentNullException(nameof(argument));
32+
Result = ArgumentConversionResultType.NoArgument;
33+
}
34+
35+
internal ArgumentConversionResult(
36+
Argument argument,
37+
Type expectedType,
38+
string value,
39+
LocalizationResources localizationResources) :
40+
this(argument, FormatErrorMessage(argument, expectedType, value, localizationResources), ArgumentConversionResultType.FailedType)
41+
{
1142
}
1243

13-
public Argument Argument { get; }
44+
internal static ArgumentConversionResult Failure(Argument argument, string error, ArgumentConversionResultType reason) => new(argument, error, reason);
45+
46+
public static ArgumentConversionResult Success(Argument argument, object? value) => new(argument, value);
1447

15-
internal string? ErrorMessage { get; set; }
48+
internal static ArgumentConversionResult None(Argument argument) => new(argument);
1649

17-
internal static FailedArgumentConversionResult Failure(Argument argument, string error) => new(argument, error);
50+
private static string FormatErrorMessage(
51+
Argument argument,
52+
Type expectedType,
53+
string value,
54+
LocalizationResources localizationResources)
55+
{
56+
if (argument.FirstParent?.Symbol is IdentifierSymbol identifierSymbol &&
57+
argument.FirstParent.Next is null)
58+
{
59+
var alias = identifierSymbol.Aliases.First();
1860

19-
public static SuccessfulArgumentConversionResult Success(Argument argument, object? value) => new(argument, value);
61+
switch (identifierSymbol)
62+
{
63+
case Command _:
64+
return localizationResources.ArgumentConversionCannotParseForCommand(value, alias, expectedType);
65+
case Option _:
66+
return localizationResources.ArgumentConversionCannotParseForOption(value, alias, expectedType);
67+
}
68+
}
2069

21-
internal static NoArgumentConversionResult None(Argument argument) => new(argument);
70+
return localizationResources.ArgumentConversionCannotParse(value, expectedType);
71+
}
2272
}
2373
}

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
// Copyright (c) .NET Foundation and contributors. All rights reserved.
2+
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
3+
4+
namespace System.CommandLine.Binding
5+
{
6+
internal enum ArgumentConversionResultType
7+
{
8+
NoArgument, // NoArgumentConversionResult
9+
Successful, // SuccessfulArgumentConversionResult
10+
Failed, // FailedArgumentConversionResult
11+
FailedArity, // FailedArgumentConversionArityResult
12+
FailedType, // FailedArgumentTypeConversionResult
13+
FailedTooManyArguments, // TooManyArgumentsConversionResult
14+
FailedMissingArgument, // MissingArgumentConversionResult
15+
}
16+
}

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

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
using System.Collections;
55
using System.Collections.Generic;
66
using System.Diagnostics.CodeAnalysis;
7-
using System.Linq;
87
using System.Reflection;
98
using System.Runtime.Serialization;
109

@@ -13,10 +12,7 @@ namespace System.CommandLine.Binding;
1312
internal static partial class ArgumentConverter
1413
{
1514
#if NET6_0_OR_GREATER
16-
private static readonly Lazy<ConstructorInfo> _listCtor =
17-
new(() => typeof(List<>)
18-
.GetConstructors()
19-
.SingleOrDefault(c => c.GetParameters().Length == 0)!);
15+
private static ConstructorInfo? _listCtor;
2016
#endif
2117

2218
private static Array CreateEmptyArray(Type itemType, int capacity = 0)
@@ -25,14 +21,19 @@ private static Array CreateEmptyArray(Type itemType, int capacity = 0)
2521
private static IList CreateEmptyList(Type listType)
2622
{
2723
#if NET6_0_OR_GREATER
28-
var ctor = (ConstructorInfo)listType.GetMemberWithSameMetadataDefinitionAs(_listCtor.Value);
24+
ConstructorInfo? listCtor = _listCtor;
25+
26+
if (listCtor is null)
27+
{
28+
_listCtor = listCtor = typeof(List<>).GetConstructor(Type.EmptyTypes)!;
29+
}
30+
31+
var ctor = (ConstructorInfo)listType.GetMemberWithSameMetadataDefinitionAs(listCtor);
2932
#else
30-
var ctor = listType
31-
.GetConstructors()
32-
.SingleOrDefault(c => c.GetParameters().Length == 0);
33+
var ctor = listType.GetConstructor(Type.EmptyTypes);
3334
#endif
3435

35-
return (IList)ctor.Invoke(new object[] { });
36+
return (IList)ctor.Invoke(null);
3637
}
3738

3839
private static IList CreateEnumerable(Type type, Type itemType, int capacity = 0)

0 commit comments

Comments
 (0)