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

[API Proposal]: Decimal.GetBits overload without required array or span creation #110713

Open
jwosty opened this issue Dec 14, 2024 · 8 comments
Open
Assignees
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Numerics untriaged New issue has not been triaged by the area owner

Comments

@jwosty
Copy link

jwosty commented Dec 14, 2024

Background and motivation

It would be nice to have a Decimal API that just gives me the 4 individual bit parameters, without having to pack them into an array or span just for the caller to pull them right out. Such an overload would also have no possibility of throwing exceptions or causing bounds checks or anything like that, and I would imagine be quite JIT friendly.

Would also be particularly helpful for allocation-free F# code, since F# doesn't have stackalloc. Currently you have to resort either to allocating/pooling an array or span (which feels silly because you know exactly how many elements there are going to be), or relying on decimal's internal representation and doing evil bit casting.

This becomes particularly unfortunate when you need to care about the distinction between -0.0m and +0.0m, as the only way to do that (as of this writing) is to inspect the binary representation.

API Proposal

namespace System;

public struct Decimal
{
    public static void GetBits(decimal d, out Int32 low, out Int32 middle, out Int32 high, out Int32 flags);
}

API Usage

public static bool IsNegativeZero(decimal n)
{
    FakeDecimal.GetBits(n, out var low, out var middle, out var high, out var flags);
    return flags >> 31 != 0;
}

Alternative Designs

No response

Risks

No response

@jwosty jwosty added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Dec 14, 2024
@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label Dec 14, 2024
Copy link
Contributor

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

@jwosty jwosty changed the title [API Proposal]: Decimal.GetBits overload without array or span [API Proposal]: Decimal.GetBits overload without required array or span creation Dec 14, 2024
@tannergooding
Copy link
Member

It would be nice to have a Decimal API that just gives me the 4 individual bit parameters, without having to pack them into an array or span just for the caller to pull them right out

This is not aligned with the normal API patterns exposed by the BCL, it's unlikely to pass API review.

The standard pattern for allocation free is the following:

Span<int> bits = stackalloc int[4];
decimal.GetBits(value, bits);

If you really prefer the out pattern style, that can be trivially achieved with your own helper API.

Such an overload would also have no possibility of throwing exceptions or causing bounds checks or anything like that, and I would imagine be quite JIT friendly.

The existing patterns around span are already JIT friendly and the JIT understands where bounds elision and other things can happen. There's potentially "more" the JIT can do beyond what its doing today, but the existing support is generally doing all the right and expected things.

This becomes particularly unfortunate when you need to care about the distinction between -0.0m and +0.0m, as the only way to do that (as of this writing) is to inspect the binary representation.

For decimal in particular, unlike float or double, the existence of negative zero largely isn't supposed to be important/relevant. The type is largely designed for and around currency, not for scientific or other usages (which would require representation of +infinity, -infinity, and some kind of NaN value).

If you do need to check for something like IsNegativeZero you can do this via (value == 0) && decimal.IsNegative(value), or you can build your own dedicated helper if you need it to be explicitly more efficient. -- Notably the API usage example isn't correct as it isn't accounting for multiple kinds of zero (-0m vs -0.0m vs -0.00m, etc) and isn't actually checking anything other than "Is the value negative".

@En3Tho
Copy link
Contributor

En3Tho commented Dec 15, 2024

F# does have stackalloc. That statement about not having it is simply untrue

@tannergooding
Copy link
Member

tannergooding commented Dec 15, 2024

Right, Nativeptr.stackalloc 4 in particular. I don’t recall if F# has a built in helper that gives you a correctly scoped span over this stack allocation, but you should be able to build an inline helper in the worst case

It would be appropriate to request a helper on the F# side for better safety and ease of use if it is missing

@kasperk81
Copy link
Contributor

they closed the proposal in favor of #52065
for now this works

let bits = Span<int>(NativePtr.toVoidPtr (NativePtr.stackalloc<int> 4), 4)
Decimal.GetBits(value, bits)

printfn "%d" bits.[0]
printfn "%d" bits.[1]
printfn "%d" bits.[2]
printfn "%d" bits.[3]

@jwosty
Copy link
Author

jwosty commented Dec 15, 2024

@tannergooding

This is not aligned with the normal API patterns exposed by the BCL, it's unlikely to pass API review.

What are the patterns to which you're referring? (asking out of genuine ignorance; I haven't interacted much with the runtime repos :) )

The standard pattern for allocation free is the following:
...
The existing patterns around span are already JIT friendly and the JIT understands where bounds elision and other things can happen. There's potentially "more" the JIT can do beyond what its doing today, but the existing support is generally doing all the right and expected things.

Sure; understood.

I guess the real thing that irks me is the lack of static type-safety, that's all. I.e. you have to read documentation to know you're supposed to get exactly 4 things out of it (and this will never change), when that knowledge could be encoded in the type system instead. It's more the principal of the thing! :) But it's true that it's so minor that you'd only make this blunder once

If you do need to check for something like IsNegativeZero you can do this via (value == 0) && decimal.IsNegative(value), or you can build your own dedicated helper if you need it to be explicitly more efficient. -- Notably the API usage example isn't correct as it isn't accounting for multiple kinds of zero (-0m vs -0.0m vs -0.00m, etc) and isn't actually checking anything other than "Is the value negative".

I could have sworn I read something in the docs yesterday specifying that IsPositive returns true for positive and zero values, and IsNegative returns true for negative and nonzero values (which doesn't even really make sense, come to think of it). Can't find anything suggesting this in the docs now that I'm re-reading them. I must have mixed something up; my mistake.

@En3Tho

F# does have stackalloc. That statement about not having it is simply untrue

@tannergooding

Right, Nativeptr.stackalloc 4 in particular.

Honestly forgot about that. This closed suggestion was more recent in my mind:

fsharp/fslang-suggestions#720 (comment)


In summary, I withdraw all of my points except for the one about type safety -- in other words, the same justification for why we prefer individual method arguments rather than just using arrays to represent them (like they sometimes do in dynamic languages where the distinction doesn't matter). (Of course it would be amazing to just provide a tuple return value but I know that's not really the BCL way and therefore I digress.) What are your thoughts on that in particular?

@jeffhandley
Copy link
Member

Assigned to @PranavSenthilnathan to consult with @tannergooding and finish triaging this.

@PranavSenthilnathan
Copy link
Member

This is not aligned with the normal API patterns exposed by the BCL, it's unlikely to pass API review.

What are the patterns to which you're referring? (asking out of genuine ignorance; I haven't interacted much with the runtime repos :) )

There are examples for GetBytes which are similar (not exactly the same since it returns returns byte[] instead of int[]).

BitConverter.TryWriteBytes(Span, Double)
Guid.TryWriteBytes(Span)

Just like those APIs, I see GetBits as a way to get the physical parts of a decimal, rather than the logical parts. This is similar to decimal.GetBytes but GetBytes needs to consider endianness while GetBits returns the raw ints. These lower level APIs always work on array/span and higher level APIs can add any convenience on top of it.

The correct way to get the logical parts would be to split up the sign and the scale factor from the fourth int like we do in our internal helper:

public static void GetBits(this decimal value, out bool isNegative, out byte scale, out uint low, out uint mid, out uint high)

As a public API I would expect this to have a different name. However, given that this can be implemented as a small helper and there are already other APIs usable for most common tasks (like decimal.IsNegative in (value == 0) && decimal.IsNegative(value) above), I don't really see much of a need for it unless we see more scenarios to justify it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Numerics untriaged New issue has not been triaged by the area owner
Projects
None yet
Development

No branches or pull requests

6 participants