-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Improve performance of Enum.{Try}Parse #21214
Conversation
ahsonkhan
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some nits/questions. Otherwise, LGTM.
78979a1 to
81923fa
Compare
|
Thanks for the good feedback, @jkotas and @ahsonkhan. |
|
@dotnet-bot test CentOS7.1 x64 Debug Innerloop Build please |
|
@dotnet-bot test Linux-musl x64 Debug Build please |
|
Could you please fix the conflict? |
In particular for the generic overloads, where this avoids all per-operation allocations when commonly used underlying types are used (for example, this improves when the underlying enum type is int but not double).
81923fa to
37df331
Compare
Rebased / fixed conflicts. |
|
@dotnet-bot test OSX10.12 x64 Checked Innerloop Build and Test please |
* Improve performance of Enum.{Try}Parse
In particular for the generic overloads, where this avoids all per-operation allocations when commonly used underlying types are used (for example, this improves when the underlying enum type is int but not double).
* Address PR feedback
Signed-off-by: dotnet-bot <[email protected]>
* Improve performance of Enum.{Try}Parse
In particular for the generic overloads, where this avoids all per-operation allocations when commonly used underlying types are used (for example, this improves when the underlying enum type is int but not double).
* Address PR feedback
Signed-off-by: dotnet-bot <[email protected]>
* Improve performance of Enum.{Try}Parse
In particular for the generic overloads, where this avoids all per-operation allocations when commonly used underlying types are used (for example, this improves when the underlying enum type is int but not double).
* Address PR feedback
Signed-off-by: dotnet-bot <[email protected]>
|
@stephentoub How did you get those performance results? I'd like to do the same with my changes at master...TylerBrinkley:enum-perf. |
|
Benchmark.NET, then copying/pasting the results into Excel to compare. (With the --coreRun argument that was recently added to Benchmark.NET, this can be achieved more easily with the right set up locally.) |
|
@stephentoub alright, I got things setup locally to benchmark my changes with the --coreRun argument. Do you happen to have the tests you used for this? I'd like to benchmark them with my changes. |
|
@TylerBrinkley, looks like I don't have these as I'd written them for this PR (I thought I'd copy/pasted into the PR description as I often try to remember to do, but apparently I didn't this time), but we do have https://github.com/dotnet/performance/blob/master/src/benchmarks/micro/corefx/System.Runtime/Perf.Enum.cs. |
|
Thanks, I saw that. I'll try to expand on those. |
* Improve performance of Enum.{Try}Parse
In particular for the generic overloads, where this avoids all per-operation allocations when commonly used underlying types are used (for example, this improves when the underlying enum type is int but not double).
* Address PR feedback
Commit migrated from dotnet/coreclr@88fb86f
Fixes https://github.com/dotnet/corefx/issues/594
Fixes https://github.com/dotnet/corefx/issues/11073
Contributes to https://github.com/dotnet/corefx/issues/15453
cc: @ahsonkhan, @jkotas, @GrabYourPitchforks, @joperezr