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

[Perf -19%] [Mono] System.Text.Tests.Perf_Encoding.GetByteCount #43476

Closed
DrewScoggins opened this issue Oct 16, 2020 · 16 comments
Closed

[Perf -19%] [Mono] System.Text.Tests.Perf_Encoding.GetByteCount #43476

DrewScoggins opened this issue Oct 16, 2020 · 16 comments
Assignees
Labels
arch-x64 area-System.Text.Encoding os-linux Linux OS (any supported distro) runtime-mono specific to the Mono runtime tenet-performance Performance related issue tenet-performance-benchmarks Issue from performance benchmark
Milestone

Comments

@DrewScoggins
Copy link
Member

Run Information

Architecture x64
OS ubuntu 18.04
Changes diff

Regressions in System.Text.Tests.Perf_Encoding

Benchmark Baseline Test Test/Base Modality Baseline Outlier Baseline ETL Comapre ETL
GetByteCount 132.16 ns 156.75 ns 1.19 False

graph
Historical Data in Reporting System

Histogram

System.Text.Tests.Perf_Encoding.GetByteCount(size: 512, encName: "utf-8")

[-127.596 ;  405.832) | @@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@
[ 405.832 ;  925.098) | 
[ 925.098 ; 1444.364) | 
[1444.364 ; 1963.630) | 
[1963.630 ; 2188.876) | 
[2188.876 ; 2708.142) | @@@@@@@@@@@@@@@@
[2708.142 ; 3145.833) | @@
[3145.833 ; 3651.283) | 
[3651.283 ; 4174.273) | @@

Docs

Profiling workflow for dotnet/runtime repository
Benchmarking workflow for dotnet/runtime repository

@Dotnet-GitSync-Bot
Copy link
Collaborator

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

@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the untriaged New issue has not been triaged by the area owner label Oct 16, 2020
@DrewScoggins DrewScoggins added arch-x64 os-linux Linux OS (any supported distro) runtime-mono specific to the Mono runtime tenet-performance Performance related issue tenet-performance-benchmarks Issue from performance benchmark labels Oct 16, 2020
@naricc
Copy link
Contributor

naricc commented Oct 16, 2020

This appears to have been caused by one of the commits in this range: f2d51b5...d6887d4

@ghost
Copy link

ghost commented Oct 16, 2020

Tagging subscribers to this area: @tarekgh, @krwq
See info in area-owners.md if you want to be subscribed.

@danmoseley
Copy link
Member

danmoseley commented Oct 16, 2020

@DrewScoggins the graph above does not show a regression (I do see it when I go to the link)

@naricc I think it's a bit narrower? f2d51b5...30e643e

