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

Excessive bounds checking when reading multi-byte values #62

Closed
drewnoakes opened this issue Oct 7, 2016 · 1 comment · Fixed by #327
Closed

Excessive bounds checking when reading multi-byte values #62

drewnoakes opened this issue Oct 7, 2016 · 1 comment · Fixed by #327

Comments

@drewnoakes
Copy link
Owner

For example, ByteArrayReader.GetInt32 validates the index for four bytes. It then calls GetByte four times, which causes a further four validations.

One potential solution is to have a non-public GetByteInternal method that doesn't validate.

Investigate at the same time whether we can have the GetByte calls be inlined. Being virtual probably nixes that.

@twest820
Copy link
Contributor

twest820 commented Mar 3, 2018

Approaches in the direction of BitConverter.ToInt32(GetBytes(index, 4)) with Array.Reverse() or such for endianness might be worth a look. I haven't profiled this space much as I'm usually working with SIMD intrinsics for such things I but recall finding they're faster than shifts, presumably because it's C# for reinterpret_cast<>. In general, among the various reader classes I've looked at, it appears to me shifting to a more conventional offset + length approach for indexing into a byte[] might be beneficial. In particular, it moves the GetBytes() contract away from having to copy to a new byte[]. However, making much of this performant is contingent on the reader's maintaining it's own byte[] buffer.

Virtual calls can be inlined when the compiler knows it's dealing with a sealed type. Haven't looked to see if this actually happens, but it's a pretty niche optimization regardless.

This was referenced May 2, 2018
drewnoakes added a commit that referenced this issue Jan 24, 2023
Fixes #62

Adds a new abstract overload for reading a single byte. That method will not do any explicit bounds validation, so callers must do that beforehand. In cases where multiple bytes are read, the validation can be performed once per value, rather than once per byte.
drewnoakes added a commit that referenced this issue May 8, 2023
Fixes #62

Adds a new abstract overload for reading a single byte. That method will not do any explicit bounds validation, so callers must do that beforehand. In cases where multiple bytes are read, the validation can be performed once per value, rather than once per byte.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants