Skip to content

Conversation

@GrabYourPitchforks
Copy link
Member

Removes unnecessary calls to the unsafe MemoryMarshal.GetArrayDataReference and MemoryMarshal.CreateSpan methods. The only thing this was doing was saving a single bounds check per call to FindIndex or TryFindItem, and this single bounds check is dwarfed by the call to IEqualityComparer<T>.Equals within the loop.

I hoisted the span into a local within the FindIndex method to ensure the bounds check occurs only once ahead of the loop, not within the loop itself.

Copilot AI review requested due to automatic review settings December 3, 2025 01:23
@github-actions github-actions bot added the needs-area-label Used by the dotnet-issue-labeler to label those issues which couldn't be triaged automatically label Dec 3, 2025
Copy link
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

This PR removes unnecessary unsafe memory operations from AdaptiveCapacityDictionary by replacing MemoryMarshal.GetArrayDataReference and MemoryMarshal.CreateSpan with the safer AsSpan method. The change also optimizes the FindIndex method by caching the span property access outside the loop.

  • Simplified ArrayStorageSpan property to use AsSpan(0, _count) instead of unsafe MemoryMarshal APIs
  • Optimized FindIndex by caching ArrayStorageSpan to avoid repeated property access within the loop

Copy link
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

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

@gfoidl gfoidl added area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed needs-area-label Used by the dotnet-issue-labeler to label those issues which couldn't be triaged automatically labels Dec 3, 2025
@GrabYourPitchforks GrabYourPitchforks enabled auto-merge (squash) December 4, 2025 01:10
@dotnet-policy-service
Copy link
Contributor

Looks like this PR hasn't been active for some time and the codebase could have been changed in the meantime.
To make sure no conflicting changes have occurred, please rerun validation before merging. You can do this by leaving an /azp run comment here (requires commit rights), or by simply closing and reopening.

@dotnet-policy-service dotnet-policy-service bot added the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Dec 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun reduce-unsafe

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants