Skip to content

Conversation

@maryamariyan
Copy link
Contributor

@maryamariyan maryamariyan commented Jun 16, 2020

cc: @adamsitnik @JunTaoLuo

PR #1288 would add Microsoft.Extensions.Primitives perf tests.

This PR adds DI, Logging and Http perf tests too.

Nate McMaster and others added 30 commits October 30, 2018 14:18
In preparation for merging many other projects into this repo, this establishes a new source code organization which groups projects together based on subject matter.


Commit migrated from dotnet/extensions@7ce647c
This includes the master branch of aspnet/HttpClientFactory


Commit migrated from dotnet/extensions@557995e
This includes the master branch of aspnet/HttpClientFactory


Commit migrated from dotnet/extensions@557995e
* Remove obsolete targets, properties, and scripts
* Replace IsProductComponent with IsShipping
* Undo bad merge to version.props
* Update documentation, and put workarounds into a common file
* Replace usages of RepositoryRoot with RepoRoot
* Remove API baselines
* Remove unnecessary restore feeds and split workarounds into two files
* Enable PR checks on all branches, and disable autocancel


Commit migrated from dotnet/extensions@f41cfde
* Remove obsolete targets, properties, and scripts
* Replace IsProductComponent with IsShipping
* Undo bad merge to version.props
* Update documentation, and put workarounds into a common file
* Replace usages of RepositoryRoot with RepoRoot
* Remove API baselines
* Remove unnecessary restore feeds and split workarounds into two files
* Enable PR checks on all branches, and disable autocancel


Commit migrated from dotnet/extensions@f41cfde
Ryan Nowak and others added 16 commits July 19, 2019 16:36
* Support netcoreapp3.1 TFM

* Unpin SDK for source build

* Update to preview1 branding


Commit migrated from dotnet/extensions@32cc816
* Support netcoreapp3.1 TFM

* Unpin SDK for source build

* Update to preview1 branding


Commit migrated from dotnet/extensions@32cc816
…-primitives-perf-move branch in performance repo
 Conflicts:
	src/benchmarks/micro/libraries/Microsoft.Extensions.Primitives/Microsoft.Extensions.Primitives.Performance.csproj
	src/benchmarks/micro/libraries/Microsoft.Extensions.Primitives/Properties/AssemblyInfo.cs
	src/benchmarks/micro/libraries/Microsoft.Extensions.Primitives/StringValuesBenchmark.cs
- Http benchmarks
- Logging benchmarks
- DI not needing InternalsVisibleTo

- [ ] Enable DI tests needing InternalsVisibleTo
- [ ] Fix for removed internal types
ServiceProviderMode = (ServiceProviderMode)Enum.Parse(typeof(ServiceProviderMode), value);
}
}
// internal ServiceProviderMode ServiceProviderMode { get; private set; }

Choose a reason for hiding this comment

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

Hey @halter73 @davidfowl do you know what the history here is? Specifically, are these benchmarks still relevant? Given I don't have much context here, I'm inclined to keep the benchmarks for each of the DI modes, but if these are no longer useful in any way, we can remove them.

Copy link
Member

Choose a reason for hiding this comment

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

Given I don't have much context here, I'm inclined to keep the benchmarks for each of the DI modes, but if these are no longer useful in any way, we can remove them.

I'm inclined to keep them too. Even if we aren't doing active perf work on DI now, it could be useful later. Is there some reason we cannot make Microsoft.Extensions.DependencyInjection's internals visable to this project?

Choose a reason for hiding this comment

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

The problem is that the benchmarks app isn't strong named so IVT doesn't work. But I think what we should do here is turn strong name on, instead of removing the benchmarks. @maryamariyan is looking into do that now.

Copy link
Contributor Author

@maryamariyan maryamariyan Jun 17, 2020

Choose a reason for hiding this comment

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

moved over the PR to #1362

@maryamariyan
Copy link
Contributor Author

closed in favor of #1362

(it's odd, push force didnt update the PR so I tried close and re-opening the PR but now I cant reopen the PR for whatever reason).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants