Skip to content

Conversation

@Rob-Hague
Copy link
Contributor

When the value is positive (we don't need to do two's complement conversion) and on a little endian machine, we can copy the bytes out from the underlying uint[] storage.

Method Job BitLength IsBigEndian Mean Error StdDev Ratio
ToByteArray PR 1024 False 21.64 ns 0.020 ns 0.015 ns 0.41
ToByteArray main 1024 False 53.38 ns 0.157 ns 0.140 ns 1.00
ToByteArray PR 1024 True 23.45 ns 0.021 ns 0.018 ns 0.44
ToByteArray main 1024 True 53.48 ns 0.150 ns 0.125 ns 1.00
public class Benchmarks
{
    [Params(1024)]
    public int BitLength;

    [Params(true, false)]
    public bool IsBigEndian;

    [Params(true)]
    public bool IsPositive;

    private BigInteger n;

    [GlobalSetup]
    public void Setup()
    {
        Random r = new(123);
        byte[] bytes = new byte[BitLength / 8];
        r.NextBytes(bytes);
        if (IsPositive)
        {
            bytes[^1] &= 0x7F;
        }
        else
        {
            bytes[^1] |= 0x80;
        }
        n = new BigInteger(bytes);
    }

    [Benchmark] public byte[] ToByteArray() => n.ToByteArray(isBigEndian: IsBigEndian);
}

When the value is positive (we don't need to do two's complement conversion) and
on a little endian machine, we can copy the bytes out from the underlying uint[]
storage.
@github-actions github-actions bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label May 10, 2025
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label May 10, 2025
Copy link
Member

@huoyaoyuan huoyaoyuan left a comment

Choose a reason for hiding this comment

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

The whole method is not as efficient as it can. There are multiple problems, for example BitConverter.IsLittleEndian is an intrinsic so it shouldn't be passed as a parameter.

A bottom-up rewrite is better.

@huoyaoyuan huoyaoyuan added area-System.Numerics and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels May 10, 2025
@dotnet-policy-service
Copy link
Contributor

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

@Rob-Hague
Copy link
Contributor Author

@huoyaoyuan isBigEndian is a parameter passed to ToByteArray

@huoyaoyuan
Copy link
Member

@huoyaoyuan isBigEndian is a parameter passed to ToByteArray

Yes, you are right, I misread it as the same of BitConverter.IsLittleEndian.

Copy link
Member

@stephentoub stephentoub left a comment

Choose a reason for hiding this comment

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

Thanks.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-System.Numerics community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants