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] Regressions in Microsoft.Extensions.DependencyInjection.GetServiceIEnumerable #56781

Closed
performanceautofiler bot opened this issue Aug 3, 2021 · 5 comments
Labels
arch-x64 area-Extensions-DependencyInjection os-linux Linux OS (any supported distro) tenet-performance Performance related issue tenet-performance-benchmarks Issue from performance benchmark
Milestone

Comments

@performanceautofiler
Copy link

Run Information

Architecture x64
OS ubuntu 18.04
Baseline fae141227266277c4fd0979818d175e78331229a
Compare 3bd0acf30587c88a1448a51a852871e3407aff5f
Diff Diff

Regressions in Microsoft.Extensions.DependencyInjection.GetServiceIEnumerable

Benchmark Baseline Test Test/Base Test Quality Edge Detector Baseline IR Compare IR IR Ratio Baseline ETL Compare ETL
Transient - Duration of single invocation 4.33 μs 5.28 μs 1.22 0.07 False

graph
Historical Data in Reporting System

Repro

git clone https://github.com/dotnet/performance.git
python3 .\performance\scripts\benchmarks_ci.py -f netcoreapp5.0 --filter 'Microsoft.Extensions.DependencyInjection.GetServiceIEnumerable*'

Payloads

Baseline
Compare

Histogram

Microsoft.Extensions.DependencyInjection.GetServiceIEnumerable.Transient


Docs

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

@DrewScoggins DrewScoggins changed the title [Perf] Changes at 7/30/2021 8:20:57 PM [Perf] Regressions in Microsoft.Extensions.DependencyInjection.GetServiceIEnumerable Aug 3, 2021
@DrewScoggins DrewScoggins transferred this issue from dotnet/perf-autofiling-issues Aug 3, 2021
@dotnet-issue-labeler dotnet-issue-labeler bot added area-Extensions-DependencyInjection untriaged New issue has not been triaged by the area owner labels Aug 3, 2021
@ghost
Copy link

ghost commented Aug 3, 2021

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

Issue Details

Run Information

Architecture x64
OS ubuntu 18.04
Baseline fae141227266277c4fd0979818d175e78331229a
Compare 3bd0acf30587c88a1448a51a852871e3407aff5f
Diff Diff

Regressions in Microsoft.Extensions.DependencyInjection.GetServiceIEnumerable

Benchmark Baseline Test Test/Base Test Quality Edge Detector Baseline IR Compare IR IR Ratio Baseline ETL Compare ETL
Transient - Duration of single invocation 4.33 μs 5.28 μs 1.22 0.07 False

graph
Historical Data in Reporting System

Repro

git clone https://github.com/dotnet/performance.git
python3 .\performance\scripts\benchmarks_ci.py -f netcoreapp5.0 --filter 'Microsoft.Extensions.DependencyInjection.GetServiceIEnumerable*'

Payloads

Baseline
Compare

Histogram

Microsoft.Extensions.DependencyInjection.GetServiceIEnumerable.Transient


Docs

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

Author: performanceautofiler[bot]
Assignees: -
Labels:

area-Extensions-DependencyInjection, untriaged

Milestone: -

@DrewScoggins DrewScoggins added arch-x64 os-linux Linux OS (any supported distro) tenet-performance Performance related issue tenet-performance-benchmarks Issue from performance benchmark labels Aug 3, 2021
@eerhardt
Copy link
Member

eerhardt commented Aug 3, 2021

@maryamariyan - any way this could have been caused by #56324?

@maryamariyan
Copy link
Member

Yes. but the regression jump here is very negligible compared to the one seen from May timeframe from PR #52140

image

@pakrym

@maryamariyan maryamariyan removed the untriaged New issue has not been triaged by the area owner label Aug 4, 2021
@maryamariyan maryamariyan added this to the 6.0.0 milestone Aug 4, 2021
@maryamariyan
Copy link
Member

maryamariyan commented Aug 5, 2021

Using a commit from today's main branch in dotnet/runtime (d329350),
and changing one line from

_realizedServices[callSite.ImplementationType] = accessor;

to

            _realizedServices[callSite.ServiceType] = accessor;

Improves perf from

|    Method |        Mean |      Error |     StdDev |      Median |         Min |         Max |  Gen 0 | Gen 1 | Gen 2 | Allocated |
|---------- |------------:|-----------:|-----------:|------------:|------------:|------------:|-------:|------:|------:|----------:|
| Transient | 5,365.04 ns | 261.285 ns | 300.896 ns | 5,291.84 ns | 4,860.25 ns | 5,890.23 ns | 0.2751 |     - |     - |   2,184 B |

to

|    Method |      Mean |    Error |   StdDev |    Median |      Min |       Max |  Gen 0 | Gen 1 | Gen 2 | Allocated |
|---------- |----------:|---------:|---------:|----------:|---------:|----------:|-------:|------:|------:|----------:|
| Transient | 100.21 ns | 5.190 ns | 5.768 ns | 100.53 ns | 91.99 ns | 108.95 ns | 0.0437 |     - |     - |     344 B |

Steps to get numbers:

(following docs page in benchmarking-workflow-dotnet-runtime.md)

  • built dotnet/runtime in release configuration
  • copied over M.E.DI and M.E.DI.Abtractions locally built dlls to folder C:\CodeHub\runtime\artifacts\bin\testhost\net6.0-windows-Release-x64\shared\Microsoft.NETCore.App\6.0.0
performance\src\benchmarks\micro>dotnet run -c Release -f net6.0 --artifacts "C:\results\after" --coreRun "C:\CodeHub\runtime\artifacts\bin\testhost\net6.0-windows-Release-x64\shared\Microsoft.NETCore.App\6.0.0\CoreRun.exe" --filter Microsoft.Extensions.DependencyInjection.GetService*

Also tried running baseline commit in this issue (fae1412) for the same test and that gives the report below: (not fully building runtime but using DI and DI.Abstractions code snapshot from the baseline commt)


|    Method |     Mean |    Error |   StdDev |   Median |      Min |       Max |  Gen 0 | Gen 1 | Gen 2 | Allocated |
|---------- |---------:|---------:|---------:|---------:|---------:|----------:|-------:|------:|------:|----------:|
| Transient | 95.06 ns | 4.593 ns | 5.290 ns | 95.43 ns | 88.86 ns | 107.74 ns | 0.0435 |     - |     - |     344 B |

@eerhardt
Copy link
Member

Fixed by #57155.

@ghost ghost locked as resolved and limited conversation to collaborators Sep 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-x64 area-Extensions-DependencyInjection os-linux Linux OS (any supported distro) tenet-performance Performance related issue tenet-performance-benchmarks Issue from performance benchmark
Projects
None yet
Development

No branches or pull requests

3 participants