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

Target .NET Core 2.0 to take advantage of the new APIs #539

Closed
adamsitnik opened this Issue Sep 5, 2017 · 17 comments

Comments

Projects
None yet
4 participants
@adamsitnik
Member

adamsitnik commented Sep 5, 2017

No description provided.

@adamsitnik adamsitnik added this to the v0.10.10 milestone Sep 5, 2017

@adamsitnik adamsitnik self-assigned this Sep 5, 2017

@AndreyAkinshin

This comment has been minimized.

Show comment
Hide comment
@AndreyAkinshin

AndreyAkinshin Sep 5, 2017

Member

@adamsitnik, how are you going to implement it? I think we should continue to support .NET Core 1.1 (but we also can target .NET Core 2.0 as well and use new API when it's possible).

Member

AndreyAkinshin commented Sep 5, 2017

@adamsitnik, how are you going to implement it? I think we should continue to support .NET Core 1.1 (but we also can target .NET Core 2.0 as well and use new API when it's possible).

@adamsitnik

This comment has been minimized.

Show comment
Hide comment
@adamsitnik

adamsitnik Sep 5, 2017

Member

@AndreyAkinshin .NET Core 1.1 support remains untouched, I am just adding .NET Core 2.0 target and changing few #if defs from #if CLASSIC to #if !NETCOREAPP1_1

Member

adamsitnik commented Sep 5, 2017

@AndreyAkinshin .NET Core 1.1 support remains untouched, I am just adding .NET Core 2.0 target and changing few #if defs from #if CLASSIC to #if !NETCOREAPP1_1

@adamsitnik adamsitnik closed this in 97ab49c Sep 5, 2017

adamsitnik added a commit that referenced this issue Sep 5, 2017

@adamsitnik

This comment has been minimized.

Show comment
Hide comment
@adamsitnik

adamsitnik Sep 5, 2017

Member

@AndreyAkinshin We have a problem ;)

Build execution time has reached the maximum allowed time for your plan (60 minutes).

Since I added new target we run tests not only for .NET 4.6 and .NET Core 1.1 but also .NET Core 2.0 ;)

Which leads to timeouts.

There is no big difference between 1.1 and 2.0. Should we always run tests for both? I am not sure.

@AndreyAkinshin what do you think?

Member

adamsitnik commented Sep 5, 2017

@AndreyAkinshin We have a problem ;)

Build execution time has reached the maximum allowed time for your plan (60 minutes).

Since I added new target we run tests not only for .NET 4.6 and .NET Core 1.1 but also .NET Core 2.0 ;)

Which leads to timeouts.

There is no big difference between 1.1 and 2.0. Should we always run tests for both? I am not sure.

@AndreyAkinshin what do you think?

@adamsitnik adamsitnik reopened this Sep 5, 2017

@AndreyAkinshin

This comment has been minimized.

Show comment
Hide comment
@AndreyAkinshin

AndreyAkinshin Sep 5, 2017

Member

@adamsitnik, I think it makes sense to run general tests on .NET Core 2.0 only; and toolchain-specific tests on both (to be sure that we still support .NET Core 1.1).

Member

AndreyAkinshin commented Sep 5, 2017

@adamsitnik, I think it makes sense to run general tests on .NET Core 2.0 only; and toolchain-specific tests on both (to be sure that we still support .NET Core 1.1).

@adamsitnik

This comment has been minimized.

Show comment
Hide comment
@adamsitnik

adamsitnik Sep 5, 2017

Member

@AndreyAkinshin very reasonable! I will try to edit the tests scripts after work and make the build green again ;)

Member

adamsitnik commented Sep 5, 2017

@AndreyAkinshin very reasonable! I will try to edit the tests scripts after work and make the build green again ;)

@Mpdreamz

This comment has been minimized.

Show comment
Hide comment
@Mpdreamz

Mpdreamz Sep 5, 2017

@adamsitnik perhaps a silly question but why is BenchMarkDotNet targetting netcoreappX.X and not netstandardX.X

Mpdreamz commented Sep 5, 2017

@adamsitnik perhaps a silly question but why is BenchMarkDotNet targetting netcoreappX.X and not netstandardX.X

@adamsitnik

This comment has been minimized.

Show comment
Hide comment
@adamsitnik

adamsitnik Sep 5, 2017

Member

@Mpdreamz Actually it's a very good question!

Today

We genereate, build and compile new exe for every benchmark. We know how to get it done for .NET, Mono and .NET Core, but not for the entire .NET Standard (no Xamarin, UWP or .NET Native support). This is why we are framework specific.

We try to take advantage of that by using some platform-specific APIs like ManagementObjectSearcher to get the CPU name on Windows.

Future

Our main dependencies are: Roslyn, ClrMD and EventTrace.
Today Roslyn supports .NET Standard and EventTrace is going to be next. The day all of them become .NET Standard compatible we might be able to replace our #if defines with simple per-platform services.

