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.

Sign up for GitHub

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

Jump to bottom

Redo minor cleanups in System.Runtime.Numerics #71274

Merged
merged 5 commits into from
Jul 10, 2022
Merged

Redo minor cleanups in System.Runtime.Numerics #71274

merged 5 commits into from
Jul 10, 2022

Conversation

huoyaoyuan
Copy link
Member

Redo a part of #53984
I don't expect it will cause regression this time. Should I run the benchmarks?

@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label Jun 24, 2022
@ghost
Copy link

ghost commented Jun 24, 2022

Tagging subscribers to this area: @dotnet/area-system-numerics
See info in area-owners.md if you want to be subscribed.

Issue Details

Redo a part of #53984
I don't expect it will cause regression this time. Should I run the benchmarks?

Author: huoyaoyuan
Assignees: -
Labels:

area-System.Numerics
Milestone: -

@stephentoub
Copy link
Member

Should I run the benchmarks?

Yes, please.

@huoyaoyuan
Copy link
Member Author 

BenchmarkDotNet=v0.13.1.1786-nightly, OS=Windows 11 (10.0.22621.160)
Intel Core i7-8700K CPU 3.70GHz (Coffee Lake), 1 CPU, 12 logical and 6 physical cores
.NET SDK=7.0.100-preview.5.22307.18
  [Host]     : .NET 7.0.0 (7.0.22.30112), X64 RyuJIT
  Job-ATGARL : .NET 7.0.0 (42.42.42.42424), X64 RyuJIT
  Job-BYBGPF : .NET 7.0.0 (42.42.42.42424), X64 RyuJIT

PowerPlanMode=00000000-0000-0000-0000-000000000000  IterationTime=250.0000 ms  MaxIterationCount=20  
MinIterationCount=15  WarmupCount=1
Method Job Toolchain numberString arguments Mean Error StdDev Median Min Max Ratio RatioSD Gen 0 Gen 1 Allocated Alloc Ratio
Ctor_ByteArray Job-ATGARL \Numerics-PR\corerun.exe -2147483648 ? 8.739 ns 0.2149 ns 0.2475 ns 8.633 ns 8.509 ns 9.357 ns 1.03 0.03 - - - NA
Ctor_ByteArray Job-BYBGPF \Numerics-main\corerun.exe -2147483648 ? 8.553 ns 0.0296 ns 0.0277 ns 8.567 ns 8.510 ns 8.586 ns 1.00 0.00 - - - NA
ToByteArray Job-ATGARL \Numerics-PR\corerun.exe -2147483648 ? 12.206 ns 0.0722 ns 0.0675 ns 12.192 ns 12.087 ns 12.350 ns 1.00 0.01 0.0051 - 32 B 1.00
ToByteArray Job-BYBGPF \Numerics-main\corerun.exe -2147483648 ? 12.193 ns 0.0726 ns 0.0643 ns 12.182 ns 12.093 ns 12.293 ns 1.00 0.00 0.0051 - 32 B 1.00
Parse Job-ATGARL \Numerics-PR\corerun.exe -2147483648 ? 122.166 ns 0.5879 ns 0.5499 ns 122.293 ns 121.152 ns 122.962 ns 1.00 0.01 0.0215 - 136 B 1.00
Parse Job-BYBGPF \Numerics-main\corerun.exe -2147483648 ? 122.340 ns 0.6627 ns 0.6199 ns 122.145 ns 121.675 ns 123.360 ns 1.00 0.00 0.0213 - 136 B 1.00
ToStringX Job-ATGARL \Numerics-PR\corerun.exe -2147483648 ? 50.111 ns 0.2322 ns 0.2059 ns 50.100 ns 49.735 ns 50.473 ns 0.96 0.00 0.0063 - 40 B 1.00
ToStringX Job-BYBGPF \Numerics-main\corerun.exe -2147483648 ? 52.200 ns 0.1349 ns 0.1196 ns 52.179 ns 51.937 ns 52.430 ns 1.00 0.00 0.0062 - 40 B 1.00
ToStringD Job-ATGARL \Numerics-PR\corerun.exe -2147483648 ? 51.335 ns 0.3742 ns 0.3500 ns 51.234 ns 50.915 ns 52.204 ns 0.98 0.03 0.0240 - 152 B 1.00
ToStringD Job-BYBGPF \Numerics-main\corerun.exe -2147483648 ? 52.586 ns 1.4284 ns 1.5284 ns 51.814 ns 51.146 ns 56.363 ns 1.00 0.00 0.0240 - 152 B 1.00
Add Job-ATGARL \Numerics-PR\corerun.exe ? 1024,1024 bits 45.880 ns 0.2484 ns 0.2323 ns 45.829 ns 45.425 ns 46.262 ns 0.99 0.01 0.0254 - 160 B 1.00
Add Job-BYBGPF \Numerics-main\corerun.exe ? 1024,1024 bits 46.143 ns 0.6876 ns 0.6432 ns 46.039 ns 45.208 ns 47.684 ns 1.00 0.00 0.0255 - 160 B 1.00
Subtract Job-ATGARL \Numerics-PR\corerun.exe ? 1024,1024 bits 48.765 ns 0.8237 ns 0.6431 ns 48.941 ns 47.891 ns 49.515 ns 0.99 0.01 0.0241 - 152 B 1.00
Subtract Job-BYBGPF \Numerics-main\corerun.exe ? 1024,1024 bits 49.356 ns 0.5143 ns 0.4559 ns 49.300 ns 48.875 ns 50.471 ns 1.00 0.00 0.0242 - 152 B 1.00
Multiply Job-ATGARL \Numerics-PR\corerun.exe ? 1024,1024 bits 800.211 ns 12.0888 ns 10.7164 ns 798.558 ns 781.974 ns 819.785 ns 1.02 0.02 0.0437 - 280 B 1.00
Multiply Job-BYBGPF \Numerics-main\corerun.exe ? 1024,1024 bits 788.252 ns 6.7923 ns 6.0212 ns 788.402 ns 777.523 ns 795.796 ns 1.00 0.00 0.0445 - 280 B 1.00
GreatestCommonDivisor Job-ATGARL \Numerics-PR\corerun.exe ? 1024,1024 bits 7,489.627 ns 32.7477 ns 30.6322 ns 7,474.985 ns 7,453.571 ns 7,566.627 ns 1.00 0.01 - - - NA
GreatestCommonDivisor Job-BYBGPF \Numerics-main\corerun.exe ? 1024,1024 bits 7,487.696 ns 32.6887 ns 27.2965 ns 7,484.724 ns 7,455.427 ns 7,546.073 ns 1.00 0.00 - - - NA
ModPow Job-ATGARL \Numerics-PR\corerun.exe ? 1024,1024,64 bits 120,271.183 ns 730.9765 ns 683.7558 ns 120,127.212 ns 119,395.240 ns 121,742.067 ns 0.99 0.01 - - 32 B 1.00
ModPow Job-BYBGPF \Numerics-main\corerun.exe ? 1024,1024,64 bits 121,061.372 ns 1,676.0098 ns 1,485.7390 ns 120,429.079 ns 119,516.698 ns 123,332.156 ns 1.00 0.00 - - 32 B 1.00
Divide Job-ATGARL \Numerics-PR\corerun.exe ? 1024,512 bits 471.102 ns 2.5855 ns 2.0186 ns 471.307 ns 466.918 ns 473.776 ns 1.00 0.01 0.0150 - 96 B 1.00
Divide Job-BYBGPF \Numerics-main\corerun.exe ? 1024,512 bits 473.339 ns 4.5414 ns 4.2481 ns 472.096 ns 468.186 ns 482.127 ns 1.00 0.00 0.0151 - 96 B 1.00
Remainder Job-ATGARL \Numerics-PR\corerun.exe ? 1024,512 bits 474.412 ns 8.0331 ns 8.5953 ns 470.739 ns 464.756 ns 494.402 ns 1.01 0.02 0.0135 - 88 B 1.00
Remainder Job-BYBGPF \Numerics-main\corerun.exe ? 1024,512 bits 469.358 ns 6.2347 ns 5.8319 ns 467.792 ns 461.599 ns 480.975 ns 1.00 0.00 0.0133 - 88 B 1.00
Ctor_ByteArray Job-ATGARL \Numerics-PR\corerun.exe 123 ? 6.200 ns 0.0217 ns 0.0182 ns 6.204 ns 6.169 ns 6.224 ns 0.99 0.00 - - - NA
Ctor_ByteArray Job-BYBGPF \Numerics-main\corerun.exe 123 ? 6.249 ns 0.0271 ns 0.0254 ns 6.247 ns 6.214 ns 6.294 ns 1.00 0.00 - - - NA
ToByteArray Job-ATGARL \Numerics-PR\corerun.exe 123 ? 11.239 ns 0.2195 ns 0.2054 ns 11.236 ns 11.001 ns 11.755 ns 1.01 0.02 0.0051 - 32 B 1.00
ToByteArray Job-BYBGPF \Numerics-main\corerun.exe 123 ? 11.108 ns 0.0885 ns 0.0785 ns 11.099 ns 10.955 ns 11.244 ns 1.00 0.00 0.0050 - 32 B 1.00
Parse Job-ATGARL \Numerics-PR\corerun.exe 123 ? 80.893 ns 0.8310 ns 0.7774 ns 80.828 ns 80.153 ns 82.426 ns 1.00 0.01 0.0165 - 104 B 1.00
Parse Job-BYBGPF \Numerics-main\corerun.exe 123 ? 80.655 ns 1.0405 ns 0.9733 ns 80.702 ns 79.281 ns 82.558 ns 1.00 0.00 0.0165 - 104 B 1.00
ToStringX Job-ATGARL \Numerics-PR\corerun.exe 123 ? 41.577 ns 0.3012 ns 0.2515 ns 41.588 ns 41.204 ns 42.139 ns 0.99 0.01 0.0050 - 32 B 1.00
ToStringX Job-BYBGPF \Numerics-main\corerun.exe 123 ? 41.979 ns 0.2573 ns 0.2281 ns 41.892 ns 41.766 ns 42.583 ns 1.00 0.00 0.0050 - 32 B 1.00
ToStringD Job-ATGARL \Numerics-PR\corerun.exe 123 ? 29.469 ns 0.6894 ns 0.7939 ns 29.146 ns 28.545 ns 30.958 ns 0.93 0.02 0.0051 - 32 B 1.00
ToStringD Job-BYBGPF \Numerics-main\corerun.exe 123 ? 31.646 ns 0.2643 ns 0.2472 ns 31.570 ns 31.386 ns 32.134 ns 1.00 0.00 0.0050 - 32 B 1.00
Ctor_ByteArray Job-ATGARL \Numerics-PR\corerun.exe 123456789012(...)901234567890 [200] ? 116.514 ns 1.2832 ns 1.0715 ns 116.228 ns 115.501 ns 119.302 ns 1.00 0.01 0.0178 - 112 B 1.00
Ctor_ByteArray Job-BYBGPF \Numerics-main\corerun.exe 123456789012(...)901234567890 [200] ? 116.690 ns 0.6362 ns 0.4967 ns 116.564 ns 116.137 ns 117.656 ns 1.00 0.00 0.0177 - 112 B 1.00
ToByteArray Job-ATGARL \Numerics-PR\corerun.exe 123456789012(...)901234567890 [200] ? 55.501 ns 0.9090 ns 0.8503 ns 55.506 ns 54.007 ns 56.784 ns 1.01 0.02 0.0177 - 112 B 1.00
ToByteArray Job-BYBGPF \Numerics-main\corerun.exe 123456789012(...)901234567890 [200] ? 54.835 ns 0.3332 ns 0.2782 ns 54.882 ns 54.291 ns 55.178 ns 1.00 0.00 0.0176 - 112 B 1.00
Parse Job-ATGARL \Numerics-PR\corerun.exe 123456789012(...)901234567890 [200] ? 1,382.088 ns 14.5772 ns 13.6355 ns 1,380.709 ns 1,364.130 ns 1,403.301 ns 1.00 0.01 0.1528 - 984 B 1.00
Parse Job-BYBGPF \Numerics-main\corerun.exe 123456789012(...)901234567890 [200] ? 1,375.142 ns 12.7599 ns 10.6551 ns 1,375.243 ns 1,360.171 ns 1,400.276 ns 1.00 0.00 0.1569 - 984 B 1.00
ToStringX Job-ATGARL \Numerics-PR\corerun.exe 123456789012(...)901234567890 [200] ? 353.388 ns 8.5455 ns 9.4983 ns 354.690 ns 338.352 ns 373.590 ns 1.04 0.03 0.0571 - 360 B 1.00
ToStringX Job-BYBGPF \Numerics-main\corerun.exe 123456789012(...)901234567890 [200] ? 340.241 ns 2.8391 ns 2.3708 ns 339.672 ns 337.284 ns 346.239 ns 1.00 0.00 0.0570 - 360 B 1.00
ToStringD Job-ATGARL \Numerics-PR\corerun.exe 123456789012(...)901234567890 [200] ? 893.034 ns 10.7371 ns 10.0434 ns 890.743 ns 878.660 ns 908.679 ns 1.00 0.01 0.1577 - 992 B 1.00
ToStringD Job-BYBGPF \Numerics-main\corerun.exe 123456789012(...)901234567890 [200] ? 889.859 ns 5.8455 ns 5.4679 ns 889.462 ns 882.115 ns 897.646 ns 1.00 0.00 0.1562 - 992 B 1.00
Add Job-ATGARL \Numerics-PR\corerun.exe ? 16,16 bits 7.944 ns 0.0573 ns 0.0536 ns 7.943 ns 7.854 ns 8.027 ns 1.00 0.01 - - - NA
Add Job-BYBGPF \Numerics-main\corerun.exe ? 16,16 bits 7.950 ns 0.0667 ns 0.0592 ns 7.936 ns 7.881 ns 8.078 ns 1.00 0.00 - - - NA
Subtract Job-ATGARL \Numerics-PR\corerun.exe ? 16,16 bits 7.773 ns 0.0409 ns 0.0342 ns 7.768 ns 7.687 ns 7.818 ns 1.00 0.01 - - - NA
Subtract Job-BYBGPF \Numerics-main\corerun.exe ? 16,16 bits 7.739 ns 0.0722 ns 0.0640 ns 7.716 ns 7.667 ns 7.867 ns 1.00 0.00 - - - NA
Multiply Job-ATGARL \Numerics-PR\corerun.exe ? 16,16 bits 7.048 ns 0.1515 ns 0.1417 ns 6.987 ns 6.899 ns 7.315 ns 0.76 0.01 - - - NA
Multiply Job-BYBGPF \Numerics-main\corerun.exe ? 16,16 bits 9.203 ns 0.0387 ns 0.0302 ns 9.199 ns 9.161 ns 9.250 ns 1.00 0.00 - - - NA
GreatestCommonDivisor Job-ATGARL \Numerics-PR\corerun.exe ? 16,16 bits 58.167 ns 0.2679 ns 0.2092 ns 58.185 ns 57.805 ns 58.545 ns 1.00 0.01 - - - NA
GreatestCommonDivisor Job-BYBGPF \Numerics-main\corerun.exe ? 16,16 bits 58.257 ns 0.2446 ns 0.2042 ns 58.192 ns 58.045 ns 58.739 ns 1.00 0.00 - - - NA
ModPow Job-ATGARL \Numerics-PR\corerun.exe ? 16,16,16 bits 128.543 ns 0.3381 ns 0.3163 ns 128.444 ns 128.095 ns 129.017 ns 1.00 0.00 - - - NA
ModPow Job-BYBGPF \Numerics-main\corerun.exe ? 16,16,16 bits 128.385 ns 0.2143 ns 0.1900 ns 128.377 ns 128.078 ns 128.758 ns 1.00 0.00 - - - NA
Divide Job-ATGARL \Numerics-PR\corerun.exe ? 16,8 bits 6.533 ns 0.0224 ns 0.0210 ns 6.526 ns 6.507 ns 6.565 ns 0.99 0.02 - - - NA
Divide Job-BYBGPF \Numerics-main\corerun.exe ? 16,8 bits 6.606 ns 0.1552 ns 0.1525 ns 6.529 ns 6.496 ns 6.998 ns 1.00 0.00 - - - NA
Remainder Job-ATGARL \Numerics-PR\corerun.exe ? 16,8 bits 6.633 ns 0.0316 ns 0.0280 ns 6.640 ns 6.593 ns 6.670 ns 1.00 0.01 - - - NA
Remainder Job-BYBGPF \Numerics-main\corerun.exe ? 16,8 bits 6.635 ns 0.0243 ns 0.0227 ns 6.642 ns 6.594 ns 6.663 ns 1.00 0.00 - - - NA
ModPow Job-ATGARL \Numerics-PR\corerun.exe ? 16384,16384,64 bits 2,155,055.709 ns 7,273.9877 ns 6,074.1092 ns 2,153,117.188 ns 2,148,750.781 ns 2,166,921.875 ns 1.00 0.00 - - 38 B 1.00
ModPow Job-BYBGPF \Numerics-main\corerun.exe ? 16384,16384,64 bits 2,146,563.504 ns 9,743.2304 ns 8,637.1199 ns 2,148,190.234 ns 2,134,060.938 ns 2,162,544.531 ns 1.00 0.00 - - 38 B 1.00
Divide Job-ATGARL \Numerics-PR\corerun.exe ? 65536,32768 bits 3,983,227.009 ns 13,428.3687 ns 11,903.8991 ns 3,981,090.625 ns 3,961,350.000 ns 4,002,900.000 ns 1.00 0.01 - - 4154 B 1.00
Divide Job-BYBGPF \Numerics-main\corerun.exe ? 65536,32768 bits 3,975,740.084 ns 15,670.9255 ns 13,085.9325 ns 3,970,307.031 ns 3,959,661.719 ns 3,999,364.844 ns 1.00 0.00 - - 4141 B 1.00
Remainder Job-ATGARL \Numerics-PR\corerun.exe ? 65536,32768 bits 3,974,530.529 ns 24,319.3436 ns 20,307.7533 ns 3,964,285.938 ns 3,959,059.375 ns 4,021,692.188 ns 1.00 0.00 - - 4133 B 1.00
Remainder Job-BYBGPF \Numerics-main\corerun.exe ? 65536,32768 bits 3,974,506.370 ns 7,520.5560 ns 6,280.0048 ns 3,974,834.375 ns 3,962,071.875 ns 3,984,807.812 ns 1.00 0.00 - - 4133 B 1.00
Add Job-ATGARL \Numerics-PR\corerun.exe ? 65536,65536 bits 1,837.498 ns 12.0748 ns 10.7040 ns 1,838.436 ns 1,824.029 ns 1,854.236 ns 0.99 0.02 1.3093 0.0370 8224 B 1.00
Add Job-BYBGPF \Numerics-main\corerun.exe ? 65536,65536 bits 1,862.005 ns 35.5598 ns 34.9245 ns 1,846.984 ns 1,834.487 ns 1,957.784 ns 1.00 0.00 1.3055 0.0369 8224 B 1.00
Subtract Job-ATGARL \Numerics-PR\corerun.exe ? 65536,65536 bits 1,832.820 ns 19.3712 ns 17.1720 ns 1,825.932 ns 1,805.557 ns 1,870.053 ns 1.00 0.01 1.3056 0.0369 8216 B 1.00
Subtract Job-BYBGPF \Numerics-main\corerun.exe ? 65536,65536 bits 1,833.221 ns 14.5310 ns 12.8813 ns 1,835.607 ns 1,813.245 ns 1,861.410 ns 1.00 0.00 1.3022 0.0366 8216 B 1.00
Multiply Job-ATGARL \Numerics-PR\corerun.exe ? 65536,65536 bits 716,077.273 ns 5,470.6546 ns 4,568.2444 ns 717,945.739 ns 707,717.614 ns 721,582.102 ns 1.00 0.02 - - 16410 B 1.00
Multiply Job-BYBGPF \Numerics-main\corerun.exe ? 65536,65536 bits 714,345.576 ns 10,665.7901 ns 9,454.9451 ns 710,489.773 ns 705,298.580 ns 735,248.580 ns 1.00 0.00 - - 16410 B 1.00
GreatestCommonDivisor Job-ATGARL \Numerics-PR\corerun.exe ? 65536,65536 bits 3,594,553.234 ns 32,745.2390 ns 25,565.3287 ns 3,596,821.642 ns 3,557,274.627 ns 3,628,395.522 ns 1.01 0.01 - - 12 B 1.00
GreatestCommonDivisor Job-BYBGPF \Numerics-main\corerun.exe ? 65536,65536 bits 3,568,474.845 ns 13,383.5828 ns 11,864.1975 ns 3,571,504.348 ns 3,550,014.493 ns 3,586,552.174 ns 1.00 0.00 - - 12 B 1.00

Slight regression for some tests. It's better to identify why.

@huoyaoyuan
Copy link
Member Author

The benchmark may benefit from #71567

@tannergooding
Copy link
Member

tannergooding commented Jul 5, 2022 •
edited
Loading

Slight regression for some tests. It's better to identify why.

Some of these tests are fairly noisy and things like loop or data alignment can visibly impact the numbers. Of the regressions, they all look to be very minor (well within noise, often just 1-5ns) deviations and so even if legitimate, this is something we'd be willing to take to improve code readability, maintainability, and in the case of GetHashCode to reduce overall collision chances.

@dakersnar is going to take a look at this as a secondary review in the next couple days and get it merged.

@drewnoakes
Copy link
Member

@drewnoakes is going to take a look at this as a secondary review in the next couple days and get it merged.

@tannergooding I think you've got the wrong Drew!

@tannergooding
Copy link
Member

Indeed, meant to tag @dakersnar. Auto completion fail, sorry for the noise 😅

@tannergooding tannergooding requested a review from dakersnar July 7, 2022 22:59
dakersnar
dakersnar approved these changes Jul 8, 2022
View reviewed changes
Copy link
Contributor

@dakersnar dakersnar left a comment

Choose a reason for hiding this comment

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

I agree with Stephen's suggestions, but looks good!

@stephentoub stephentoub merged commit c8cc6c1 into dotnet:main Jul 10, 2022
@huoyaoyuan huoyaoyuan deleted the numerics-cleaup-redo branch July 11, 2022 03:15
@ghost ghost locked as resolved and limited conversation to collaborators Aug 10, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@tannergooding tannergooding tannergooding approved these changes

@dakersnar dakersnar dakersnar approved these changes

@stephentoub stephentoub stephentoub approved these changes

Assignees
No one assigned
Labels
area-System.Numerics community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@huoyaoyuan @stephentoub @tannergooding @drewnoakes @dakersnar