Skip to content

Optimize Half.Equals - #41554

Merged
jkotas merged 4 commits into
dotnet:masterfrom
huoyaoyuan:half-equals-performance
Aug 31, 2020
Merged

Optimize Half.Equals#41554
jkotas merged 4 commits into
dotnet:masterfrom
huoyaoyuan:half-equals-performance

Conversation

@huoyaoyuan

Copy link
Copy Markdown
Member

Fixes #41329

Benchmark:
(See full benchmark at #41329 (comment))

Method Job left right Mean Error StdDev Ratio RatioSD Code Size
Equal PR 0 -0 0.4526 ns 0.0127 ns 0.0112 ns 1.00 0.00 75 B
Equal master 0 -0 1.8901 ns 0.0135 ns 0.0126 ns 4.18 0.12 156 B
Equal PR 65500 -65500 0.6604 ns 0.0088 ns 0.0082 ns 1.00 0.00 75 B
Equal master 65500 -65500 1.6911 ns 0.0131 ns 0.0116 ns 2.56 0.03 156 B
Equal PR 65500 0 0.6683 ns 0.0042 ns 0.0038 ns 1.00 0.00 75 B
Equal master 65500 0 1.6832 ns 0.0076 ns 0.0068 ns 2.52 0.02 156 B
Equal PR 65500 65500 0.2071 ns 0.0095 ns 0.0089 ns 1.00 0.00 75 B
Equal master 65500 65500 2.2191 ns 0.0173 ns 0.0162 ns 10.73 0.43 156 B
Equal PR 65500 NaN 0.6647 ns 0.0081 ns 0.0076 ns 1.00 0.00 75 B
Equal master 65500 NaN 1.4638 ns 0.0015 ns 0.0014 ns 2.20 0.02 156 B
Equal PR NaN 0 0.4603 ns 0.0009 ns 0.0008 ns 1.00 0.00 75 B
Equal master NaN 0 2.4231 ns 0.0039 ns 0.0036 ns 5.26 0.01 156 B
Equal PR NaN NaN 0.2098 ns 0.0017 ns 0.0016 ns 1.00 0.00 75 B
Equal PR NaN NaN 0.4320 ns 0.0041 ns 0.0038 ns 2.06 0.02 75 B
Equal master NaN NaN 2.4281 ns 0.0078 ns 0.0069 ns 11.58 0.09 156 B
Equal master NaN NaN 2.0738 ns 0.0086 ns 0.0076 ns 9.89 0.09 156 B

Further question

Should we apply more AggressiveInlineing on Half members?
If we've get hardware acceleration, the implementation should change massively, and can be considered later.
Should call size bloat be considered?

@Dotnet-GitSync-Bot

Copy link
Copy Markdown
Collaborator

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

/// <summary>
/// Returns a value that indicates whether this instance is equal to a specified <paramref name="other"/> value.
/// </summary>
[MethodImpl(MethodImplOptions.AggressiveInlining)]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

AggressiveInlining does improve microbenchmark results, but it introduces code bloat that tends to hurt real workloads. We are careful about using it.

Given that nothing else is annotated with AggressiveInlining in Half, I would not introduce one-off here. I think we would need some real world examples to justify it, and likely apply it to number of other methods in this file.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Sure. Removed for this PR. Let it be a part of optimizing the whole Half type.

@jkotas jkotas left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks

@jkotas
jkotas merged commit 7b77174 into dotnet:master Aug 31, 2020
@huoyaoyuan
huoyaoyuan deleted the half-equals-performance branch August 31, 2020 08:03
@ghost ghost locked as resolved and limited conversation to collaborators Dec 7, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Optimize Half.Equals

4 participants