Skip to content

Commit b805d09

Browse files
adamsitnikadamsitnik
authored
Perf improvements part 4 (#1543)
* use StringComparer.Ordinal for aliases matching * allocate Validators list when it's actually needed * use the number of arguments as initial token list capacity * there is no need to have an abstract class with a single implementation * don't create expensive BindingContext until we actually need it * avoid allocating empty validator list * fetch count only once * Command is the only symbol that can have multiple Children * don't check the name if we know it's null * there is no need for these methods to be virtual
1 parent 74ae46e commit b805d09

File tree

13 files changed

+235
-245
lines changed

13 files changed

+235
-245
lines changed

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@
5050
public class Command : IdentifierSymbol, System.Collections.Generic.IEnumerable<Symbol>, System.Collections.IEnumerable, System.CommandLine.Completions.ICompletionSource
5151
.ctor(System.String name, System.String description = null)
5252
public System.Collections.Generic.IReadOnlyList<Argument> Arguments { get; }
53+
public System.CommandLine.Collections.SymbolSet Children { get; }
5354
public System.CommandLine.Invocation.ICommandHandler Handler { get; set; }
5455
public System.Collections.Generic.IReadOnlyList<Option> Options { get; }
5556
public System.Boolean TreatUnmatchedTokensAsErrors { get; set; }
@@ -60,6 +61,7 @@
6061
public System.Void AddGlobalOption(Option option)
6162
public System.Void AddOption(Option option)
6263
public System.Void AddValidator(System.CommandLine.Parsing.ValidateSymbolResult<System.CommandLine.Parsing.CommandResult> validate)
64+
public System.Collections.Generic.IEnumerable<System.CommandLine.Completions.CompletionItem> GetCompletions(System.CommandLine.Completions.CompletionContext context)
6365
public System.Collections.Generic.IEnumerator<Symbol> GetEnumerator()
6466
public static class CommandExtensions
6567
public static System.Int32 Invoke(this Command command, System.String[] args, IConsole console = null)
@@ -183,6 +185,7 @@
183185
public System.Boolean IsRequired { get; set; }
184186
public System.Type ValueType { get; }
185187
public System.Void AddValidator(System.CommandLine.Parsing.ValidateSymbolResult<System.CommandLine.Parsing.OptionResult> validate)
188+
public System.Collections.Generic.IEnumerable<System.CommandLine.Completions.CompletionItem> GetCompletions(System.CommandLine.Completions.CompletionContext context)
186189
public System.Boolean HasAliasIgnoringPrefix(System.String alias)
187190
public System.Void SetDefaultValue(System.Object value)
188191
public System.Void SetDefaultValueFactory(System.Func<System.Object> getDefaultValue)
@@ -212,7 +215,6 @@
212215
public static System.String ExecutablePath { get; }
213216
.ctor(System.String description = )
214217
public abstract class Symbol, System.CommandLine.Completions.ICompletionSource
215-
public System.CommandLine.Collections.SymbolSet Children { get; }
216218
public System.String Description { get; set; }
217219
public System.Boolean IsHidden { get; set; }
218220
public System.String Name { get; set; }

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -55,10 +55,10 @@ public void SetupRootCommand()
5555
GenerateTestNestedCommands(rootCommand, TestCommandsDepth, TestCommandsDepth);
5656

5757
// Choose only one path from the commands tree for the test arguments string
58-
Symbol currentCmd = rootCommand;
59-
while (currentCmd.Children.Count > 0)
58+
Command currentCmd = rootCommand;
59+
while (currentCmd is not null && currentCmd.Children.Count > 0)
6060
{
61-
currentCmd = currentCmd.Children[0];
61+
currentCmd = currentCmd.Children[0] as Command;
6262
_testSymbolsAsString = string.Join(" ", _testSymbolsAsString, currentCmd.Name);
6363
}
6464

‎src/System.CommandLine/Argument.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ public class Argument : Symbol, IValueDescriptor
1919
private TryConvertArgument? _convertArguments;
2020
private Type _valueType = typeof(string);
2121
private CompletionSourceList? _completions = null;
22+
private List<ValidateSymbolResult<ArgumentResult>>? _validators = null;
2223

2324
/// <summary>
2425
/// Initializes a new instance of the Argument class.
@@ -108,7 +109,7 @@ private protected override string DefaultName
108109
}
109110
}
110111

