From 52af22da5c79925949bbb61f102561fa32afa92c Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Tue, 20 Dec 2022 17:37:37 +0100 Subject: [PATCH 01/26] delay error list allocation until it's needed --- src/System.CommandLine/Parsing/StringExtensions.cs | 4 ++-- src/System.CommandLine/Parsing/TokenizeResult.cs | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/System.CommandLine/Parsing/StringExtensions.cs b/src/System.CommandLine/Parsing/StringExtensions.cs index 6fcf60328a..e4db585b35 100644 --- a/src/System.CommandLine/Parsing/StringExtensions.cs +++ b/src/System.CommandLine/Parsing/StringExtensions.cs @@ -73,7 +73,7 @@ internal static TokenizeResult Tokenize( CommandLineConfiguration configuration, bool inferRootCommand = true) { - var errorList = new List(); + List? errorList = null; var currentCommand = configuration.RootCommand; var foundDoubleDash = false; @@ -136,7 +136,7 @@ configuration.TokenReplacer is { } replacer && } else if (!string.IsNullOrWhiteSpace(error)) { - errorList.Add(error!); + (errorList ??= new()).Add(error!); continue; } } diff --git a/src/System.CommandLine/Parsing/TokenizeResult.cs b/src/System.CommandLine/Parsing/TokenizeResult.cs index 565a0cb8e7..4c396fba61 100644 --- a/src/System.CommandLine/Parsing/TokenizeResult.cs +++ b/src/System.CommandLine/Parsing/TokenizeResult.cs @@ -9,13 +9,13 @@ internal class TokenizeResult { internal TokenizeResult( List tokens, - List errors) + List? errors) { Tokens = tokens; - Errors = errors; + Errors = errors is null ? Array.Empty() : errors; } public List Tokens { get; } - public List Errors { get; } + public IReadOnlyList Errors { get; } } \ No newline at end of file From 105e5921646769a9f35853d2a25f81f3785b83f9 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Tue, 20 Dec 2022 17:53:38 +0100 Subject: [PATCH 02/26] add nullable annotation for Parser.Parse(arguments), avoid allocation when it's actually null --- src/System.CommandLine/Parsing/Parser.cs | 4 +++- src/System.CommandLine/Parsing/StringExtensions.cs | 7 +------ 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/src/System.CommandLine/Parsing/Parser.cs b/src/System.CommandLine/Parsing/Parser.cs index 3271e062fb..3a57ae95b7 100644 --- a/src/System.CommandLine/Parsing/Parser.cs +++ b/src/System.CommandLine/Parsing/Parser.cs @@ -41,9 +41,11 @@ public Parser() : this(new RootCommand()) /// The complete command line input prior to splitting and tokenization. This input is not typically available when the parser is called from Program.Main. It is primarily used when calculating completions via the dotnet-suggest tool. /// A providing details about the parse operation. public ParseResult Parse( - IReadOnlyList arguments, + IReadOnlyList? arguments, string? rawInput = null) { + arguments ??= Array.Empty(); + var tokenizeResult = arguments.Tokenize( Configuration, inferRootCommand: rawInput is not null); diff --git a/src/System.CommandLine/Parsing/StringExtensions.cs b/src/System.CommandLine/Parsing/StringExtensions.cs index e4db585b35..b4b63330f1 100644 --- a/src/System.CommandLine/Parsing/StringExtensions.cs +++ b/src/System.CommandLine/Parsing/StringExtensions.cs @@ -287,15 +287,10 @@ bool PreviousTokenIsAnOptionExpectingAnArgument(out Option? option) } private static List NormalizeRootCommand( - IReadOnlyList? args, + IReadOnlyList args, Command rootCommand, bool inferRootCommand = true) { - if (args is null) - { - args = new List(); - } - var list = new List(); if (args.Count > 0) From 02c0daf94e4c13e59949ffc99ae1a6e3a17e6ebf Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Tue, 20 Dec 2022 18:25:08 +0100 Subject: [PATCH 03/26] minor Token cleanup --- src/System.CommandLine/Parsing/Token.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/System.CommandLine/Parsing/Token.cs b/src/System.CommandLine/Parsing/Token.cs index 6798c0f694..74dc55a63d 100644 --- a/src/System.CommandLine/Parsing/Token.cs +++ b/src/System.CommandLine/Parsing/Token.cs @@ -49,7 +49,7 @@ internal Token(string? value, TokenType type, Symbol? symbol, int position) internal Symbol? Symbol { get; } /// - public override bool Equals(object? obj) => obj is Token other && Equals(other); + public override bool Equals(object? obj) => Equals(obj as Token); /// public bool Equals(Token? other) => other is not null && Value == other.Value && Type == other.Type && ReferenceEquals(Symbol, other.Symbol); @@ -75,6 +75,5 @@ internal Token(string? value, TokenType type, Symbol? symbol, int position) /// The second . /// if the objects are not equal. public static bool operator !=(Token? left, Token? right) => left is null ? right is not null : !left.Equals(right); - } } From 974108049600170f8d61f059fe8b85c16ca35aaf Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Tue, 20 Dec 2022 18:39:32 +0100 Subject: [PATCH 04/26] don't allocate a new list to normalize RootCommand --- .../Parsing/StringExtensions.cs | 68 +++++++++---------- 1 file changed, 31 insertions(+), 37 deletions(-) diff --git a/src/System.CommandLine/Parsing/StringExtensions.cs b/src/System.CommandLine/Parsing/StringExtensions.cs index b4b63330f1..58d372d27b 100644 --- a/src/System.CommandLine/Parsing/StringExtensions.cs +++ b/src/System.CommandLine/Parsing/StringExtensions.cs @@ -71,23 +71,29 @@ internal static (string? Prefix, string Alias) SplitPrefix(this string rawAlias) internal static TokenizeResult Tokenize( this IReadOnlyList args, CommandLineConfiguration configuration, - bool inferRootCommand = true) + bool inferRootCommand) { + const int FirstArgIsNotRootCommand = -1; + List? errorList = null; var currentCommand = configuration.RootCommand; var foundDoubleDash = false; var foundEndOfDirectives = !configuration.EnableDirectives; - var argList = NormalizeRootCommand(args, configuration.RootCommand, inferRootCommand); - - var tokenList = new List(argList.Count); + var tokenList = new List(args.Count); var knownTokens = configuration.RootCommand.ValidTokens(); - for (var i = 0; i < argList.Count; i++) + int i = FirstArgumentIsRootCommand(args, configuration.RootCommand, inferRootCommand) + ? 0 + : FirstArgIsNotRootCommand; + + for (; i < args.Count; i++) { - var arg = argList[i]; + var arg = i == FirstArgIsNotRootCommand + ? configuration.RootCommand.Name + : args[i]; if (foundDoubleDash) { @@ -131,7 +137,12 @@ configuration.TokenReplacer is { } replacer && out var newTokens, out var error)) { - argList.InsertRange(i + 1, newTokens ?? Array.Empty()); + if (newTokens is not null && newTokens.Count > 0) + { + List listWithReplacedTokens = args.ToList(); + listWithReplacedTokens.InsertRange(i + 1, newTokens); + args = listWithReplacedTokens; + } continue; } else if (!string.IsNullOrWhiteSpace(error)) @@ -286,48 +297,31 @@ bool PreviousTokenIsAnOptionExpectingAnArgument(out Option? option) } } - private static List NormalizeRootCommand( - IReadOnlyList args, - Command rootCommand, - bool inferRootCommand = true) + private static bool FirstArgumentIsRootCommand(IReadOnlyList args, Command rootCommand, bool inferRootCommand) { - var list = new List(); - if (args.Count > 0) { - if (inferRootCommand && - args[0] == RootCommand.ExecutablePath) + if (inferRootCommand && args[0] == RootCommand.ExecutablePath) { - list.AddRange(args); - return list; + return true; } - else + + try { - try - { - var potentialRootCommand = Path.GetFileName(args[0]); + var potentialRootCommand = Path.GetFileName(args[0]); - if (rootCommand.HasAlias(potentialRootCommand)) - { - list.AddRange(args); - return list; - } - } - catch (ArgumentException) + if (rootCommand.HasAlias(potentialRootCommand)) { - // possible exception for illegal characters in path on .NET Framework + return true; } } + catch (ArgumentException) + { + // possible exception for illegal characters in path on .NET Framework + } } - list.Add(rootCommand.Name); - - for (var i = 0; i < args.Count; i++) - { - list.Add(args[i]); - } - - return list; + return false; } private static string? GetReplaceableTokenValue(this string arg) => From d6a50268b57668454fd52c0c7fc54e57a1df4e36 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Tue, 20 Dec 2022 18:58:01 +0100 Subject: [PATCH 05/26] don't call EndsWith just to check one character --- src/System.CommandLine/Parsing/StringExtensions.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/System.CommandLine/Parsing/StringExtensions.cs b/src/System.CommandLine/Parsing/StringExtensions.cs index 58d372d27b..9aa883a90f 100644 --- a/src/System.CommandLine/Parsing/StringExtensions.cs +++ b/src/System.CommandLine/Parsing/StringExtensions.cs @@ -116,7 +116,7 @@ internal static TokenizeResult Tokenize( arg[0] == '[' && arg[1] != ']' && arg[1] != ':' && - arg.EndsWith("]", StringComparison.Ordinal)) + arg[arg.Length - 1] == ']') { tokenList.Add(Directive(arg)); continue; From bc087b617de9e3d8f485559f2061127b71b589c0 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Tue, 20 Dec 2022 19:42:01 +0100 Subject: [PATCH 06/26] 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) --- .../Parsing/ParseOperation.cs | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/src/System.CommandLine/Parsing/ParseOperation.cs b/src/System.CommandLine/Parsing/ParseOperation.cs index 1b6849c40e..9a1ddd0f2a 100644 --- a/src/System.CommandLine/Parsing/ParseOperation.cs +++ b/src/System.CommandLine/Parsing/ParseOperation.cs @@ -202,16 +202,14 @@ private void ParseDirectives(CommandNode rootCommandNode) void ParseDirective(CommandNode parent) { var token = CurrentToken; - var withoutBrackets = token.Value.Substring(1, token.Value.Length - 2); - var keyAndValue = withoutBrackets.Split(new[] - { - ':' - }, 2); - - var key = keyAndValue[0]; - var value = keyAndValue.Length == 2 - ? keyAndValue[1] - : null; + ReadOnlySpan withoutBrackets = token.Value.AsSpan(1, token.Value.Length - 2); + int commaIndex = withoutBrackets.IndexOf(':'); + string key = commaIndex >= 0 + ? withoutBrackets.Slice(0, commaIndex).ToString() + : withoutBrackets.ToString(); + string? value = commaIndex > 0 + ? withoutBrackets.Slice(commaIndex + 1).ToString() + : null; var directiveNode = new DirectiveNode(token, parent, key, value); From c38ccc1f68fdf780006bbfcd2390fac2125e9711 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Tue, 20 Dec 2022 19:44:16 +0100 Subject: [PATCH 07/26] delay List allocation --- src/System.CommandLine/Parsing/NonterminalSyntaxNode.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/System.CommandLine/Parsing/NonterminalSyntaxNode.cs b/src/System.CommandLine/Parsing/NonterminalSyntaxNode.cs index 6726d9625e..175f892232 100644 --- a/src/System.CommandLine/Parsing/NonterminalSyntaxNode.cs +++ b/src/System.CommandLine/Parsing/NonterminalSyntaxNode.cs @@ -7,14 +7,14 @@ namespace System.CommandLine.Parsing { internal abstract class NonterminalSyntaxNode : SyntaxNode { - private readonly List _children = new(); + private List? _children; protected NonterminalSyntaxNode(Token token, SyntaxNode? parent) : base(token, parent) { } - public IReadOnlyList Children => _children; + public IReadOnlyList Children => _children is not null ? _children : Array.Empty(); - internal void AddChildNode(SyntaxNode node) => _children.Add(node); + internal void AddChildNode(SyntaxNode node) => (_children ??= new()).Add(node); } } From d0e99ee0bbc766780c26858969d67e0a25853580 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Tue, 20 Dec 2022 19:51:14 +0100 Subject: [PATCH 08/26] token type checks can be performed only for debug builds (these are internal methods that we control) --- src/System.CommandLine/Parsing/CommandArgumentNode.cs | 7 +++---- src/System.CommandLine/Parsing/DirectiveNode.cs | 7 +++---- src/System.CommandLine/Parsing/OptionArgumentNode.cs | 7 +++---- 3 files changed, 9 insertions(+), 12 deletions(-) diff --git a/src/System.CommandLine/Parsing/CommandArgumentNode.cs b/src/System.CommandLine/Parsing/CommandArgumentNode.cs index 7072ccdfdc..d9d776a1fe 100644 --- a/src/System.CommandLine/Parsing/CommandArgumentNode.cs +++ b/src/System.CommandLine/Parsing/CommandArgumentNode.cs @@ -1,6 +1,8 @@ // Copyright (c) .NET Foundation and contributors. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. +using System.Diagnostics; + namespace System.CommandLine.Parsing { internal class CommandArgumentNode : SyntaxNode @@ -10,10 +12,7 @@ public CommandArgumentNode( Argument argument, CommandNode parent) : base(token, parent) { - if (token.Type != TokenType.Argument) - { - throw new ArgumentException($"Incorrect token type: {token}"); - } + Debug.Assert(token.Type == TokenType.Argument, $"Incorrect token type: {token}"); Argument = argument; ParentCommandNode = parent; diff --git a/src/System.CommandLine/Parsing/DirectiveNode.cs b/src/System.CommandLine/Parsing/DirectiveNode.cs index 4b50b0e176..e4bebe1897 100644 --- a/src/System.CommandLine/Parsing/DirectiveNode.cs +++ b/src/System.CommandLine/Parsing/DirectiveNode.cs @@ -1,6 +1,8 @@ // Copyright (c) .NET Foundation and contributors. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. +using System.Diagnostics; + namespace System.CommandLine.Parsing { internal class DirectiveNode : SyntaxNode @@ -11,10 +13,7 @@ public DirectiveNode( string name, string? value) : base(token, parent) { - if (token.Type != TokenType.Directive) - { - throw new ArgumentException($"Incorrect token type: {token}"); - } + Debug.Assert(token.Type == TokenType.Directive, $"Incorrect token type: {token}"); Name = name; Value = value; diff --git a/src/System.CommandLine/Parsing/OptionArgumentNode.cs b/src/System.CommandLine/Parsing/OptionArgumentNode.cs index 7980c9f190..36cd51006d 100644 --- a/src/System.CommandLine/Parsing/OptionArgumentNode.cs +++ b/src/System.CommandLine/Parsing/OptionArgumentNode.cs @@ -1,6 +1,8 @@ // Copyright (c) .NET Foundation and contributors. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. +using System.Diagnostics; + namespace System.CommandLine.Parsing { internal class OptionArgumentNode : SyntaxNode @@ -10,10 +12,7 @@ public OptionArgumentNode( Argument argument, OptionNode parent) : base(token, parent) { - if (token.Type != TokenType.Argument) - { - throw new ArgumentException($"Incorrect token type: {token}"); - } + Debug.Assert(token.Type == TokenType.Argument, $"Incorrect token type: {token}"); Argument = argument; ParentOptionNode = parent; From e919201d39abf3c8699bb2c547b3c0a1f18630a0 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Tue, 20 Dec 2022 20:04:27 +0100 Subject: [PATCH 09/26] remove unused property that was allocating a list --- src/System.CommandLine/Parsing/ParseOperation.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/System.CommandLine/Parsing/ParseOperation.cs b/src/System.CommandLine/Parsing/ParseOperation.cs index 9a1ddd0f2a..7170f0dfa3 100644 --- a/src/System.CommandLine/Parsing/ParseOperation.cs +++ b/src/System.CommandLine/Parsing/ParseOperation.cs @@ -21,8 +21,6 @@ public ParseOperation( private Token CurrentToken => _tokenizeResult.Tokens[_index]; - public List Errors { get; } = new(); - public CommandNode? RootCommandNode { get; private set; } public List? UnmatchedTokens { get; private set; } From 11d72b00dde85f5e90dfe64debe87b0e3a2dc628 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Tue, 20 Dec 2022 20:13:23 +0100 Subject: [PATCH 10/26] delay List allocation --- .../Parsing/ParseResultVisitor.cs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/System.CommandLine/Parsing/ParseResultVisitor.cs b/src/System.CommandLine/Parsing/ParseResultVisitor.cs index 3e3c0fa47e..f1fa71d4a0 100644 --- a/src/System.CommandLine/Parsing/ParseResultVisitor.cs +++ b/src/System.CommandLine/Parsing/ParseResultVisitor.cs @@ -21,7 +21,7 @@ internal sealed class ParseResultVisitor private readonly Dictionary _symbolResults = new(); - private readonly List _optionResults = new(); + private List? _optionResults; private readonly List _argumentResults = new(); private RootCommandResult? _rootCommandResult; @@ -113,7 +113,7 @@ private void AddToResult(OptionResult result) _innermostCommandResult?.AddChild(result); if (_symbolResults.TryAdd(result.Option, result)) { - _optionResults.Add(result); + (_optionResults ??= new()).Add(result); } } @@ -249,9 +249,12 @@ private void Stop() ValidateCommandResult(); - foreach (var optionResult in _optionResults) + if (_optionResults is not null) { - ValidateAndConvertOptionResult(optionResult); + foreach (var optionResult in _optionResults) + { + ValidateAndConvertOptionResult(optionResult); + } } if (_argumentResults.Count > 0) @@ -591,7 +594,7 @@ void Handle(SymbolResult? symbolResult, Symbol symbol) commandResult.AddChild(optionResult); if (_symbolResults.TryAdd(optionResult.Symbol, optionResult)) { - _optionResults.Add(optionResult); + (_optionResults ??= new()).Add(optionResult); } break; From 4a71f26f6d753979fa4d374b9873bf4bee25c87e Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Tue, 20 Dec 2022 20:30:12 +0100 Subject: [PATCH 11/26] delay List _symbolResults = new(); private List? _optionResults; - private readonly List _argumentResults = new(); + private List? _argumentResults; private RootCommandResult? _rootCommandResult; private CommandResult? _innermostCommandResult; @@ -122,7 +123,7 @@ private void AddToResult(ArgumentResult result) _innermostCommandResult?.AddChild(result); if (_symbolResults.TryAdd(result.Argument, result)) { - _argumentResults.Add(result); + (_argumentResults ??= new()).Add(result); } } @@ -257,25 +258,27 @@ private void Stop() } } - if (_argumentResults.Count > 0) + if (_argumentResults is not null) { - ValidateAndConvertArgumentResults(_innermostCommandResult!.Command.Arguments, _argumentResults.Count); + ValidateAndConvertArgumentResults(_innermostCommandResult!.Command.Arguments, _argumentResults); } } - private void ValidateAndConvertArgumentResults(IList arguments, int commandArgumentResultCount) + private void ValidateAndConvertArgumentResults(IList arguments, List argumentResults) { + int commandArgumentResultCount = argumentResults.Count; + for (var i = 0; i < arguments.Count; i++) { - if (i > 0 && _argumentResults.Count > i) + if (i > 0 && argumentResults.Count > i) { - var previousArgumentResult = _argumentResults[i - 1]; + var previousArgumentResult = argumentResults[i - 1]; var passedOnTokensCount = previousArgumentResult.PassedOnTokens?.Count; if (passedOnTokensCount > 0) { - ShiftPassedOnTokensToNextResult(previousArgumentResult, _argumentResults[i], passedOnTokensCount); + ShiftPassedOnTokensToNextResult(previousArgumentResult, argumentResults[i], passedOnTokensCount); } } @@ -287,13 +290,13 @@ private void ValidateAndConvertArgumentResults(IList arguments, int co nextArgument, _innermostCommandResult); - var previousArgumentResult = _argumentResults[i - 1]; + var previousArgumentResult = argumentResults[i - 1]; var passedOnTokensCount = _innermostCommandResult?.Tokens.Count; ShiftPassedOnTokensToNextResult(previousArgumentResult, nextArgumentResult, passedOnTokensCount); - _argumentResults.Add(nextArgumentResult); + argumentResults.Add(nextArgumentResult); if (previousArgumentResult.Parent is CommandResult) { @@ -303,9 +306,9 @@ private void ValidateAndConvertArgumentResults(IList arguments, int co _symbolResults.TryAdd(nextArgumentResult.Symbol, nextArgumentResult); } - if (commandArgumentResultCount >= _argumentResults.Count) + if (commandArgumentResultCount >= argumentResults.Count) { - var argumentResult = _argumentResults[i]; + var argumentResult = argumentResults[i]; ValidateAndConvertArgumentResult(argumentResult); @@ -318,11 +321,11 @@ private void ValidateAndConvertArgumentResults(IList arguments, int co } } - if (_argumentResults.Count > arguments.Count) + if (argumentResults.Count > arguments.Count) { - for (var i = arguments.Count; i < _argumentResults.Count - 1; i++) + for (var i = arguments.Count; i < argumentResults.Count - 1; i++) { - var result = _argumentResults[i]; + var result = argumentResults[i]; if (result.Parent is CommandResult) { From c0e3594ba84e7ade45fe3936e48a96b4c9768528 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Tue, 20 Dec 2022 20:35:50 +0100 Subject: [PATCH 12/26] delay Dictionary> allocation --- src/System.CommandLine/ParseResult.cs | 7 ++++--- src/System.CommandLine/Parsing/ParseResultVisitor.cs | 6 +++--- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/System.CommandLine/ParseResult.cs b/src/System.CommandLine/ParseResult.cs index 8c0b61b167..0aaf32b946 100644 --- a/src/System.CommandLine/ParseResult.cs +++ b/src/System.CommandLine/ParseResult.cs @@ -17,13 +17,14 @@ public class ParseResult private readonly List _errors; private readonly RootCommandResult _rootCommandResult; private readonly IReadOnlyList _unmatchedTokens; + private Dictionary>? _directives; private CompletionContext? _completionContext; internal ParseResult( Parser parser, RootCommandResult rootCommandResult, CommandResult commandResult, - IReadOnlyDictionary> directives, + Dictionary>? directives, TokenizeResult tokenizeResult, IReadOnlyList? unmatchedTokens, List? errors, @@ -32,7 +33,7 @@ internal ParseResult( Parser = parser; _rootCommandResult = rootCommandResult; CommandResult = commandResult; - Directives = directives; + _directives = directives; // skip the root command when populating Tokens property if (tokenizeResult.Tokens.Count > 1) @@ -99,7 +100,7 @@ internal ParseResult( /// Gets the directives found while parsing command line input. /// /// If is set to , then this collection will be empty. - public IReadOnlyDictionary> Directives { get; } + public IReadOnlyDictionary> Directives => _directives ??= new (); /// /// Gets the tokens identified while parsing command line input. diff --git a/src/System.CommandLine/Parsing/ParseResultVisitor.cs b/src/System.CommandLine/Parsing/ParseResultVisitor.cs index d527dd4ba0..6459c7a97e 100644 --- a/src/System.CommandLine/Parsing/ParseResultVisitor.cs +++ b/src/System.CommandLine/Parsing/ParseResultVisitor.cs @@ -16,7 +16,7 @@ internal sealed class ParseResultVisitor private readonly TokenizeResult _tokenizeResult; private readonly string? _rawInput; - private readonly Dictionary> _directives = new(); + private Dictionary>? _directives; private List? _unmatchedTokens; private readonly List _errors; @@ -224,11 +224,11 @@ private void VisitOptionArgumentNode( private void VisitDirectiveNode(DirectiveNode directiveNode) { - if (!_directives.TryGetValue(directiveNode.Name, out var values)) + if (_directives is null || !_directives.TryGetValue(directiveNode.Name, out var values)) { values = new List(); - _directives.Add(directiveNode.Name, values); + (_directives ??= new()).Add(directiveNode.Name, values); } if (directiveNode.Value is not null) From e35365642f786fc833ac8dd6b98e47a1bfcc3dbf Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Tue, 20 Dec 2022 21:02:32 +0100 Subject: [PATCH 13/26] store the results of ParseResult.UnmatchedTokens and re-use it since it's creating a copy every time it's called! --- src/System.CommandLine/Invocation/TypoCorrection.cs | 5 +++-- src/System.CommandLine/Parsing/ParseResultExtensions.cs | 8 +++++--- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/System.CommandLine/Invocation/TypoCorrection.cs b/src/System.CommandLine/Invocation/TypoCorrection.cs index 5ded31a9eb..a845a54060 100644 --- a/src/System.CommandLine/Invocation/TypoCorrection.cs +++ b/src/System.CommandLine/Invocation/TypoCorrection.cs @@ -23,9 +23,10 @@ public TypoCorrection(int maxLevenshteinDistance) public void ProvideSuggestions(ParseResult result, IConsole console) { - for (var i = 0; i < result.UnmatchedTokens.Count; i++) + var unmatchedTokens = result.UnmatchedTokens; + for (var i = 0; i < unmatchedTokens.Count; i++) { - var token = result.UnmatchedTokens[i]; + var token = unmatchedTokens[i]; var suggestions = GetPossibleTokens(result.CommandResult.Command, token).ToList(); if (suggestions.Count > 0) { diff --git a/src/System.CommandLine/Parsing/ParseResultExtensions.cs b/src/System.CommandLine/Parsing/ParseResultExtensions.cs index 7cd3ebb8d6..54eb8fa8f3 100644 --- a/src/System.CommandLine/Parsing/ParseResultExtensions.cs +++ b/src/System.CommandLine/Parsing/ParseResultExtensions.cs @@ -2,6 +2,7 @@ // Licensed under the MIT license. See LICENSE file in the project root for full license information. using System.Collections; +using System.Collections.Generic; using System.CommandLine.Binding; using System.CommandLine.Invocation; using System.Linq; @@ -53,13 +54,14 @@ public static string Diagram(this ParseResult parseResult) { builder.Diagram(parseResult.RootCommandResult, parseResult); - if (parseResult.UnmatchedTokens.Count > 0) + var unmatchedTokens = parseResult.UnmatchedTokens; + if (unmatchedTokens.Count > 0) { builder.Append(" ???-->"); - for (var i = 0; i < parseResult.UnmatchedTokens.Count; i++) + for (var i = 0; i < unmatchedTokens.Count; i++) { - var error = parseResult.UnmatchedTokens[i]; + var error = unmatchedTokens[i]; builder.Append(" "); builder.Append(error); } From b9c4d42c7ad402a2637dea827fe3affbb7146d3a Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Tue, 20 Dec 2022 21:02:51 +0100 Subject: [PATCH 14/26] avoid allocations when there are no unmatched tokens --- src/System.CommandLine/ParseResult.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/System.CommandLine/ParseResult.cs b/src/System.CommandLine/ParseResult.cs index 0aaf32b946..f313518887 100644 --- a/src/System.CommandLine/ParseResult.cs +++ b/src/System.CommandLine/ParseResult.cs @@ -116,7 +116,8 @@ internal ParseResult( /// /// Gets the list of tokens used on the command line that were not matched by the parser. /// - public IReadOnlyList UnmatchedTokens => _unmatchedTokens.Select(t => t.Value).ToArray(); + public IReadOnlyList UnmatchedTokens + => _unmatchedTokens.Count == 0 ? Array.Empty() : _unmatchedTokens.Select(t => t.Value).ToArray(); /// /// Gets the completion context for the parse result. From ac725a3df71f324b34b505244607f31928ed0a60 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Wed, 21 Dec 2022 15:35:43 +0100 Subject: [PATCH 15/26] 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. --- src/System.CommandLine/ParseResult.cs | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/System.CommandLine/ParseResult.cs b/src/System.CommandLine/ParseResult.cs index f313518887..0f9c856068 100644 --- a/src/System.CommandLine/ParseResult.cs +++ b/src/System.CommandLine/ParseResult.cs @@ -38,14 +38,11 @@ internal ParseResult( // skip the root command when populating Tokens property if (tokenizeResult.Tokens.Count > 1) { - var tokens = new Token[tokenizeResult.Tokens.Count - 1]; - for (var i = 0; i < tokenizeResult.Tokens.Count - 1; i++) - { - var token = tokenizeResult.Tokens[i + 1]; - tokens[i] = token; - } - - Tokens = 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. + tokenizeResult.Tokens.RemoveAt(0); + Tokens = tokenizeResult.Tokens; } else { From fea7753409538b44e6df9a25db215099edcc735d Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Wed, 21 Dec 2022 15:51:13 +0100 Subject: [PATCH 16/26] remove TokenizeResult to reduce JIT time and allocations --- src/System.CommandLine/ParseResult.cs | 8 +++---- .../Parsing/ParseOperation.cs | 12 +++++------ .../Parsing/ParseResultVisitor.cs | 20 +++++++++++------- src/System.CommandLine/Parsing/Parser.cs | 11 ++++++---- .../Parsing/StringExtensions.cs | 10 ++++++--- .../Parsing/TokenizeResult.cs | 21 ------------------- 6 files changed, 36 insertions(+), 46 deletions(-) delete mode 100644 src/System.CommandLine/Parsing/TokenizeResult.cs diff --git a/src/System.CommandLine/ParseResult.cs b/src/System.CommandLine/ParseResult.cs index 0f9c856068..4167edc3a9 100644 --- a/src/System.CommandLine/ParseResult.cs +++ b/src/System.CommandLine/ParseResult.cs @@ -25,7 +25,7 @@ internal ParseResult( RootCommandResult rootCommandResult, CommandResult commandResult, Dictionary>? directives, - TokenizeResult tokenizeResult, + List tokens, IReadOnlyList? unmatchedTokens, List? errors, string? commandLineText = null) @@ -36,13 +36,13 @@ internal ParseResult( _directives = directives; // skip the root command when populating Tokens property - if (tokenizeResult.Tokens.Count > 1) + if (tokens.Count > 1) { // 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. - tokenizeResult.Tokens.RemoveAt(0); - Tokens = tokenizeResult.Tokens; + tokens.RemoveAt(0); + Tokens = tokens; } else { diff --git a/src/System.CommandLine/Parsing/ParseOperation.cs b/src/System.CommandLine/Parsing/ParseOperation.cs index 7170f0dfa3..6d48ca8990 100644 --- a/src/System.CommandLine/Parsing/ParseOperation.cs +++ b/src/System.CommandLine/Parsing/ParseOperation.cs @@ -7,19 +7,19 @@ namespace System.CommandLine.Parsing { internal class ParseOperation { - private readonly TokenizeResult _tokenizeResult; + private readonly List _tokens; private readonly CommandLineConfiguration _configuration; private int _index; public ParseOperation( - TokenizeResult tokenizeResult, + List tokens, CommandLineConfiguration configuration) { - _tokenizeResult = tokenizeResult; + _tokens = tokens; _configuration = configuration; } - private Token CurrentToken => _tokenizeResult.Tokens[_index]; + private Token CurrentToken => _tokens[_index]; public CommandNode? RootCommandNode { get; private set; } @@ -29,8 +29,8 @@ public ParseOperation( private bool More(out TokenType currentTokenType) { - bool result = _index < _tokenizeResult.Tokens.Count; - currentTokenType = result ? _tokenizeResult.Tokens[_index].Type : (TokenType)(-1); + bool result = _index < _tokens.Count; + currentTokenType = result ? _tokens[_index].Type : (TokenType)(-1); return result; } diff --git a/src/System.CommandLine/Parsing/ParseResultVisitor.cs b/src/System.CommandLine/Parsing/ParseResultVisitor.cs index 6459c7a97e..54461d2a19 100644 --- a/src/System.CommandLine/Parsing/ParseResultVisitor.cs +++ b/src/System.CommandLine/Parsing/ParseResultVisitor.cs @@ -13,7 +13,7 @@ namespace System.CommandLine.Parsing internal sealed class ParseResultVisitor { private readonly Parser _parser; - private readonly TokenizeResult _tokenizeResult; + private readonly List _tokens; private readonly string? _rawInput; private Dictionary>? _directives; @@ -29,22 +29,26 @@ internal sealed class ParseResultVisitor private CommandResult? _innermostCommandResult; private bool _isHelpRequested; - public ParseResultVisitor( + internal ParseResultVisitor( Parser parser, - TokenizeResult tokenizeResult, + List tokens, + List? tokenizeErrors, List? unmatchedTokens, string? rawInput) { _parser = parser; - _tokenizeResult = tokenizeResult; + _tokens = tokens; _unmatchedTokens = unmatchedTokens; _rawInput = rawInput; - _errors = new List(_tokenizeResult.Errors.Count); - for (var i = 0; i < _tokenizeResult.Errors.Count; i++) { - var error = _tokenizeResult.Errors[i]; - _errors.Add(new ParseError(error)); + _errors = new List(tokenizeErrors.Count); + + for (var i = 0; i < tokenizeErrors.Count; i++) + { + var error = tokenizeErrors[i]; + _errors.Add(new ParseError(error)); + } } } diff --git a/src/System.CommandLine/Parsing/Parser.cs b/src/System.CommandLine/Parsing/Parser.cs index 3a57ae95b7..6ea53be755 100644 --- a/src/System.CommandLine/Parsing/Parser.cs +++ b/src/System.CommandLine/Parsing/Parser.cs @@ -46,19 +46,22 @@ public ParseResult Parse( { arguments ??= Array.Empty(); - var tokenizeResult = arguments.Tokenize( + arguments.Tokenize( Configuration, - inferRootCommand: rawInput is not null); + inferRootCommand: rawInput is not null, + out List tokens, + out List? tokenizationErrors); var operation = new ParseOperation( - tokenizeResult, + tokens, Configuration); operation.Parse(); var visitor = new ParseResultVisitor( this, - tokenizeResult, + tokens, + tokenizationErrors, operation.UnmatchedTokens, rawInput); diff --git a/src/System.CommandLine/Parsing/StringExtensions.cs b/src/System.CommandLine/Parsing/StringExtensions.cs index 9aa883a90f..eea8b03d9b 100644 --- a/src/System.CommandLine/Parsing/StringExtensions.cs +++ b/src/System.CommandLine/Parsing/StringExtensions.cs @@ -68,10 +68,13 @@ internal static (string? Prefix, string Alias) SplitPrefix(this string rawAlias) return (null, rawAlias); } - internal static TokenizeResult Tokenize( + // this method is not returning a Value Tuple or a dedicated type to avoid JITting + internal static void Tokenize( this IReadOnlyList args, CommandLineConfiguration configuration, - bool inferRootCommand) + bool inferRootCommand, + out List tokens, + out List? errors) { const int FirstArgIsNotRootCommand = -1; @@ -219,7 +222,8 @@ configuration.TokenReplacer is { } replacer && Token Directive(string value) => new(value, TokenType.Directive, default, i); } - return new TokenizeResult(tokenList, errorList); + tokens = tokenList; + errors = errorList; bool CanBeUnbundled(string arg) => arg.Length > 2 diff --git a/src/System.CommandLine/Parsing/TokenizeResult.cs b/src/System.CommandLine/Parsing/TokenizeResult.cs deleted file mode 100644 index 4c396fba61..0000000000 --- a/src/System.CommandLine/Parsing/TokenizeResult.cs +++ /dev/null @@ -1,21 +0,0 @@ -// Copyright (c) .NET Foundation and contributors. All rights reserved. -// Licensed under the MIT license. See LICENSE file in the project root for full license information. - -using System.Collections.Generic; - -namespace System.CommandLine.Parsing; - -internal class TokenizeResult -{ - internal TokenizeResult( - List tokens, - List? errors) - { - Tokens = tokens; - Errors = errors is null ? Array.Empty() : errors; - } - - public List Tokens { get; } - - public IReadOnlyList Errors { get; } -} \ No newline at end of file From a31e1eca492cd26d2273a46fcb72f1b7a025f745 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Wed, 21 Dec 2022 15:51:51 +0100 Subject: [PATCH 17/26] delay List allocation --- src/System.CommandLine/ParseResult.cs | 7 ++++--- src/System.CommandLine/Parsing/ParseResultVisitor.cs | 10 +++++----- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/System.CommandLine/ParseResult.cs b/src/System.CommandLine/ParseResult.cs index 4167edc3a9..b4573b78b0 100644 --- a/src/System.CommandLine/ParseResult.cs +++ b/src/System.CommandLine/ParseResult.cs @@ -14,7 +14,7 @@ namespace System.CommandLine /// public class ParseResult { - private readonly List _errors; + private readonly IReadOnlyList _errors; private readonly RootCommandResult _rootCommandResult; private readonly IReadOnlyList _unmatchedTokens; private Dictionary>? _directives; @@ -49,7 +49,6 @@ internal ParseResult( Tokens = Array.Empty(); } - _errors = errors ?? new List(); CommandLineText = commandLineText; if (unmatchedTokens is null) @@ -65,10 +64,12 @@ internal ParseResult( for (var i = 0; i < _unmatchedTokens.Count; i++) { var token = _unmatchedTokens[i]; - _errors.Add(new ParseError(parser.Configuration.LocalizationResources.UnrecognizedCommandOrArgument(token.Value), rootCommandResult)); + (errors ??= new()).Add(new ParseError(parser.Configuration.LocalizationResources.UnrecognizedCommandOrArgument(token.Value), rootCommandResult)); } } } + + _errors = errors is not null ? errors : Array.Empty(); } internal static ParseResult Empty() => new RootCommand().Parse(Array.Empty()); diff --git a/src/System.CommandLine/Parsing/ParseResultVisitor.cs b/src/System.CommandLine/Parsing/ParseResultVisitor.cs index 54461d2a19..98edca06d1 100644 --- a/src/System.CommandLine/Parsing/ParseResultVisitor.cs +++ b/src/System.CommandLine/Parsing/ParseResultVisitor.cs @@ -5,7 +5,6 @@ using System.CommandLine.Binding; using System.CommandLine.Completions; using System.CommandLine.Help; -using System.Diagnostics; using System.Linq; namespace System.CommandLine.Parsing @@ -18,7 +17,7 @@ internal sealed class ParseResultVisitor private Dictionary>? _directives; private List? _unmatchedTokens; - private readonly List _errors; + private List? _errors; private readonly Dictionary _symbolResults = new(); @@ -41,6 +40,7 @@ internal ParseResultVisitor( _unmatchedTokens = unmatchedTokens; _rawInput = rawInput; + if (tokenizeErrors is not null) { _errors = new List(tokenizeErrors.Count); @@ -448,7 +448,7 @@ private void ValidateCommandHandler() return; } - _errors.Insert( + (_errors ??= new()).Insert( 0, new ParseError( _innermostCommandResult.LocalizationResources.RequiredCommandWasNotProvided(), @@ -628,7 +628,7 @@ private void AddErrorToResult(SymbolResult symbolResult, ParseError parseError) optionResult.ErrorMessage ??= symbolResult.ErrorMessage; } - _errors.Add(parseError); + (_errors ??= new()).Add(parseError); } public ParseResult GetResult() => @@ -636,7 +636,7 @@ public ParseResult GetResult() => _rootCommandResult ?? throw new InvalidOperationException("No root command was found"), _innermostCommandResult ?? throw new InvalidOperationException("No command was found"), _directives, - _tokenizeResult, + _tokens, _unmatchedTokens, _errors, _rawInput); From 100234333b76f87e301e9ea133e68c52379e0b44 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Wed, 21 Dec 2022 16:23:59 +0100 Subject: [PATCH 18/26] don't access Command.Options if there are no options defined (it allocates a list) --- src/System.CommandLine/Command.cs | 18 +++++++--- .../Help/HelpBuilder.Default.cs | 20 +++++++---- src/System.CommandLine/Help/HelpBuilder.cs | 4 +-- .../Parsing/ParseResultVisitor.cs | 36 +++++++++++-------- .../Parsing/StringExtensions.cs | 30 +++++++++------- 5 files changed, 67 insertions(+), 41 deletions(-) diff --git a/src/System.CommandLine/Command.cs b/src/System.CommandLine/Command.cs index c24d041660..b7bbfdc75f 100644 --- a/src/System.CommandLine/Command.cs +++ b/src/System.CommandLine/Command.cs @@ -64,6 +64,8 @@ public IEnumerable Children /// public IList