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

Use spans to improve performance in StreamExts. #21186

Merged
merged 1 commit into from Nov 10, 2023

Conversation

RoosterDragon
Copy link
Member

Also avoid ReadBytes calls that allocate a buffer by either updating the stream position (if not interested in the bytes), by reusing an input buffer (if interested in the bytes), or using a stackalloc buffer to avoid the allocation (for small reads).


This isn't motivated by any specific performance benchmark, but rather just on the basis that moving towards using more span features in something that affects a large chunk of the codebase such as streams will provide some incremental gains.

Copy link
Member

@PunkPun PunkPun left a comment

Choose a reason for hiding this comment

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

otherwise LGTM

OpenRA.Game/StreamExts.cs Show resolved Hide resolved
OpenRA.Mods.Common/AudioLoaders/OggLoader.cs Outdated Show resolved Hide resolved
@RoosterDragon
Copy link
Member Author

Added some missing newlines.

Also avoid ReadBytes calls that allocate a buffer by either updating the stream position (if not interested in the bytes), by reusing an input buffer (if interested in the bytes), or using a stackalloc buffer to avoid the allocation (for small reads).
Copy link
Member

@PunkPun PunkPun left a comment

Choose a reason for hiding this comment

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

LGTM

@PunkPun PunkPun merged commit 5d91b67 into OpenRA:bleed Nov 10, 2023
3 checks passed
@PunkPun
Copy link
Member

PunkPun commented Nov 10, 2023

Changelog

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 this pull request may close these issues.

None yet

5 participants