Skip to content

Commit 88fb455

Browse files
authored
Reducing memory allocations (#2001)
* delay error list allocation until it's needed * add nullable annotation for Parser.Parse(arguments), avoid allocation when it's actually null * don't allocate a new list to normalize RootCommand * don't call EndsWith just to check one character * avoid allocations for parsing directives: * don't allocate new string to remove brackets * don't allocate an array of characters (string.Split argument) * don't allocate an array of strings (result of string.Split) * delay List<SyntaxNode> allocation * token type checks can be performed only for debug builds (these are internal methods that we control) * remove unused property that was allocating a list * delay List<OptionResult> allocation * delay List<ArgumentResult allocation * delay Dictionary<string, IReadOnlyList<string>> allocation * store the results of ParseResult.UnmatchedTokens and re-use it since it's creating a copy every time it's called! * avoid allocations when there are no unmatched tokens * Since TokenizeResult.Tokens is not public and not used anywhere after the parsing, we take advantage of its mutability and remove the root command token instead of creating a copy of the whole list. * remove TokenizeResult to reduce JIT time and allocations * delay List<ParseError> allocation * don't access Command.Options if there are no options defined (it allocates a list) * don't access Command.Arguments if there are no arguments defined (it allocates a list) * don't access Command.Subcommands if there are no commands defined (it allocates a list) * delay List<SymbolResult> allocation * delay List<Token> allocation * not every SymbolResult needs to contain a Dictionary<Argument, ArgumentResult> duplicate the code rather than introducing new public type like IdentifierSymbolResult * Aliases are internally backed by a HashSet, there is no point in creating a Union of the same hash set * avoid list allocation * address code review feedback
1 parent 23a3b3f commit 88fb455

20 files changed

+303
-255
lines changed

src/System.CommandLine/Command.cs

Lines changed: 31 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -57,18 +57,22 @@ public IEnumerable<Symbol> Children
5757
/// </summary>
5858
public IList<Argument> Arguments => _arguments ??= new(this);
5959

60-
internal bool HasArguments => _arguments is not null;
60+
internal bool HasArguments => _arguments is not null && _arguments.Count > 0 ;
6161

6262
/// <summary>
6363
/// Represents all of the options for the command, including global options that have been applied to any of the command's ancestors.
6464
/// </summary>
6565
public IList<Option> Options => _options ??= new (this);
6666

67+
internal bool HasOptions => _options is not null && _options.Count > 0;
68+
6769
/// <summary>
6870
/// Represents all of the subcommands for the command.
6971
/// </summary>
7072
public IList<Command> Subcommands => _subcommands ??= new(this);
7173

74+
internal bool HasSubcommands => _subcommands is not null && _subcommands.Count > 0;
75+
7276
/// <summary>
7377
/// Validators to the command. Validators can be used
7478
/// to create custom validation logic.
@@ -151,34 +155,44 @@ public override IEnumerable<CompletionItem> GetCompletions(CompletionContext con
151155

152156
if (context.WordToComplete is { } textToMatch)
153157
{
154-
var commands = Subcommands;
155-
for (int i = 0; i < commands.Count; i++)
158+
if (HasSubcommands)
156159
{
157-
AddCompletionsFor(commands[i]);
160+
var commands = Subcommands;
161+
for (int i = 0; i < commands.Count; i++)
162+
{
163+
AddCompletionsFor(commands[i]);
164+
}
158165
}
159166

160-
var options = Options;
161-
for (int i = 0; i < options.Count; i++)
167+
if (HasOptions)
162168
{
163-
AddCompletionsFor(options[i]);
169+
var options = Options;
170+
for (int i = 0; i < options.Count; i++)
171+
{
172+
AddCompletionsFor(options[i]);
173+
}
164174
}
165175

166-
var arguments = Arguments;
167-
for (int i = 0; i < arguments.Count; i++)
176+
if (HasArguments)
168177
{
169-
var argument = arguments[i];
170-
foreach (var completion in argument.GetCompletions(context))
178+
var arguments = Arguments;
179+
for (int i = 0; i < arguments.Count; i++)
171180
{
172-
if (completion.Label.ContainsCaseInsensitive(textToMatch))
181+
var argument = arguments[i];
182+
foreach (var completion in argument.GetCompletions(context))
173183
{
174-
completions.Add(completion);
184+
if (completion.Label.ContainsCaseInsensitive(textToMatch))
185+
{
186+
completions.Add(completion);
187+
}
175188
}
176189
}
177190
}
178191

179-
foreach (var parent in Parents.FlattenBreadthFirst(p => p.Parents))
192+
ParentNode? parent = FirstParent;
193+
while (parent is not null)
180194
{
181-
if (parent is Command parentCommand)
195+
if (parent.Symbol is Command parentCommand && parentCommand.HasOptions)
182196
{
183197
for (var i = 0; i < parentCommand.Options.Count; i++)
184198
{
@@ -190,6 +204,8 @@ public override IEnumerable<CompletionItem> GetCompletions(CompletionContext con
190204
}
191205
}
192206
}
207+
208+
parent = parent.Next;
193209
}
194210
}
195211

src/System.CommandLine/Help/HelpBuilder.Default.cs

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -201,11 +201,14 @@ public static Action<HelpContext> OptionsSection() =>
201201
// by making this logic more complex, we were able to get some nice perf wins elsewhere
202202
List<TwoColumnHelpRow> options = new();
203203
HashSet<Option> uniqueOptions = new();
204-
foreach (Option option in ctx.Command.Options)
204+
if (ctx.Command.HasOptions)
205205
{
206-
if (!option.IsHidden && uniqueOptions.Add(option))
206+
foreach (Option option in ctx.Command.Options)
207207
{
208-
options.Add(ctx.HelpBuilder.GetTwoColumnRow(option, ctx));
208+
if (!option.IsHidden && uniqueOptions.Add(option))
209+
{
210+
options.Add(ctx.HelpBuilder.GetTwoColumnRow(option, ctx));
211+
}
209212
}
210213
}
211214

@@ -218,12 +221,15 @@ public static Action<HelpContext> OptionsSection() =>
218221
{
219222
if ((parentCommand = parent.Symbol as Command) is not null)
220223
{
221-
foreach (var option in parentCommand.Options)
224+
if (parentCommand.HasOptions)
222225
{
223-
// global help aliases may be duplicated, we just ignore them
224-
if (option.IsGlobal && !option.IsHidden && uniqueOptions.Add(option))
226+
foreach (var option in parentCommand.Options)
225227
{
226-
options.Add(ctx.HelpBuilder.GetTwoColumnRow(option, ctx));
228+
// global help aliases may be duplicated, we just ignore them
229+
if (option.IsGlobal && !option.IsHidden && uniqueOptions.Add(option))
230+
{
231+
options.Add(ctx.HelpBuilder.GetTwoColumnRow(option, ctx));
232+
}
227233
}
228234
}
229235

src/System.CommandLine/Help/HelpBuilder.cs

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -125,22 +125,25 @@ IEnumerable<string> GetUsageParts()
125125
{
126126
if (!displayOptionTitle)
127127
{
128-
displayOptionTitle = parentCommand.Options.Any(x => x.IsGlobal && !x.IsHidden);
128+
displayOptionTitle = parentCommand.HasOptions && parentCommand.Options.Any(x => x.IsGlobal && !x.IsHidden);
129129
}
130130

131131
yield return parentCommand.Name;
132132

133-
yield return FormatArgumentUsage(parentCommand.Arguments);
133+
if (parentCommand.HasArguments)
134+
{
135+
yield return FormatArgumentUsage(parentCommand.Arguments);
136+
}
134137
}
135138

136-
var hasCommandWithHelp = command.Subcommands.Any(x => !x.IsHidden);
139+
var hasCommandWithHelp = command.HasSubcommands && command.Subcommands.Any(x => !x.IsHidden);
137140

138141
if (hasCommandWithHelp)
139142
{
140143
yield return LocalizationResources.HelpUsageCommand();
141144
}
142145

143-
displayOptionTitle = displayOptionTitle || command.Options.Any(x => !x.IsHidden);
146+
displayOptionTitle = displayOptionTitle || (command.HasOptions && command.Options.Any(x => !x.IsHidden));
144147

145148
if (displayOptionTitle)
146149
{

src/System.CommandLine/Invocation/TypoCorrection.cs

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -23,31 +23,39 @@ public TypoCorrection(int maxLevenshteinDistance)
2323

2424
public void ProvideSuggestions(ParseResult result, IConsole console)
2525
{
26-
for (var i = 0; i < result.UnmatchedTokens.Count; i++)
26+
var unmatchedTokens = result.UnmatchedTokens;
27+
for (var i = 0; i < unmatchedTokens.Count; i++)
2728
{
28-
var token = result.UnmatchedTokens[i];
29-
var suggestions = GetPossibleTokens(result.CommandResult.Command, token).ToList();
30-
if (suggestions.Count > 0)
29+
var token = unmatchedTokens[i];
30+
31+
bool first = true;
32+
foreach (string suggestion in GetPossibleTokens(result.CommandResult.Command, token))
3133
{
32-
console.Out.WriteLine(result.CommandResult.LocalizationResources.SuggestionsTokenNotMatched(token));
33-
foreach(string suggestion in suggestions)
34+
if (first)
3435
{
35-
console.Out.WriteLine(suggestion);
36+
console.Out.WriteLine(result.CommandResult.LocalizationResources.SuggestionsTokenNotMatched(token));
37+
first = false;
3638
}
39+
40+
console.Out.WriteLine(suggestion);
3741
}
3842
}
3943
}
4044

4145
private IEnumerable<string> GetPossibleTokens(Command targetSymbol, string token)
4246
{
47+
if (!targetSymbol.HasOptions && !targetSymbol.HasSubcommands)
48+
{
49+
return Array.Empty<string>();
50+
}
51+
4352
IEnumerable<string> possibleMatches = targetSymbol
4453
.Children
4554
.OfType<IdentifierSymbol>()
4655
.Where(x => !x.IsHidden)
4756
.Where(x => x.Aliases.Count > 0)
4857
.Select(symbol =>
4958
symbol.Aliases
50-
.Union(symbol.Aliases)
5159
.OrderBy(x => GetDistance(token, x))
5260
.ThenByDescending(x => GetStartsWithDistance(token, x))
5361
.First()

src/System.CommandLine/ParseResult.cs

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -14,44 +14,41 @@ namespace System.CommandLine
1414
/// </summary>
1515
public class ParseResult
1616
{
17-
private readonly List<ParseError> _errors;
17+
private readonly IReadOnlyList<ParseError> _errors;
1818
private readonly RootCommandResult _rootCommandResult;
1919
private readonly IReadOnlyList<Token> _unmatchedTokens;
20+
private Dictionary<string, IReadOnlyList<string>>? _directives;
2021
private CompletionContext? _completionContext;
2122

2223
internal ParseResult(
2324
Parser parser,
2425
RootCommandResult rootCommandResult,
2526
CommandResult commandResult,
26-
IReadOnlyDictionary<string, IReadOnlyList<string>> directives,
27-
TokenizeResult tokenizeResult,
27+
Dictionary<string, IReadOnlyList<string>>? directives,
28+
List<Token> tokens,
2829
IReadOnlyList<Token>? unmatchedTokens,
2930
List<ParseError>? errors,
3031
string? commandLineText = null)
3132
{
3233
Parser = parser;
3334
_rootCommandResult = rootCommandResult;
3435
CommandResult = commandResult;
35-
Directives = directives;
36+
_directives = directives;
3637

3738
// skip the root command when populating Tokens property
38-
if (tokenizeResult.Tokens.Count > 1)
39+
if (tokens.Count > 1)
3940
{
40-
var tokens = new Token[tokenizeResult.Tokens.Count - 1];
41-
for (var i = 0; i < tokenizeResult.Tokens.Count - 1; i++)
42-
{
43-
var token = tokenizeResult.Tokens[i + 1];
44-
tokens[i] = token;
45-
}
46-
41+
// Since TokenizeResult.Tokens is not public and not used anywhere after the parsing,
42+
// we take advantage of its mutability and remove the root command token
43+
// instead of creating a copy of the whole list.
44+
tokens.RemoveAt(0);
4745
Tokens = tokens;
4846
}
4947
else
5048
{
5149
Tokens = Array.Empty<Token>();
5250
}
5351

54-
_errors = errors ?? new List<ParseError>();
5552
CommandLineText = commandLineText;
5653

5754
if (unmatchedTokens is null)
@@ -67,10 +64,12 @@ internal ParseResult(
6764
for (var i = 0; i < _unmatchedTokens.Count; i++)
6865
{
6966
var token = _unmatchedTokens[i];
70-
_errors.Add(new ParseError(parser.Configuration.LocalizationResources.UnrecognizedCommandOrArgument(token.Value), rootCommandResult));
67+
(errors ??= new()).Add(new ParseError(parser.Configuration.LocalizationResources.UnrecognizedCommandOrArgument(token.Value), rootCommandResult));
7168
}
7269
}
7370
}
71+
72+
_errors = errors is not null ? errors : Array.Empty<ParseError>();
7473
}
7574

7675
internal static ParseResult Empty() => new RootCommand().Parse(Array.Empty<string>());
@@ -99,7 +98,7 @@ internal ParseResult(
9998
/// Gets the directives found while parsing command line input.
10099
/// </summary>
101100
/// <remarks>If <see cref="CommandLineConfiguration.EnableDirectives"/> is set to <see langword="false"/>, then this collection will be empty.</remarks>
102-
public IReadOnlyDictionary<string, IReadOnlyList<string>> Directives { get; }
101+
public IReadOnlyDictionary<string, IReadOnlyList<string>> Directives => _directives ??= new ();
103102

104103
/// <summary>
105104
/// Gets the tokens identified while parsing command line input.
@@ -115,7 +114,8 @@ internal ParseResult(
115114
/// <summary>
116115
/// Gets the list of tokens used on the command line that were not matched by the parser.
117116
/// </summary>
118-
public IReadOnlyList<string> UnmatchedTokens => _unmatchedTokens.Select(t => t.Value).ToArray();
117+
public IReadOnlyList<string> UnmatchedTokens
118+
=> _unmatchedTokens.Count == 0 ? Array.Empty<string>() : _unmatchedTokens.Select(t => t.Value).ToArray();
119119

120120
/// <summary>
121121
/// Gets the completion context for the parse result.

src/System.CommandLine/Parsing/ArgumentResult.cs

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -64,11 +64,14 @@ public void OnlyTake(int numberOfTokens)
6464
throw new InvalidOperationException($"{nameof(OnlyTake)} can only be called once.");
6565
}
6666

67-
var passedOnTokensCount = _tokens.Count - numberOfTokens;
67+
if (_tokens is not null)
68+
{
69+
var passedOnTokensCount = _tokens.Count - numberOfTokens;
70+
71+
PassedOnTokens = new List<Token>(_tokens.GetRange(numberOfTokens, passedOnTokensCount));
6872

69-
PassedOnTokens = new List<Token>(_tokens.GetRange(numberOfTokens, passedOnTokensCount));
70-
71-
_tokens.RemoveRange(numberOfTokens, passedOnTokensCount);
73+
_tokens.RemoveRange(numberOfTokens, passedOnTokensCount);
74+
}
7275
}
7376

7477
/// <inheritdoc/>

src/System.CommandLine/Parsing/CommandArgumentNode.cs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
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.Diagnostics;
5+
46
namespace System.CommandLine.Parsing
57
{
68
internal class CommandArgumentNode : SyntaxNode
@@ -10,10 +12,7 @@ public CommandArgumentNode(
1012
Argument argument,
1113
CommandNode parent) : base(token, parent)
1214
{
13-
if (token.Type != TokenType.Argument)
14-
{
15-
throw new ArgumentException($"Incorrect token type: {token}");
16-
}
15+
Debug.Assert(token.Type == TokenType.Argument, $"Incorrect token type: {token}");
1716

1817
Argument = argument;
1918
ParentCommandNode = parent;

src/System.CommandLine/Parsing/CommandResult.cs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,17 @@
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.Collections.Generic;
5+
46
namespace System.CommandLine.Parsing
57
{
68
/// <summary>
79
/// A result produced when parsing a <see cref="Command" />.
810
/// </summary>
911
public class CommandResult : SymbolResult
1012
{
13+
private Dictionary<Argument, ArgumentResult>? _defaultArgumentValues;
14+
1115
internal CommandResult(
1216
Command command,
1317
Token token,
@@ -36,5 +40,10 @@ internal override bool UseDefaultValueFor(Argument argument) =>
3640
arg.Tokens.Count == 0,
3741
_ => false
3842
};
43+
44+
internal ArgumentResult GetOrCreateDefaultArgumentResult(Argument argument) =>
45+
(_defaultArgumentValues ??= new()).GetOrAdd(
46+
argument,
47+
arg => new ArgumentResult(arg, this));
3948
}
4049
}

src/System.CommandLine/Parsing/DirectiveNode.cs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
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.Diagnostics;
5+
46
namespace System.CommandLine.Parsing
57
{
68
internal class DirectiveNode : SyntaxNode
@@ -11,10 +13,7 @@ public DirectiveNode(
1113
string name,
1214
string? value) : base(token, parent)
1315
{
14-
if (token.Type != TokenType.Directive)
15-
{
16-
throw new ArgumentException($"Incorrect token type: {token}");
17-
}
16+
Debug.Assert(token.Type == TokenType.Directive, $"Incorrect token type: {token}");
1817

1918
Name = name;
2019
Value = value;

0 commit comments

Comments
 (0)