Skip to content

Fix: Address missing sizeHint in BuffersExtensions write logic#126776

Closed
Plerx2493 wants to merge 1 commit intodotnet:mainfrom
Plerx2493:fix/missing-size-hint
Closed

Fix: Address missing sizeHint in BuffersExtensions write logic#126776
Plerx2493 wants to merge 1 commit intodotnet:mainfrom
Plerx2493:fix/missing-size-hint

Conversation

@Plerx2493
Copy link
Copy Markdown

Refactor BuffersExtensions to improve memory allocation efficiency. This update ensures the IBufferWriter receives the correct sizeHint based on the provided span size during data write operations.

Copilot AI review requested due to automatic review settings April 10, 2026 22:53
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Apr 10, 2026
@dotnet-policy-service
Copy link
Copy Markdown
Contributor

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

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Refactors BuffersExtensions.Write to pass an explicit sizeHint to IBufferWriter<T>.GetSpan(...), improving allocation behavior by requesting spans sized to the data being written.

Changes:

  • Update initial Write path to call GetSpan(value.Length) instead of GetSpan().
  • Update multi-segment write loop to call GetSpan(input.Length) for remaining data rather than GetSpan().

Comment on lines 116 to 120
public static void Write<T>(this IBufferWriter<T> writer, ReadOnlySpan<T> value)
{
Span<T> destination = writer.GetSpan();
Span<T> destination = writer.GetSpan(value.Length);

// Fast path, try copying to the available memory directly
Copy link

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

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

This change alters the observable behavior of Write for writers that allocate based on sizeHint (e.g., the existing TestBufferWriterMultiSegment in System.Memory tests). With GetSpan(value.Length) / GetSpan(input.Length), those tests will no longer write one byte per segment (they'll typically write the whole span in a single segment), so current assertions like Comitted.Count == 12 will fail. Please update/add tests to validate the new contract (that the sizeHint passed equals the remaining input length) rather than relying on the old sizeHint == 0 growth behavior.

Copilot uses AI. Check for mistakes.
@Plerx2493
Copy link
Copy Markdown
Author

As it seems i was confused by the naming of the parameter and this would be the wrong behaivior.

/// <param name="sizeHint">The minimum length of the returned <see cref="System.Span{T}" />. If 0, a non-empty buffer of arbitrary size is returned.</param>

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

Labels

area-System.Memory 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.

2 participants