Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Include corerun in Mono testhost #80082

Closed

Conversation

AustinWise
Copy link
Contributor

@AustinWise AustinWise commented Dec 31, 2022

Benchmark.NET has a --coreRun parameter that enables running a benchmark against a private build of the .NET runtime. See these instructions in dotnet/performance for how it is used. Currently the corerun program is only being included in the CoreCLR version of the testhost. This change also includes it in the Mono version of the testhost, making it slightly easier to run benchmarks against the desktop version of Mono.

I tested building ./build.sh -s mono+libs to confirm that this does not introduce a requirement to build the clr.hosts subset. I also tested running ./build.sh -s mono+libs+clr.hosts and then running Benchmark.NET against the resulting test host.

@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label Dec 31, 2022
@ghost
Copy link

ghost commented Dec 31, 2022

Tagging subscribers to this area: @dotnet/area-infrastructure-libraries
See info in area-owners.md if you want to be subscribed.

Issue Details

Benchmark.NET has a --coreRun parameter that enables running a benchmark against a private build of the .NET runtime. See these instructions in dotnet/performance for how it is used. Currently the corerun program is only being included in the CoreCLR version of the testhost. This change also includes it in the Mono version of the testhost.

I tested building ./build.sh -s mono+libs to confirm that this does not introduce a requirement to build the clr.hosts subset. I also tested running ./build.sh -s mono+libs+clr.hosts and then running Benchmark.NET against the resulting test host.

Author: AustinWise
Assignees: -
Labels:

area-Infrastructure-libraries, community-contribution

Milestone: -

@ViktorHofer
Copy link
Member

I tested building ./build.sh -s mono+libs to confirm that this does not introduce a requirement to build the clr.hosts subset.

Some subset must be building the corerun executable, otherwise it wouldn't be available. Do you know which one does that? cc @steveisok @jkotas

@AustinWise
Copy link
Contributor Author

The clr.hosts subset builds corerun. If you have not built it, building mono+libs will not include corerun in the testhost but otherwise succeed.

If this nondeterminism is undesirable, clr.hosts could be included in the mono subset.

@ViktorHofer
Copy link
Member

ViktorHofer commented Jan 2, 2023

Ideally we would not include corerun in the libraries testhost artifact folder at all, as the testhost layout should represent the shipping testhost in the SDK. We only added it back as @adamsitnik explicitly asked for it. I can't remember what the exact reason was, but I think it was BenchmarkDotNet related and using the dotnet host wasn't possible at that time (also cc @am11). Maybe we should re-evaluate BDN's corerun dependency instead?

@am11
Copy link
Member

am11 commented Jan 2, 2023

BDN supports several mono flavors (https://github.com/dotnet/BenchmarkDotNet/blob/master/src/BenchmarkDotNet/ConsoleArguments/CommandLineOptions.cs). We can specify --cli <path to dotnet.exe with mono's libcoreclr.dll and System.Private.Corelib.dll> as well as --runtime mono. AFAIK, --corerun option in BDN doesn't support mono.

@AustinWise
Copy link
Contributor Author

Thanks for the pointer to the code. I found a work around: the --coreRun option can be pointed at the dotnet.exe in the testhost and works just fine. So perhaps CoreRun does not need to be included at for any runtime.

@AustinWise AustinWise closed this Jan 2, 2023
@AustinWise AustinWise deleted the austin/MonoCoreRunTestHost branch January 2, 2023 18:08
@adamsitnik
Copy link
Member

making it slightly easier to run benchmarks against the desktop version of Mono

It looks like we need to update the docs with Mono benchmarking instructions. cc @radical @lewing

So perhaps CoreRun does not need to be included at for any runtime

I am afraid that we still need it, as it's simpler and just works.

@ViktorHofer
Copy link
Member

I am afraid that we still need it, as it's simpler and just works.

@adamsitnik can you please remind me what the technical reason was for using it over the dotnet host?

@ghost ghost locked as resolved and limited conversation to collaborators Feb 10, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Infrastructure-libraries community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants