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

Mono perf regression based on TechEmpower PlaintextPlatform benchmark #38718

Closed
fanyang-mono opened this issue Jul 2, 2020 · 12 comments
Closed
Assignees
Labels
area-Codegen-JIT-mono runtime-mono specific to the Mono runtime
Milestone

Comments

@fanyang-mono
Copy link
Member

fanyang-mono commented Jul 2, 2020

Recently, there is a ~5% regression on TechEmpower PlaintextPlatform benchmark for all 3 different flavors of mono (JIT, LLVM JIT, LLVM AOT)

Based on bisect, the regression was introduced by 778be98

For mono JIT, with the commit before this one, RPS (requests per second) was 9,173,356; with this commit, RPS was 8,709,109. For LLVM JIT and LLVM AOT, the regression percentage was similar. For LLVM AOT, the number went from 10,244,296 to 9,795,509.

To get the results from your own machine:

  1. Clone https://github.com/aspnet/Benchmarks
  2. cd Benchmarks/src/BenchmarksDriver2
  3. dotnet run -- --config ../BenchmarksApps/Kestrel/PlatformBenchmarks/platform.benchmarks.yml --scenario plaintext --profile aspnet-citrine-lin --application.selfContained true --application.runtimeVersion 5.0.0-preview.8.20324.2 -i 3 --application.options.outputFiles path_to/System.Private.CoreLib.dll --application.options.outputFiles path_to/libcoreclr.so
  • Note that System.Private.CoreLib.dll and libcoreclr.so should comes from linux build. They usually live at
    artifacts/obj/mono/Linux.x64.Release/mono/mini/.libs/libmonosgen-2.0.so (needs to be rename to libcoreclr.so)
    artifacts/bin/mono/Linux.x64.Release/System.Private.CoreLib.dll

/cc @tannergooding @imhameed

@fanyang-mono fanyang-mono added the runtime-mono specific to the Mono runtime label Jul 2, 2020
@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI untriaged New issue has not been triaged by the area owner labels Jul 2, 2020
@fanyang-mono fanyang-mono added area-Codegen-JIT-mono and removed area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI labels Jul 2, 2020
@ghost
Copy link

ghost commented Jul 2, 2020

Tagging subscribers to this area: @lewurm
Notify danmosemsft if you want to be subscribed.

@tannergooding
Copy link
Member

Mono fully supports Vector<T>, correct? If so, this sounds like a dead-code optimization issue

@SamMonoRT SamMonoRT added this to the 5.0.0 milestone Jul 2, 2020
@danmoseley
Copy link
Member

How can this commit affect Mono benchmark performance? It's a change to CoreCLR JIT, and associated tests.

@tannergooding
Copy link
Member

It was a change to the library code, to ensure that it properly throws for unsupported T. So it would also impact Mono

@danmoseley
Copy link
Member

Oh! My bad. I did not scroll enough.

@imhameed
Copy link
Contributor

imhameed commented Jul 2, 2020

I'll look into this later today.

@imhameed imhameed self-assigned this Jul 2, 2020
monojenkins pushed a commit to monojenkins/mono that referenced this issue Jul 14, 2020
Recognize `get_AllBitsSet` instead of `get_AllOnes`; this internal
intrinsic was renamed in dotnet/runtime#36579
(1c66ad1).

Implement `Vector<T>.One` as an intrinsic.

See also: dotnet/runtime#38718
@fanyang-mono
Copy link
Member Author

With Imran's recent PR #39322, RPS for mono JIT went from 8,351,923 to 8,409,044.

@SamMonoRT
Copy link
Member

We haven't had regular runs in the perf labs since Jun 25th as they are upgrading the driver system and can't be certain if there were other regressions creeping in while the lab was offline(Mono runs not being executed). We haven't quite achieved the same perf numbers as before, and are waiting on the new runs to be enabled in perf labs. if those numbers from newer runs are trending in positive direction we can close this issue. @fanyang-mono @imhameed

@SamMonoRT SamMonoRT removed the untriaged New issue has not been triaged by the area owner label Jul 27, 2020
vargaz added a commit to vargaz/runtime that referenced this issue Jul 27, 2020
monojenkins pushed a commit to monojenkins/mono that referenced this issue Jul 27, 2020
vargaz added a commit to mono/mono that referenced this issue Jul 28, 2020
Part of dotnet/runtime#38718.

Co-authored-by: vargaz <vargaz@users.noreply.github.com>
@vargaz
Copy link
Contributor

vargaz commented Jul 28, 2020

Would it be possible to rerun the tests after this:
fa72c75

@SamMonoRT
Copy link
Member

SamMonoRT commented Jul 28, 2020

@vargaz - Yes, luckily the official runs(atleast for JIT) have just restarted in the labs again. We should have newer set of results at end of the day.

@SamMonoRT
Copy link
Member

@vargaz new numbers from last night's run are published. For plaintextplatform benchmark, MonoVM- Jit config had ~3% increase. I have not pinpointed the exact change which attributed to the increase, but your change in fa72c75 could surely have had an effect.
8.828M reqs/sec June 26th
9.081M reqs/sec June 28th (which should have your change merged)

@SamMonoRT
Copy link
Member

We can close this issue.

@ghost ghost locked as resolved and limited conversation to collaborators Dec 8, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Codegen-JIT-mono runtime-mono specific to the Mono runtime
Projects
None yet
Development

No branches or pull requests

7 participants