The other thing is that some of our users can not take full advantage of .NET Standard. There are some IDEs that our users use that would not work with .NET Standard only. This is why we still need to target net46

My personal goal is to make BenchmarkDotNet the tool that people love to use. We take care of all the details and our users get a product that just works out of the box. I hope that we are at least close to that ;)

Member

adamsitnik commented Sep 5, 2017

@Mpdreamz Actually it's a very good question!

Today

We genereate, build and compile new exe for every benchmark. We know how to get it done for .NET, Mono and .NET Core, but not for the entire .NET Standard (no Xamarin, UWP or .NET Native support). This is why we are framework specific.

We try to take advantage of that by using some platform-specific APIs like ManagementObjectSearcher to get the CPU name on Windows.

Future

Our main dependencies are: Roslyn, ClrMD and EventTrace.
Today Roslyn supports .NET Standard and EventTrace is going to be next. The day all of them become .NET Standard compatible we might be able to replace our #if defines with simple per-platform services.

The other thing is that some of our users can not take full advantage of .NET Standard. There are some IDEs that our users use that would not work with .NET Standard only. This is why we still need to target net46

My personal goal is to make BenchmarkDotNet the tool that people love to use. We take care of all the details and our users get a product that just works out of the box. I hope that we are at least close to that ;)

@adamsitnik

This comment has been minimized.

Show comment
Hide comment
@adamsitnik

adamsitnik Sep 5, 2017

Member

@Ky7m Hello Igor!

You have done a tremendous job to port BenchmarkDotNet to cake. But now I need your help.

I have just added new TFM: netcoreapp2.0. We want to run all of our tests for .NET 4.6 and .NET Core 2.0.

The problem is that we would like to run only some of our tests for .NET Core 1.1:

We can't run all of them for all frameworks because it would take too much time (1h +)

To do this from command line I just need to run dotnet test --filter with the right arguments.

But how can I make it work with Cake?

Member

adamsitnik commented Sep 5, 2017

@Ky7m Hello Igor!

You have done a tremendous job to port BenchmarkDotNet to cake. But now I need your help.

I have just added new TFM: netcoreapp2.0. We want to run all of our tests for .NET 4.6 and .NET Core 2.0.

The problem is that we would like to run only some of our tests for .NET Core 1.1:

We can't run all of them for all frameworks because it would take too much time (1h +)

To do this from command line I just need to run dotnet test --filter with the right arguments.

But how can I make it work with Cake?

@Ky7m

This comment has been minimized.

Show comment
Hide comment
@Ky7m

Ky7m Sep 5, 2017

Collaborator

@adamsitnik Hello Adam!
It was my pleasure. There are different ways and I would like to take some time to propose an optimal solution.
To be on the same page, can you list arguments for --filter tests that should be run against .NET Core 1.1. Maybe it makes a sense to introduce a new test category.

Collaborator

Ky7m commented Sep 5, 2017

@adamsitnik Hello Adam!
It was my pleasure. There are different ways and I would like to take some time to propose an optimal solution.
To be on the same page, can you list arguments for --filter tests that should be run against .NET Core 1.1. Maybe it makes a sense to introduce a new test category.

@adamsitnik

This comment has been minimized.

Show comment
Hide comment
@adamsitnik

adamsitnik Sep 5, 2017

Member

@Ky7m great!

with dotnet cli it's: dotnet test --filter "FullyQualifiedName=Namespace.ClassName.MethodName"

So in our case it would be:
dotnet test -c Release -f netcoreapp1.1 --filter "FullyQualifiedName~BenchmarkDotNet.IntegrationTests.MultipleRuntimesTest"
dotnet test -c Release -f netcoreapp1.1 --filter "FullyQualifiedName~BenchmarkDotNet.IntegrationTests.DisassemblyDiagnoserTests"
dotnet test -c Release -f netcoreapp1.1 --filter "FullyQualifiedName~BenchmarkDotNet.IntegrationTests.MemoryDiagnoserTests"

Member

adamsitnik commented Sep 5, 2017

@Ky7m great!

with dotnet cli it's: dotnet test --filter "FullyQualifiedName=Namespace.ClassName.MethodName"

So in our case it would be:
dotnet test -c Release -f netcoreapp1.1 --filter "FullyQualifiedName~BenchmarkDotNet.IntegrationTests.MultipleRuntimesTest"
dotnet test -c Release -f netcoreapp1.1 --filter "FullyQualifiedName~BenchmarkDotNet.IntegrationTests.DisassemblyDiagnoserTests"
dotnet test -c Release -f netcoreapp1.1 --filter "FullyQualifiedName~BenchmarkDotNet.IntegrationTests.MemoryDiagnoserTests"

@AndreyAkinshin

This comment has been minimized.

Show comment
Hide comment
@AndreyAkinshin

AndreyAkinshin Sep 6, 2017

Member

@adamsitnik I think we have to introduce a new category so we will be able to maintain such methods from the source code. We should not have any information about test names in the build scripts.

Member

AndreyAkinshin commented Sep 6, 2017

