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

[C#] BitUtility.cs performance improvement #45559

Open
m-v-w opened this issue Feb 18, 2025 · 1 comment
Open

[C#] BitUtility.cs performance improvement #45559

m-v-w opened this issue Feb 18, 2025 · 1 comment

Comments

@m-v-w
Copy link

m-v-w commented Feb 18, 2025

Describe the enhancement requested

The C# implementation of the BitUtility helper class seems to allocate an array every time GetBit is called:

private static ReadOnlySpan<byte> BitMask => new byte[] {
    1, 2, 4, 8, 16, 32, 64, 128
};
public static bool GetBit(ReadOnlySpan<byte> data, int index) =>
    (data[index / 8] & BitMask[index % 8]) != 0;

using a static cached BitMask array or creating the bit mask using a shift operator should increase performance.

Component(s)

C#

@kou kou changed the title BitUtility.cs performance improvement [C#] BitUtility.cs performance improvement Feb 19, 2025
@alexdegroot
Copy link

I've created a PR which solves the arrow allocation on every call. It could be further optimized by using a shift operator:

public static bool GetBit(ReadOnlySpan<byte> data, int index) => 
        (data[index / 8] & (1 << (index % 8))) != 0;

Unfortunately, this will break this contract:

            [Theory]
            [InlineData(null, 0)]
            [InlineData(new byte[] { 0b00000000 }, -1)]
            public void ThrowsWhenBitIndexOutOfRange(byte[] data, int index)
            {
                Assert.Throws<IndexOutOfRangeException>(() =>
                    BitUtility.GetBit(data, index));
            }

I'm curious though if that contract makes sense. The other overload of the method(although only used for test purposes), doesn't throw and fails therefore:

            [Theory]
            [InlineData(0b_11110011, 0)]
            [InlineData(0b_11110011, -1)]
            public void ThrowsWhenBitIndexOutOfRange_ByteOverload(byte data, int index)
            {
                Assert.Throws<IndexOutOfRangeException>(() =>
                    BitUtility.GetBit(data, index));
            }

Proposal: either accept my PR or let me create a new one where I include both fixes:

  1. Remove the ThrowsWhenBitIndexOutOfRange-tests
  2. Use the shift operator

@CurtHagenlocher maybe you can shine a light on this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants