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

Update performance tests to build against netcoreapp #9635

Closed
jkotas opened this issue Jan 31, 2018 · 12 comments
Closed

Update performance tests to build against netcoreapp #9635

jkotas opened this issue Jan 31, 2018 · 12 comments
Assignees
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI bug tenet-performance-benchmarks Issue from performance benchmark
Milestone

Comments

@jkotas
Copy link
Member

jkotas commented Jan 31, 2018

Performance tests are building against .NET Standard 1.4: https://github.com/dotnet/coreclr/blob/master/tests/src/JIT/config/benchmark/benchmark.csproj#L5

This is a problem for testing netcoreapp-specific APIs. We have hit with MemoryMarshal.CreateSpan API that was changed to netcoreapp-specific. The test for this API was disable via #if false and link to this issue: dotnet/coreclr#16091

category:correctness
theme:testing
skill-level:expert
cost:small

@RussKeldorph
Copy link
Contributor

Would this supercede #8991? There was some debate about what is appropriate there. Do we all agree this issue is the path forward? @DrewScoggins @jorive @AndyAyersMS

@AndyAyersMS
Copy link
Member

In the past we've wanted to keep perf tests building against a lowest common denominator so we could run the exact same tests on older releases to assess release to release perf changes.

I don't believe we need to support running current tests against 1.1 anymore, but it would be nice to be able to run them vs 2.0. So maybe we need to multi-target this test or just let the 2.0 version diverge.

Moving up to at least netstandard2.0 would let us clean up some clunky code that searches for related files (#9281).

For whatever reason updating the test dependencies have proven difficult.

@jorive
Copy link
Member

jorive commented Jan 31, 2018

@jkotas Why not create a tests/src/JIT/config/benchmark+span/benchmark.csproj where it targets a different netstandard? I do not see a reason to move all benchmarks for this reason.
Or even better, we could create different configs/projects like this: tests/src/JIT/config/benchmark+netstandard[1.1|1.2|1.3|1.4|1.5|1.6|2.0] as needed :)

@jkotas
Copy link
Member Author

jkotas commented Jan 31, 2018

Sounds fine to me.

@jkotas
Copy link
Member Author

jkotas commented Jan 31, 2018

I do think you really need the tests/src/JIT/config/benchmark+netstandard* .csproj files. They have a lot of cruft in them, e.g. the 4.4.0-beta-24913-0 package versions do not look right. I think you should be just able to reference netstandard1.1/.../2.0 TFM in the .csproj file.

@jorive
Copy link
Member

jorive commented Jan 31, 2018

I do not know what was the initial reasoning behind these config/csproj, but I do agree with each test being able to specify its own target.

@AndyAyersMS
Copy link
Member

I'll start working on this.

@AndyAyersMS AndyAyersMS self-assigned this Feb 12, 2018
@AndyAyersMS
Copy link
Member

Because we produce a unified environment for all the CoreCLR tests, all the projects involved have to agree on which version of a package we'll use, both for building and running. We have complex web of dependencies here which makes updating all this challenging (at least for us jit developers). I know previous attempts have all hit roadblocks.

I'm not sure if it is worth the trouble to keep individual projects referencing the minimum level of standard they require, since this is something we'd need to discover test by test. It seems a lot simpler to baseline all test projects at the same version, say netstandard 2.0, and then make exceptions for the tests that need 2.1. I realize this means we can't back port the perf test changes to the 1.1 branch, but I think that's ok.

AndyAyersMS referenced this issue in AndyAyersMS/coreclr Feb 14, 2018
Remove the special benchmark configs and update benchmark projects accordingly.
Also update other random projects that were referencing benchmark configs.

Benchmarks now build against netstandard 2.0.

Addresses #14124, #16126.
AndyAyersMS referenced this issue in dotnet/coreclr Feb 26, 2018
Remove the special benchmark configs and update benchmark projects accordingly.
Also update other random projects that were referencing benchmark configs.

Benchmarks now build against the default standard.

Addresses #14124, #16126.
@AndyAyersMS
Copy link
Member

AndyAyersMS commented Feb 27, 2018

The dependencies were updated via dotnet/coreclr#16378, so the disabled tests can now be re-enabled.

@ahsonkhan is doing this as part of dotnet/coreclr#16521. 😕

@AndyAyersMS AndyAyersMS assigned ahsonkhan and unassigned ahsonkhan Feb 27, 2018
@AndyAyersMS
Copy link
Member

Had to revert the changes in dotnet/coreclr#16378 via dotnet/coreclr#16647.

@AndyAyersMS
Copy link
Member

Will get to this after 2.1 is out the door.

@msftgits msftgits transferred this issue from dotnet/coreclr Jan 31, 2020
@msftgits msftgits added this to the Future milestone Jan 31, 2020
@AndyAyersMS
Copy link
Member

No longer relevant.

@ghost ghost locked as resolved and limited conversation to collaborators Dec 18, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI bug tenet-performance-benchmarks Issue from performance benchmark
Projects
None yet
Development

No branches or pull requests

6 participants