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

add Int128 and CopySign benchmarks #3462

Merged
merged 2 commits into from
Nov 6, 2023
Merged

Conversation

adamsitnik
Copy link
Member

Benchmarks for dotnet/runtime#77579

@adamsitnik adamsitnik changed the title add CopySign benchmarks add Int128 and CopySign benchmarks Nov 2, 2023

#if NET7_0_OR_GREATER
[Benchmark]
[Arguments(1, -1)]
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this test copying both a positive and negative sign?

The actual code, due to it being two's complement, has different costs based on whether value is positive or negative and whether sign is positive or negative.

Copy link
Member Author

Choose a reason for hiding this comment

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

Shouldn't this test copying both a positive and negative sign?

I tried that and the reported time was almost identical for all test cases (like 0.4 ns for Int16)

namespace System.Tests
{
[BenchmarkCategory(Categories.Libraries)]
public class Perf_Int128
Copy link
Member

Choose a reason for hiding this comment

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

Why only this subset of tests? Do we want to provide meaningful benchmarks for most of Int128/UInt128 since it is not a language primitive, but does provide the full set of functionality?

Copy link
Member Author

Choose a reason for hiding this comment

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

Why only this subset of tests

My main goal was to add CopySign benchmarks. I've realized that Int128 has zero benchmarks, so I've copy-pasted Int64 benchmarks and adjusted them. Just to have some benchmarks as a starting point

@adamsitnik
Copy link
Member Author

@cincuranet could you please merge it (the failures are unrelated)

@cincuranet cincuranet merged commit b685a3f into dotnet:main Nov 6, 2023
28 of 32 checks passed
@cincuranet
Copy link
Contributor

Done.

@adamsitnik adamsitnik deleted the copySign branch November 6, 2023 09:25
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.

3 participants