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

Fix #33948 by decreasing number of tests per group #40491

Merged
merged 2 commits into from
Aug 10, 2020

Conversation

echesakov
Copy link
Contributor

@echesakov echesakov commented Aug 6, 2020

Decrease MaxGroupSize to 100

Fixes: #33948

@Dotnet-GitSync-Bot
Copy link
Collaborator

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

@echesakov
Copy link
Contributor Author

echesakov commented Aug 6, 2020

Let's see if 100 is small enough to allow runtime-coreclr gcstress-extra to pass without timing out

@echesakov
Copy link
Contributor Author

No timeouts in gcstress-extra jobs.

@dotnet/jit-contrib @tannergooding PTAL

@tannergooding
Copy link
Member

Is it potentially interesting that the group count is timing out on ARM while x86 has significantly more and doesn't?

@echesakov
Copy link
Contributor Author

Is it potentially interesting that the group count is timing out on ARM while x86 has significantly more and doesn't?

You mean, that test groups under HardwareIntrinsics.X86 have the greater number of tests per group and still not timing out?

@tannergooding
Copy link
Member

You mean, that test groups under HardwareIntrinsics.X86 have the greater number of tests per group and still not timing out?

Yes, they currently aren't split at all (outside where the default file names conflicted for 128-bit vs 256-bit variants)

AVX has 150, AVX2 has 250, SSE2 has 257

@echesakov
Copy link
Contributor Author

@tannergooding There could be a dozen of potential reasons why this happens... For example, different performance characteristics of hardware we are using to run the tests, or the average load of these machines. It definitely wouldn't hurt to know - but I am not sure if such analysis should be done at the moment.

Ideally, I would like to have the partitioning of the tests done as in #40177 But as I described in that PR we would have to pay some price for such approach. I hope to re-visit this again in 6.0

@echesakov
Copy link
Contributor Author

Can anyone sign off on the changes?

@echesakov echesakov merged commit 4988c25 into dotnet:master Aug 10, 2020
@echesakov echesakov deleted the Runtime_33948 branch August 10, 2020 18:10
@karelz karelz added this to the 5.0.0 milestone Aug 18, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 7, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Test failure: JIT/HardwareIntrinsics/Arm/AdvSimd/AdvSimd_r/AdvSimd_r.sh
5 participants