Skip to content

Conversation

@Chasmical
Copy link

Change the length of the char buffer used in Version.ToString() from 47 to 43 chars (4 non-negative Int32s + 3 periods). 2147483647.2147483647.2147483647.2147483647. Even if a version has invalid internal state (any component less than -1), the TryFormat method just casts them to UInt32 anyway.

Change the length of the char buffer used in Version.ToString()
from 47 to 43 chars (4 non-negative Int32s + 3 periods). Even if
a version has invalid internal state (any component less than -1),
the TryFormat method just casts them to UInt32 anyway.
@ghost ghost added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label May 21, 2024
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label May 21, 2024
@Chasmical
Copy link
Author

Oh, I didn't realize that Number.UInt32NumberBufferLength is the same as Int32NumberBufferLength - 11, for some reason.

@EgorBo
Copy link
Member

EgorBo commented May 21, 2024

To be fair, this doesn't reduce the actual stack allocation size - JIT is going to round up both to % 16 anyway, e.g. see sharplab

Number.Int32NumberBufferLength is only used in System.Number class and
here. The extra 1 to length is designated for the terminating nulls.
It would make more sense not to use it here.
@Chasmical
Copy link
Author

I see. 😄 Then, I guess it could still improve readability of the decompiled code. It was confusing to see 47 characters allocated when the actual maximum length is 43.

On another note, looks like Number.Int32NumberBufferLength isn't used anywhere else, besides the Number class and here, and the extra 1 to length is actually for the terminating nulls. Would make more sense not to use it here, in case Number.NumberBuffer changes.

// We need 1 additional byte, per length, for the terminating null
internal const int DecimalNumberBufferLength = 29 + 1 + 1; // 29 for the longest input + 1 for rounding
internal const int DoubleNumberBufferLength = 767 + 1 + 1; // 767 for the longest input + 1 for rounding: 4.9406564584124654E-324
internal const int Int32NumberBufferLength = 10 + 1; // 10 for the longest input: 2,147,483,647

@stephentoub
Copy link
Member

Thank you, but I'd rather keep it how it is today. I think it's more maintainable as it is currently and this doesn't actually improve perf.

@Chasmical Chasmical closed this May 21, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Jun 21, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

community-contribution Indicates that the PR has been added by a community member needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants