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

Improve performance of IsRootScope check #54555

Merged
1 commit merged into from
Jun 22, 2021
Merged

Conversation

davidfowl
Copy link
Member

  • Stash a field instead of doing an equality comparison.

Fixes #54351

PS: I'm not sure how to run those performance tests.

- Stash a field instead of doing an equality comparison.

Fixes #54351
@ghost
Copy link

ghost commented Jun 22, 2021

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

Issue Details
  • Stash a field instead of doing an equality comparison.

Fixes #54351

PS: I'm not sure how to run those performance tests.

Author: davidfowl
Assignees: -
Labels:

area-Extensions-DependencyInjection

Milestone: -

Copy link
Contributor

@pakrym pakrym left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixes #54351

Optimistic :)

@eerhardt
Copy link
Member

PS: I'm not sure how to run those performance tests.

There is a Repro section in the issue #54351

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

@davidfowl
Copy link
Member Author

python3 wtf. Is there an easy way to run on the same hardware? Also do I need to manually patch bits and run before and after?

@eerhardt
Copy link
Member

python3 wtf.

+💯

Is there an easy way to run on the same hardware? Also do I need to manually patch bits and run before and after?

cc @DrewScoggins @billwert

@billwert
Copy link
Member

What's wrong with python? :)

If you have two runtimes you want to run back to back, I recommend running benchmarks_ci.py once to bootstrap everything and then you can follow the directions here:
https://github.com/dotnet/performance/blob/main/docs/benchmarking-workflow-dotnet-runtime.md#preventing-regressions

Snapshot your baseline, then you can iterate as necessary with the after build.

@davidfowl
Copy link
Member Author

OR I could just merge it.

@billwert
Copy link
Member

Sure. We'll see the impact in the system soon enough if you do.

@ghost
Copy link

ghost commented Jun 22, 2021

Hello @davidfowl!

Because this pull request has the auto-merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

@ghost ghost merged commit 9da9bfc into main Jun 22, 2021
@ghost ghost deleted the davidfowl/faster-scope-check branch June 22, 2021 21:07
@ghost ghost locked as resolved and limited conversation to collaborators Jul 22, 2021
This pull request was closed.
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.

Regressions in Microsoft.Extensions.DependencyInjection.GetService
5 participants