dotnet / BenchmarkDotNet

Powerful .NET library for benchmarking
https://benchmarkdotnet.org
MIT License
10.62k stars 973 forks source link

Switch from CommandLineParser and McMaster.Extensions.CommandLineUtils to System.CommandLine #1016

Open adamsitnik opened 5 years ago

adamsitnik commented 5 years ago

As of today, we are using:

Having two dependencies to parse command line arguments is not good.

I think that we should switch to System.CommandLine if possible (project)

System.CommandLine is the future of parsing command line arguments in .NET. It's not a MS-only project, it has been built together by the community and MS based on experiences from many existing command line parsing projects.

I have seen a 1h internal demo at MS and to tell the long story short it has everything that CommandLineParser and McMaster.Extensions.CommandLineUtils have and a lot of more cool features. You can read more about the motivations for it here

What this task requires:

  1. Make sure that System.CommandLine supports everything that CommandLineParser gives us as of today:
    • parsing simple arguments (1 value)
    • parsing more complex arguments (IEnumerable<string>)
    • parsing arguments with aliases (example: -i --inprocess)
    • help with samples
  2. Making sure that System.CommandLine supports everything that McMaster.Extensions.CommandLineUtils gives us as of today for the global tool

The global tool was merged to https://github.com/dotnet/BenchmarkDotNet/tree/tools branch. So a person working on this task would have to create branch out of tools branch.

If System.CommandLine meets all our needs, then I can talk with @KathleenDollard about publishing a signed package to Nuget.org and we can switch (https://github.com/dotnet/command-line-api/issues/356)

@wojtpl2 perhaps you would be interested in this issue?

WojciechNagorski commented 5 years ago

@adamsitnik I would like to do this but I think System.CommandLine is still not ready yet. There is more information https://github.com/dotnet/command-line-api/pull/345#issuecomment-451575313.

@KathleenDollard made great PR https://github.com/dotnet/command-line-api/pull/363 but it is not finished yet. This approache for me it will be the best option. Something similar to CommandLineParser.

Currently we can use only core APIs directly from System.CommandLine like here: https://github.com/dotnet/command-line-api/wiki/Your-first-app-with-System.CommandLine Of course I can use it, but keep in mind that this code will change in the future.

WojciechNagorski commented 5 years ago

@adamsitnik If we want to use System.CommandLine we need published package to Nuget.org or at least a preview version.

WojciechNagorski commented 5 years ago

System.CommandLine supports:

I don't know yet:

I'll create PR next week.

some preview:

C:\Users\wnagorsx\source\repos\ConsoleApp7\ConsoleApp7
λ dotnet run -- --job Dry -m 1
Unrecognized command or argument '1'

ConsoleApp7:
  BDN :)

Usage:
  ConsoleApp7 [options]

Options:
  -j, --job          Dry/Short/Medium/Long or Default
  -r, --runtimes     Full target framework moniker for .NET Core and .NET. For Mono just 'Mono', for CoreRT just 'CoreRT'. First one will be marked as baseline!
  -e, --exporters    GitHub/StackOverflow/RPlot/CSV/JSON/HTML/XML
  -m, --memory       Prints memory statistics
  --version          Display version information
WojciechNagorski commented 5 years ago

I found one problem in System.CommandLine. It supports multiple value, but in different way. You always have to repeat alias of parameter for each value like this: --runtime net472 --runtime netcoreapp2.1

I created issue in System.CommandLine https://github.com/dotnet/command-line-api/issues/383

WojciechNagorski commented 5 years ago

My PR https://github.com/dotnet/command-line-api/pull/384 to System.CommandLine is marged. Now we can switch to System.CommandLine.

WojciechNagorski commented 5 years ago

I created second PR https://github.com/dotnet/command-line-api/pull/385

adamsitnik commented 5 years ago

@wojtpl2 awesome! great job!

WojciechNagorski commented 5 years ago

I push changes to https://github.com/wojtpl2/BenchmarkDotNet/tree/system_commandLine All test are passed: image Example output:

BenchmarkDotNet.Samples:
  BenchmarkDotNet

Usage:
  BenchmarkDotNet.Samples [options]

Options:
  -j, --job <J>                                  Dry/Short/Medium/Long or Default
  -r, --runtimes <R>                             Full target framework moniker for .NET Core and .NET. For Mono just 'Mono', for CoreRT just 'CoreRT'. First one will be marked as baseline!
  -e, --exporters <E>                            GitHub/StackOverflow/RPlot/CSV/JSON/HTML/XML
  -m, --memory <M>                               Prints memory statistics
  -d, --disasm <D>                               Gets disassembly of benchmarked code
  -p, --profiler <P>                             Profiles benchmarked code using selected profiler. Currently the only available is "ETW" for Windows.
  -f, --filter <F>                               Glob patterns
  -i, --inProcess <I>                            Run benchmarks in Process
  -a, --artifacts <A>                            Valid path to accessible directory
  --outliers <OUTLIERS>                          None/OnlyUpper/OnlyLower/All
  --affinity <AFFINITY>                          Affinity mask to set for the benchmark process
  --allStats <ALLSTATS>                          Displays all statistics (min, max & more)
  --allCategories <ALLCATEGORIES>                Categories to run. If few are provided, only the benchmarks which belong to all of them are going to be executed
  --anyCategories <ANYCATEGORIES>                Any Categories to run
  --attribute <ATTRIBUTE>                        Run all methods with given attribute (applied to class or method)
  --join <JOIN>                                  Prints single table with results for all benchmarks
  --keepFiles <KEEPFILES>                        Determines if all auto-generated files should be kept or removed after running the benchmarks.
  --counters <COUNTERS>                          Hardware Counters
  --cli <CLI>                                    Path to dotnet cli (optional).
  --packages <PACKAGES>                          The directory to restore packages to (optional).
  --coreRun <CORERUN>                            Path(s) to CoreRun (optional).
  --monoPath <MONOPATH>                          Optional path to Mono which should be used for running benchmarks.
  --clrVersion <CLRVERSION>                      Optional version of private CLR build used as the value of COMPLUS_Version env var.
  --coreRtVersion <CORERTVERSION>                Optional version of Microsoft.DotNet.ILCompiler which should be used to run with CoreRT. Example: "1.0.0-alpha-26414-01"
  --ilcPath <ILCPATH>                            Optional IlcPath which should be used to run with private CoreRT build.
  --launchCount <LAUNCHCOUNT>                    Affinity mask to set for the benchmark process
  --warmupCount <WARMUPCOUNT>                    How many warmup iterations should be performed. If you set it, the minWarmupCount and maxWarmupCount are ignored. By default calculated by the heuristic.
  --minWarmupCount <MINWARMUPCOUNT>              Minimum count of warmup iterations that should be performed. The default is 6.
  --maxWarmupCount <MAXWARMUPCOUNT>              Maximum count of warmup iterations that should be performed. The default is 50.
  --iterationTime <ITERATIONTIME>                Desired time of execution of an iteration in milliseconds. Used by Pilot stage to estimate the number of invocations per iteration. 500ms by default
  --iterationCount <ITERATIONCOUNT>              How many target iterations should be performed. By default calculated by the heuristic.
  --minIterationCount <MINITERATIONCOUNT>        Minimum number of iterations to run. The default is 15.
  --maxIterationCount <MAXITERATIONCOUNT>        Maximum number of iterations to run. The default is 100.
  --invocationCount <INVOCATIONCOUNT>            Invocation count in a single iteration. By default calculated by the heuristic.
  --unrollFactor <UNROLLFACTOR>                  How many times the benchmark method will be invoked per one iteration of a generated loop. 16 by default
  --runOncePerIteration <RUNONCEPERITERATION>    Run the benchmark exactly once per iteration.
  --info <INFO>                                  Run the benchmark exactly once per iteration.
  --list <LIST>                                  Prints all of the available benchmark names. Flat/Tree
  --disasmDepth <DISASMDEPTH>                    Sets the recursive depth for the disassembler.
  --disasmDiff <DISASMDIFF>                      Generates diff reports for the disassembler.
  --buildTimeout <BUILDTIMEOUT>                  Build timeout in seconds.
  --stopOnFirstError <STOPONFIRSTERROR>          Stop on first error.
  --statisticalTest <STATISTICALTEST>            Threshold for Mann-Whitney U Test. Examples: 5%, 10ms, 100ns, 1s
  --version                                      Display version information

But there is still some work to do. I'm waiting for third PR https://github.com/dotnet/command-line-api/pull/389 and for published version of System.CommandLine.

@adamsitnik Should I wait for published version of System.CommandLine or should I use version from MyGet?

dotnet add package --source https://dotnet.myget.org/F/system-commandline/api/v3/index.json System.CommandLine.Experimental -v 0.1.0-alpha-63724-02
adamsitnik commented 5 years ago

I have left a suggestion for https://github.com/dotnet/command-line-api/pull/389#pullrequestreview-197358801

Should I wait for published version of System.CommandLine

You can open a PR. As soon as @kathleendollard let's me know that System.CommandLine will be published to nuget.org I am going to merge it

Once again big thanks for doing this!

WojciechNagorski commented 5 years ago

There is still some missing function in System.CommandLine:

adamsitnik commented 5 years ago

Parameter value with + separator

it's nice to have, but not mandatory

Case sensitive - BDN supports case sensitive

it depends on what is the golden standard of command line tools - should every tool be case sensitive or insensitive?

Examples of usage

this is a must have, without it the --help option is not very usefull

jonsequitur commented 5 years ago

Windows command line tools are more often case insensitive and *nix/macOS tools are usually case sensitive. (Think git branch -d vs git branch -D.)

System.CommandLine can support different casing like the example above via aliases but it would be awkward for example if you want to support all mixed case variants, e.g. --eXpoRterS.

adamsitnik commented 5 years ago

@jonsequitur thanks for the explanation! in such case, I think that it's fine to be case-sensitive (we might add some aliases, but rather very few)

jzabroski commented 5 years ago

Interesting. I prefer FluentCommandLineParser, personally. I dislike Attributes on my classes.

Related to this, I know some of you work for Microsoft, and I had previously requested the .NET team consider modifying the .dll format so that System.CommandLine parameters were discoverable via PowerShell so that we could have auto-complete, but the issue was closed. See: https://github.com/dotnet/coreclr/issues/19821

jzabroski commented 5 years ago

It might be an interesting Trojan Horse to get my idea adopted if I build a PowerShell script that enables auto-completion for benchmarkDotNet. Thoughts? (I felt the response in the CoreCLR thread was one of "I don't see a demo so its not worth doing" circular logic).

jonsequitur commented 5 years ago

Why limit it to Powershell? :)

The new System.CommandLine does the heavy lifting for completions regardless of the shell:

https://github.com/dotnet/command-line-api/wiki/Features-overview#Suggestions

jzabroski commented 5 years ago

@jonsequitur PERFECT! When did this happen??

WojciechNagorski commented 5 years ago

@adamsitnik I've seen that `System.CommandLine' is on Nuget: https://www.nuget.org/packages/System.CommandLine.Experimental/0.2.0-alpha.19156.3

I'm going to update my PR, this or next week.

adamsitnik commented 5 years ago

@wojtpl2 awesome! thanks for all your contributions to System.CommandLine to make the port possible!

JobaDiniz commented 1 year ago

Is this still needed? I have experience with System.ComandLine and would like to grab this issue if BenchmarkDotNet team are still interested to make this move.

AndreyAkinshin commented 1 year ago

@JobaDiniz Unfortunately, System.CommandLine is still in the preview stage (the current version is 2.0.0-beta4.22272.1 published on June 02, 2022). NuGet doesn't allow to create a stable version of a package that references another prerelease package. Therefore, if we switch to System.CommandLine right now, we will not be able to publish stable versions of BenchmarkDotNet.

While switching to System.CommandLine still looks reasonable, we have to wait until a stable version of this package is released.