111-
internal List<ValidateSymbolResult<ArgumentResult>> Validators { get; } = new();
112+
internal List<ValidateSymbolResult<ArgumentResult>> Validators => _validators ??= new ();
112113

113114
/// <summary>
114115
/// Adds a custom <see cref="ValidateSymbolResult{ArgumentResult}"/> to the argument. Validators can be used

‎src/System.CommandLine/Command.cs

Lines changed: 67 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,11 @@
33

44
using System.Collections;
55
using System.Collections.Generic;
6+
using System.CommandLine.Collections;
7+
using System.CommandLine.Completions;
68
using System.CommandLine.Invocation;
79
using System.CommandLine.Parsing;
10+
using System.Linq;
811

912
namespace System.CommandLine
1013
{
@@ -27,6 +30,11 @@ public Command(string name, string? description = null) : base(name, description
2730
{
2831
}
2932

33+
/// <summary>
34+
/// Gets the child symbols.
35+
/// </summary>
36+
public SymbolSet Children { get; } = new();
37+
3038
/// <summary>
3139
/// Represents all of the arguments for the command.
3240
/// </summary>
@@ -41,7 +49,11 @@ public Command(string name, string? description = null) : base(name, description
4149
/// Adds an <see cref="Argument"/> to the command.
4250
/// </summary>
4351
/// <param name="argument">The argument to add to the command.</param>
44-
public void AddArgument(Argument argument) => AddArgumentInner(argument);
52+
public void AddArgument(Argument argument)
53+
{
54+
argument.AddParent(this);
55+
Children.AddWithoutAliasCollisionCheck(argument);
56+
}
4557

4658
/// <summary>
4759
/// Adds a subcommand to the command.
@@ -116,5 +128,59 @@ public void AddGlobalOption(Option option)
116128
IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();
117129

118130
internal Parser? ImplicitParser { get; set; }
131+
132+
private protected void AddSymbol(Symbol symbol)
133+
{
134+
Children.AddWithoutAliasCollisionCheck(symbol);
135+
symbol.AddParent(this);
136+
}
137+
138+
public override IEnumerable<CompletionItem> GetCompletions(CompletionContext context)
139+
{
140+
if (Children.Count == 0)
141+
{
142+
return Array.Empty<CompletionItem>();
143+
}
144+
145+
var completions = new List<CompletionItem>();
146+
147+
if (context.WordToComplete is { } textToMatch)
148+
{
149+
for (var i = 0; i < Children.Count; i++)
150+
{
151+
var child = Children[i];
152+
153+
switch (child)
154+
{
155+
case IdentifierSymbol identifier when !child.IsHidden:
156+
foreach (var alias in identifier.Aliases)
157+
{
158+
if (alias is { } &&
159+
alias.ContainsCaseInsensitive(textToMatch))
160+
{
161+
completions.Add(new CompletionItem(alias, CompletionItemKind.Keyword, detail: child.Description));
162+
}
163+
}
164+
165+
break;
166+
167+
case Argument argument:
168+
foreach (var completion in argument.GetCompletions(context))
169+
{
170+
if (completion.Label.ContainsCaseInsensitive(textToMatch))
171+
{
172+
completions.Add(completion);
173+
}
174+
}
175+
176+
break;
177+
}
178+
}
179+
}
180+
181+
return completions
182+
.OrderBy(item => item.SortText.IndexOfCaseInsensitive(context.WordToComplete))
183+
.ThenBy(symbol => symbol.Label, StringComparer.OrdinalIgnoreCase);
184+
}
119185
}
120186
}

‎src/System.CommandLine/IdentifierSymbol.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ namespace System.CommandLine
1111
/// </summary>
1212
public abstract class IdentifierSymbol : Symbol
1313
{
14-
private protected readonly HashSet<string> _aliases = new();
14+
private protected readonly HashSet<string> _aliases = new(StringComparer.Ordinal);
1515
private string? _specifiedName;
1616

1717
/// <summary>
@@ -45,7 +45,7 @@ public override string Name
4545
get => _specifiedName ??= DefaultName;
4646
set
4747
{
48-
if (!string.Equals(_specifiedName, value, StringComparison.Ordinal))
48+
if (_specifiedName is null || !string.Equals(_specifiedName, value, StringComparison.Ordinal))
4949
{
5050
AddAlias(value);
5151

‎src/System.CommandLine/Invocation/InvocationContext.cs

Lines changed: 22 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
using System.CommandLine.Binding;
55
using System.CommandLine.Help;
6+
using System.CommandLine.IO;
67
using System.CommandLine.Parsing;
78
using System.Threading;
89

@@ -16,27 +17,41 @@ public sealed class InvocationContext : IDisposable
1617
private CancellationTokenSource? _cts;
1718
private Action<CancellationTokenSource>? _cancellationHandlingAddedEvent;
1819
private HelpBuilder? _helpBuilder;
20+
private BindingContext? _bindingContext;
21+
private IConsole _console;
1922

2023
/// <param name="parseResult">The result of the current parse operation.</param>
2124
/// <param name="console">The console to which output is to be written.</param>
2225
public InvocationContext(
2326
ParseResult parseResult,
2427
IConsole? console = null)
2528
{
26-
BindingContext = new BindingContext(parseResult, console);
27-
BindingContext.ServiceProvider.AddService(_ => GetCancellationToken());
28-
BindingContext.ServiceProvider.AddService(_ => this);
29+
ParseResult = parseResult;
30+
_console = console ?? new SystemConsole();
2931
}
3032

3133
/// <summary>
3234
/// The binding context for the current invocation.
3335
/// </summary>
34-
public BindingContext BindingContext { get; }
36+
public BindingContext BindingContext
37+
{
38+
get
39+
{
40+
if (_bindingContext == null)
41+
{
42+
_bindingContext = new BindingContext(ParseResult, Console);
43+
_bindingContext.ServiceProvider.AddService(_ => GetCancellationToken());
44+
_bindingContext.ServiceProvider.AddService(_ => this);
45+
}
46+
47+
return _bindingContext;
48+
}
49+
}
3550

3651
/// <summary>
3752
/// The console to which output should be written during the current invocation.
3853
/// </summary>
39-
public IConsole Console => BindingContext.Console;
54+
public IConsole Console => _bindingContext?.Console ?? _console;
4055

4156
/// <summary>
4257
/// Enables writing help output.
@@ -46,7 +61,7 @@ public InvocationContext(
4661
/// <summary>
4762
/// The parser used to create the <see cref="ParseResult"/>.
4863
/// </summary>
49-
public Parser Parser => BindingContext.ParseResult.Parser;
64+
public Parser Parser => ParseResult.Parser;
5065

5166
/// <summary>
5267
/// Provides localizable strings for help and error messages.
@@ -56,11 +71,7 @@ public InvocationContext(
5671
/// <summary>
5772
/// The parse result for the current invocation.
5873
/// </summary>
59-
public ParseResult ParseResult
60-
{
61-
get => BindingContext.ParseResult;
62-
set => BindingContext.ParseResult = value;
63-
}
74+
public ParseResult ParseResult { get; set; }
6475

6576
/// <summary>
6677
/// A value that can be used to set the exit code for the process.

‎src/System.CommandLine/Invocation/TypoCorrection.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ public void ProvideSuggestions(ParseResult result, IConsole console)
3939
}
4040
}
4141

42-
private IEnumerable<string> GetPossibleTokens(Symbol targetSymbol, string token)
42+
private IEnumerable<string> GetPossibleTokens(Command targetSymbol, string token)
4343
{
4444
IEnumerable<string> possibleMatches = targetSymbol
4545
.Children

‎src/System.CommandLine/Option.cs

Lines changed: 46 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@
33

44
using System.Collections.Generic;
55
using System.CommandLine.Binding;
6+
using System.CommandLine.Completions;
67
using System.CommandLine.Parsing;
8+
using System.Linq;
79

810
namespace System.CommandLine
911
{
@@ -15,6 +17,7 @@ public class Option : IdentifierSymbol, IValueDescriptor
1517
{
1618
private string? _name;
1719
private List<ValidateSymbolResult<OptionResult>>? _validators;
20+
private Argument? _argument;
1821

1922
/// <summary>
2023
/// Initializes a new instance of the <see cref="Option"/> class.
@@ -90,8 +93,7 @@ internal Option(
9093

9194
for (var i = 0; i < aliases.Length; i++)
9295
{
93-
var alias = aliases[i];
94-
AddAlias(alias);
96+
AddAlias(aliases[i]);
9597
}
9698

9799
if (argument is not null)
@@ -100,6 +102,12 @@ internal Option(
100102
}
101103
}
102104

105+
private void AddArgumentInner(Argument argument)
106+
{
107+
argument.AddParent(this);
108+
_argument = argument;
109+
}
110+
103111
private static Argument? CreateArgument(Type? argumentType, Func<object?>? getDefaultValue, ArgumentArity arity)
104112
{
105113
if (argumentType is null &&
@@ -132,17 +140,14 @@ internal virtual Argument Argument
132140
{
133141
get
134142
{
135-
switch (Children.Arguments.Count)
143+
if (_argument is null)
136144
{
137-
case 0:
138-
var none = Argument.None();
139-
AddSymbol(none);
140-
return none;
141-
142-
default:
143-
DebugAssert.ThrowIf(Children.Arguments.Count > 1, $"Unexpected number of option arguments: {Children.Arguments.Count}");
144-
return Children.Arguments[0];
145+
var none = Argument.None();
146+
none.AddParent(this);
147+
_argument = none;
145148
}
149+
150+
return _argument;
146151
}
147152
}
148153

@@ -200,6 +205,8 @@ public override string Name
200205

201206
internal List<ValidateSymbolResult<OptionResult>> Validators => _validators ??= new();
202207

208+
internal bool HasValidators => _validators is not null && _validators.Count > 0;
209+
203210
/// <summary>
204211
/// Adds a validator that will be called when the option is matched by the parser.
205212
/// </summary>
@@ -257,18 +264,7 @@ public void SetDefaultValueFactory(Func<object?> getDefaultValue) =>
257264
public bool AllowMultipleArgumentsPerToken { get; set; }
258265

259266
internal virtual bool IsGreedy
260-
{
261-
get
262-
{
263-
if (Children.Count == 0 || Children.Arguments.Count == 0)
264-
{
265-
return false;
266-
}
267-
268-
var argument = Children.Arguments[0];
269-
return argument.Arity.MinimumNumberOfValues > 0 && argument.ValueType != typeof(bool);
270-
}
271-
}
267+
=> _argument is not null && _argument.Arity.MinimumNumberOfValues > 0 && _argument.ValueType != typeof(bool);
272268

273269
/// <summary>
274270
/// Indicates whether the option is required when its parent command is invoked.
@@ -301,5 +297,32 @@ private string GetLongestAlias()
301297
}
302298
return max.RemovePrefix();
303299
}
300+
301+
public override IEnumerable<CompletionItem> GetCompletions(CompletionContext context)
302+
{
303+
if (_argument is null)
304+
{
305+
return Array.Empty<CompletionItem>();
306+
}
307+
308+
List<CompletionItem>? completions = null;
309+
310+
foreach (var completion in _argument.GetCompletions(context))
311+
{
312+
if (completion.Label.ContainsCaseInsensitive(context.WordToComplete))
313+
{
314+
(completions ??= new List<CompletionItem>()).Add(completion);
315+
}
316+
}
317+
318+
if (completions is null)
319+
{
320+
return Array.Empty<CompletionItem>();
321+
}
322+
323+
return completions
324+
.OrderBy(item => item.SortText.IndexOfCaseInsensitive(context.WordToComplete))
325+
.ThenBy(symbol => symbol.Label, StringComparer.OrdinalIgnoreCase);
326+
}
304327
}
305328
}

0 commit comments

Comments
 (0)