Most likely commit is probably
30e643e
Jit: Remove bounds checks with tests against length. (#40180)

@dotnet/jit-contrib

@danmoseley danmoseley added area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI and removed area-System.Text.Encoding labels Oct 16, 2020
@SamMonoRT SamMonoRT removed the untriaged New issue has not been triaged by the area owner label Oct 16, 2020
@SamMonoRT SamMonoRT added this to the 6.0.0 milestone Oct 16, 2020
@danmoseley danmoseley added area-System.Text.Encoding and removed area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI labels Oct 16, 2020
@ghost
Copy link

ghost commented Oct 16, 2020

Tagging subscribers to this area: @tarekgh, @krwq
See info in area-owners.md if you want to be subscribed.

@danmoseley
Copy link
Member

Scratch that, it's Mono. @SamMonoRT looks like you're already tracking it?

Although, I don't see any interesting Mono changes in this range.

@DrewScoggins in these issues, it would be very helpful to indicate what platforms and what VM's the repro is on. For example, is this specific to x64? to ubuntu? to Mono? I have spent some time in https://aka.ms/dotnetperfindex and I was not able to figure that out.

@danmoseley
Copy link
Member

danmoseley commented Oct 16, 2020

Same question for all the issues you recently opened @DrewScoggins -- are these all specific to Mono+Ubuntu+x64 or is that just an example? You can see how that is critical info.

#43463
#43465
#43476
#43477
#43478

@tarekgh
Copy link
Member

tarekgh commented Oct 16, 2020

Looks all are Mono specific on Ubuntu

System.Text.Tests.Perf_Encoding.GetByteCount(size: 512, encName: "utf-8")

  Name Architecture OS Queue Git Hash Branch Repo Locale Configurations
Baseline 20201014.6 x64 ubuntu 18.04 Ubuntu.1804.Amd64.Tiger.Perf 51f6b8b refs/heads/master dotnet/runtime en-US CompilationMode=tiered,LLVM=false,MonoAOT=false,MonoInterpreter=false,RunKind=micro_mono
Compare 20201014.6 x64 ubuntu 18.04 Ubuntu.1804.Amd64.Tiger.Perf 51f6b8b refs/heads/master dotnet/runtime en-US CompilationMode=tiered,LLVM=false,MonoAOT=false,MonoInterpreter=false,RunKind=micro_mono

System.Collections.CtorGivenSizeNonGeneric.SortedList(Size: 512)

  Name Architecture OS Queue Git Hash Branch Repo Locale Configurations
Baseline 2020101.8 x64 ubuntu 18.04 Ubuntu.1804.Amd64.Tiger.Perf 722b8a6 refs/heads/master dotnet/runtime en-US CompilationMode=tiered,LLVM=false,MonoAOT=false,MonoInterpreter=true,RunKind=micro_mono
Compare 2020101.8 x64 ubuntu 18.04 Ubuntu.1804.Amd64.Tiger.Perf 722b8a6 refs/heads/master dotnet/runtime en-US CompilationMode=tiered,LLVM=false,MonoAOT=false,MonoInterpreter=true,RunKind=micro_mono

System.Memory.ReadOnlyMemory.ToArray(Size: 512)

  Name Architecture OS Queue Git Hash Branch Repo Locale Configurations
Baseline 2020101.8 x64 ubuntu 18.04 Ubuntu.1804.Amd64.Tiger.Perf 722b8a6 refs/heads/master dotnet/runtime en-US CompilationMode=tiered,LLVM=false,MonoAOT=false,MonoInterpreter=false,RunKind=micro_mono
Compare 2020101.8 x64 ubuntu 18.04 Ubuntu.1804.Amd64.Tiger.Perf 722b8a6 refs/heads/master dotnet/runtime en-US CompilationMode=tiered,LLVM=false,MonoAOT=false,MonoInterpreter=false,RunKind=micro_mono

System.Net.Primitives.Tests.IPAddressPerformanceTests.Ctor_Span(address: [16, 32, 48, 64, 80, ...])

  Name Architecture OS Queue Git Hash Branch Repo Locale Configurations
Baseline 2020101.8 x64 ubuntu 18.04 Ubuntu.1804.Amd64.Tiger.Perf 722b8a6 refs/heads/master dotnet/runtime en-US CompilationMode=tiered,LLVM=false,MonoAOT=false,MonoInterpreter=false,RunKind=micro_mono
Compare 2020101.8 x64 ubuntu 18.04 Ubuntu.1804.Amd64.Tiger.Perf 722b8a6 refs/heads/master dotnet/runtime en-US CompilationMode=tiered,LLVM=false,MonoAOT=false,MonoInterpreter=false,RunKind=micro_mono

System.Tests.Perf_Int16.ToString(value: 32767)

  Name Architecture OS Queue Git Hash Branch Repo Locale Configurations
Baseline 2020101.8 x64 ubuntu 18.04 Ubuntu.1804.Amd64.Tiger.Perf 722b8a6 refs/heads/master dotnet/runtime en-US CompilationMode=tiered,LLVM=false,MonoAOT=false,MonoInterpreter=false,RunKind=micro_mono
Compare 2020101.8 x64 ubuntu 18.04 Ubuntu.1804.Amd64.Tiger.Perf 722b8a6 refs/heads/master dotnet/runtime en-US CompilationMode=tiered,LLVM=false,MonoAOT=false,MonoInterpreter=false,RunKind=micro_mono

@SamMonoRT
Copy link
Member

SamMonoRT commented Oct 16, 2020

Need to dig in a little more, could be any BCL or Mono runtime change causing the regressions. Also, to note we just enabled the auto-filling for Mono runs, still getting familiar with commit disection

@SamMonoRT
Copy link
Member

@DrewScoggins - to avoid confusion, is it possible to add [Mono] to the issue title as part of the auto-filing logic ?

@DrewScoggins
Copy link
Member Author

In every bug that I file I always mark with labels all of the affected platforms, that the lab is able to detect. I thought that marking these issues with the runtime-mono label would make it clear that these were on the mono runtime, I can see that this is not the case. I will make sure that all issues that get moved into the runtime repo going forward that are from Mono have [Mono] in the title of the bug.

@danmoseley
Copy link
Member

In every bug that I file I always mark with labels all of the affected platforms, that the lab is able to detect. I thought that marking these issues with the runtime-mono label would make it clear that these were on the mono runtime

@DrewScoggins thanks - I didn't know this.

@danmoseley
Copy link
Member

@SamMonoRT any thoughts about where investigation should be? I'm at a bit of a loss as to which of these changes could have caused it.

@naricc
Copy link
Contributor

naricc commented Oct 19, 2020

@danmosemsft

None of the changes look particularly likely to have caused this regression. The ones that only change coreclr code can be ruled out; the ones that change libraries could could theoretically causes a performance regression in Mono. But looking at them and looking at the benchmarks, it seems really unlikely.

Since this is not affecting many benchmarks, I think its likely just a sort of random alignment issue or something like that. We don't have much historical data for these benchmarks on Mono, so it is hard to know what variation is normal. So my plan for right now is to just keep this open and monitor this benchmark over the next month or so.

@danmoseley
Copy link
Member

@naricc sounds good - thanks!

@SamMonoRT SamMonoRT changed the title [Perf -19%] System.Text.Tests.Perf_Encoding.GetByteCount [Perf -19%] [Mono] System.Text.Tests.Perf_Encoding.GetByteCount Oct 23, 2020
@naricc
Copy link
Contributor

naricc commented Feb 19, 2021

Having followed the benchmarks, I don't think there is any real regression here.

@naricc naricc closed this as completed Feb 19, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Mar 21, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-x64 area-System.Text.Encoding os-linux Linux OS (any supported distro) runtime-mono specific to the Mono runtime tenet-performance Performance related issue tenet-performance-benchmarks Issue from performance benchmark
Projects
None yet
Development

No branches or pull requests

7 participants