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

OutPoint: Use CompareTo in operator== #1191

Conversation

kiminuo
Copy link
Contributor

@kiminuo kiminuo commented Oct 21, 2023

I have not measured the change but it seems that for many cases it should be faster.

@lontivero
Copy link
Contributor

lontivero commented Oct 21, 2023

I don't think it improves the perf. It should be measured.
Also, I think N follows the Benford's law. Most Ns are 0 then 1, then 2, then 3 and so on.

@kiminuo kiminuo force-pushed the feature/2023-10-21-OutPoint-faster-equality branch from 5a0e008 to 3d9aad0 Compare October 23, 2023 11:32
@kiminuo kiminuo force-pushed the feature/2023-10-21-OutPoint-faster-equality branch from 3d9aad0 to ff09630 Compare October 23, 2023 11:34
@kiminuo kiminuo changed the title OutPoint: Fist compare Ns and then hashes in operator== OutPoint: Use CompareTo in operator== Oct 23, 2023
@kiminuo
Copy link
Contributor Author

kiminuo commented Oct 23, 2023

I don't think it improves the perf.

You are seem to be right. It goes very much against my intuition but bechmarks confirm your opinion (it's actually much slower in the current benchmark).

Interestingly changing:

-return (a.hash == b.hash && a.n == b.n);
+return (a.hash == b.hash && a.n.CompareTo(b.n) == 0);

seems to show like 8% improvement on the benchmark

dotnet build -c Release --framework net6.0 && dotnet run -c Release -f net6.0 -- --runtimes net6.0 --filter *OutPointBench*
Method Mean Error StdDev
Old 539.4 ms 7.11 ms 6.30 ms
New 479.9 ms 7.51 ms 7.03 ms

I'm not sure why.

I don't want to invest too much time in this PR as the original proved to be a bad one. But if the improvement sounds good and it actually exists, then I can finish this PR.

@kiminuo kiminuo closed this Oct 23, 2023
@kiminuo kiminuo deleted the feature/2023-10-21-OutPoint-faster-equality branch October 23, 2023 18:08
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.

2 participants