Reduce overhead reading byte/sbyte arrays from IndexedReader #405
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Traces gathered over the test suite show: - ~2.7% CPU spent in `IndexedReader.GetByte(int)` - ~0.4% CPU spent in `IndexedReader.GetSByte(int)` Looking at the main callers shows loops in `TiffReader` that call these methods in loops. This approach accrues overhead per-byte due to bounds checking and virtual dispatch. Instead, use the `Span<byte>` overload of `GetBytes` that performs the bounds checking once, then copies data in a single call. It may be possible to give a similar treatment to the handling of other TIFF format codes, though they're not currently showing up on traces and would be more complex to implement due to byte-ordering issues (`byte` and `sbyte` being immune from those).
|
Looks good! |
|
Once these changes are in I'll gather some new traces and see what surfaces. We're so IO-bound that I think we'll need to rethink how we read from disk/network to get much more improvement on the perf side. |
As part of that a good goal would be to enable async IO so that we're not blocking threads on IO operations. The way we currently read data, byte-by-byte, doesn't lend itself well to that much async, as the overhead adds up. I lean towards async IO for pulling larger chunks of data from the file, storing those chunks as |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Traces gathered over the test suite show:
IndexedReader.GetByte(int)IndexedReader.GetSByte(int)Looking at the main callers shows loops in
TiffReaderthat call these methods in loops. This approach accrues overhead per-byte due to bounds checking and virtual dispatch.Instead, use the
Span<byte>overload ofGetBytesthat performs the bounds checking once, then copies data in a single call.It may be possible to give a similar treatment to the handling of other TIFF format codes, though they're not currently showing up on traces and would be more complex to implement due to byte-ordering issues (
byteandsbytebeing immune from those).