@adamsitnik I think we have to introduce a new category so we will be able to maintain such methods from the source code. We should not have any information about test names in the build scripts.

@Mpdreamz

This comment has been minimized.

Show comment
Hide comment
@Mpdreamz

Mpdreamz Sep 6, 2017

@adamsitnik thanks, that makes sense! </threadhijack>

Mpdreamz commented Sep 6, 2017

@adamsitnik thanks, that makes sense! </threadhijack>

@adamsitnik

This comment has been minimized.

Show comment
Hide comment
@adamsitnik

adamsitnik Sep 6, 2017

Member

@AndreyAkinshin It depends if it's easy to filter the tests by category with dotnet cli. I am not sure about that.

The other solution could be to create new project, let's say "BackwardCompatibilityTests" which would target .NET 4.6, Core 1.1 and Core 2.0 and move these 3 classes to this project.

@AndreyAkinshin What do you think?

Member

adamsitnik commented Sep 6, 2017

@AndreyAkinshin It depends if it's easy to filter the tests by category with dotnet cli. I am not sure about that.

The other solution could be to create new project, let's say "BackwardCompatibilityTests" which would target .NET 4.6, Core 1.1 and Core 2.0 and move these 3 classes to this project.

@AndreyAkinshin What do you think?

@Ky7m

This comment has been minimized.

Show comment
Hide comment
@Ky7m

Ky7m Sep 6, 2017

Collaborator

Regarding an ability to filter tests by category, it can be achieved:

[Trait("Category", "BackwardCompatibilityTests")]
[Fact]
public void Bar()
{
}

and to run only tests from this category you can use next command:
dotnet test --filter Category=BackwardCompatibilityTests

I personally vote for this option.

Collaborator

Ky7m commented Sep 6, 2017

Regarding an ability to filter tests by category, it can be achieved:

[Trait("Category", "BackwardCompatibilityTests")]
[Fact]
public void Bar()
{
}

and to run only tests from this category you can use next command:
dotnet test --filter Category=BackwardCompatibilityTests

I personally vote for this option.

@adamsitnik

This comment has been minimized.

Show comment
Hide comment
@adamsitnik

adamsitnik Sep 6, 2017

Member

The build is green again ;)

Task                          Duration            
--------------------------------------------------
Clean                         00:00:00.0067974    
Restore                       00:01:07.3485825    
Build                         00:00:40.7787859    
FastTests                     00:00:29.6169533    
SlowTestsNet46                00:11:51.7022103    
SlowTestsNetCore2             00:30:51.0481593    
BackwardCompatibilityTests    00:13:20.9649978    
Pack                          00:00:39.6555365    
Default                       00:00:00.0007921    
--------------------------------------------------
Total:                        00:59:01.1228151  
Member

adamsitnik commented Sep 6, 2017

The build is green again ;)

Task                          Duration            
--------------------------------------------------
Clean                         00:00:00.0067974    
Restore                       00:01:07.3485825    
Build                         00:00:40.7787859    
FastTests                     00:00:29.6169533    
SlowTestsNet46                00:11:51.7022103    
SlowTestsNetCore2             00:30:51.0481593    
BackwardCompatibilityTests    00:13:20.9649978    
Pack                          00:00:39.6555365    
Default                       00:00:00.0007921    
--------------------------------------------------
Total:                        00:59:01.1228151  

@adamsitnik adamsitnik closed this Sep 6, 2017

@Ky7m

This comment has been minimized.

Show comment
Hide comment
@Ky7m

Ky7m Sep 6, 2017

Collaborator

@adamsitnik, great done!
From the timeline, I see that tests are still close to 1-hour limit.
By the way, I do not know why SlowTestsNetCore2 take much more than SlowTestsNet46 group, for example. Any thoughts?
/cc @AndreyAkinshin

Collaborator

Ky7m commented Sep 6, 2017

@adamsitnik, great done!
From the timeline, I see that tests are still close to 1-hour limit.
By the way, I do not know why SlowTestsNetCore2 take much more than SlowTestsNet46 group, for example. Any thoughts?
/cc @AndreyAkinshin

@adamsitnik

This comment has been minimized.

Show comment
Hide comment
@adamsitnik

adamsitnik Sep 6, 2017

Member

@Ky7m For .NET 4.6 we use Roslyn for compilation. For .NET Core we use dotnet cli. So we run dotnet restore and dotnet build as new processes. It takes much more time ;/

Member

adamsitnik commented Sep 6, 2017

@Ky7m For .NET 4.6 we use Roslyn for compilation. For .NET Core we use dotnet cli. So we run dotnet restore and dotnet build as new processes. It takes much more time ;/

alinasmirnova added a commit to alinasmirnova/BenchmarkDotNet that referenced this issue Sep 22, 2018

alinasmirnova added a commit to alinasmirnova/BenchmarkDotNet that referenced this issue Sep 22, 2018

alinasmirnova added a commit to alinasmirnova/BenchmarkDotNet that referenced this issue Sep 22, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment