-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Improve double
formatting performance on net8+ and fix equality incorrectness re special doubles
#2928
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
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- use in physicalconnection
double
formatting performance on net8+double
formatting performance on net8+ and fix equality incorrectness re special doubles
delta, before (limiting to relevant) - note in particular allocs (scroll right): | Method | s | value | host | port | Mean | Error | StdDev | Median | Op/s | Gen0 | Allocated |
|-------------- |------- |---------- |--------- |------ |-----------:|----------:|----------:|-----------:|--------------:|-------:|----------:|
| ParseDouble | -1 | ? | ? | ? | 56.424 ns | 1.1702 ns | 2.7122 ns | 56.113 ns | 17,722,894.2 | - | - |
| ParseDouble | -inf | ? | ? | ? | 2.900 ns | 0.0993 ns | 0.1815 ns | 2.879 ns | 344,866,763.5 | - | - |
| FormatDouble | ? | -Infinity | ? | ? | 3.012 ns | 0.1001 ns | 0.1558 ns | 3.008 ns | 331,955,012.2 | - | - |
| FormatDouble | ? | -1 | ? | ? | 93.365 ns | 1.8845 ns | 2.8779 ns | 93.333 ns | 10,710,608.6 | 0.0050 | 32 B |
| ParseDouble | 0 | ? | ? | ? | 2.694 ns | 0.0872 ns | 0.1134 ns | 2.703 ns | 371,187,328.7 | - | - |
| FormatDouble | ? | 0 | ? | ? | 45.925 ns | 0.9217 ns | 0.9052 ns | 45.977 ns | 21,774,478.6 | 0.0038 | 24 B |
| ParseDouble | 123442 | ? | ? | ? | 81.211 ns | 1.6605 ns | 2.1000 ns | 81.056 ns | 12,313,592.5 | - | - |
| FormatDouble | ? | 123442 | ? | ? | 149.446 ns | 3.0354 ns | 4.8145 ns | 149.223 ns | 6,691,386.7 | 0.0062 | 40 B |
| ParseDouble | 64 | ? | ? | ? | 62.042 ns | 1.2612 ns | 2.5763 ns | 61.844 ns | 16,118,182.2 | - | - |
| FormatDouble | ? | NaN | ? | ? | 13.124 ns | 0.2994 ns | 0.4920 ns | 13.071 ns | 76,197,072.3 | - | - |
| FormatDouble | ? | 64 | ? | ? | 124.475 ns | 2.5356 ns | 3.4708 ns | 124.267 ns | 8,033,736.3 | 0.0050 | 32 B |
| ParseDouble | nan | ? | ? | ? | 3.468 ns | 0.1105 ns | 0.1512 ns | 3.459 ns | 288,367,071.9 | - | - | after (note no allocs): | Method | s | value | host | port | Mean | Error | StdDev | Op/s | Gen0 | Allocated |
|-------------- |------- |---------- |--------- |------ |-----------:|----------:|----------:|--------------:|-------:|----------:|
| ParseDouble | -1 | ? | ? | ? | 54.380 ns | 1.1219 ns | 1.9051 ns | 18,389,178.2 | - | - |
| ParseDouble | -inf | ? | ? | ? | 4.124 ns | 0.1213 ns | 0.2423 ns | 242,509,622.1 | - | - |
| FormatDouble | ? | -Infinity | ? | ? | 2.991 ns | 0.1011 ns | 0.2087 ns | 334,327,800.4 | - | - |
| FormatDouble | ? | -1 | ? | ? | 80.053 ns | 1.6459 ns | 4.2191 ns | 12,491,696.1 | - | - |
| ParseDouble | 0 | ? | ? | ? | 2.862 ns | 0.0978 ns | 0.1372 ns | 349,429,539.4 | - | - |
| FormatDouble | ? | 0 | ? | ? | 32.396 ns | 0.6835 ns | 0.9356 ns | 30,867,908.5 | - | - |
| ParseDouble | 123442 | ? | ? | ? | 77.273 ns | 1.5881 ns | 2.6094 ns | 12,941,150.5 | - | - |
| FormatDouble | ? | 123442 | ? | ? | 129.659 ns | 2.5727 ns | 5.5379 ns | 7,712,560.7 | - | - |
| ParseDouble | 64 | ? | ? | ? | 60.785 ns | 1.2505 ns | 3.0675 ns | 16,451,422.2 | - | - |
| FormatDouble | ? | NaN | ? | ? | 20.836 ns | 0.4540 ns | 0.5047 ns | 47,994,833.7 | - | - |
| FormatDouble | ? | 64 | ? | ? | 109.433 ns | 2.1685 ns | 2.5814 ns | 9,138,011.9 | - | - |
| ParseDouble | nan | ? | ? | ? | 3.551 ns | 0.1058 ns | 0.2114 ns | 281,591,155.7 | - | - | only "regression" is parse |
NickCraver
approved these changes
Jul 25, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
by using
double : IUtf8Formattable
open question: this is net8+; however, for anything netcoreapp2.1+ we could actually use the
Span<char>
overload to a scratch buffer, which would avoid astring
alloc (and copy down if needed, like we do today fromstring
). However, this would only really impact net6 which is EOL, so I do not propose to introduce extra code for that.This PR also fixes a pre-existing oddity where, (as
RedisValue
):"na" != "NA"
"nan" == "NAN"
"nand" != "NAND"
(and similar for
inf
)