Skip to content

Move IPv6 ScopeId storage slot out of common path in Uri #117292

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.

Sign up for GitHub

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

Jump to bottom
Merged
MihaZupan merged 1 commit into from
Jul 4, 2025
Merged

Move IPv6 ScopeId storage slot out of common path in Uri #117292

MihaZupan merged 1 commit into from
Jul 4, 2025

Conversation

@MihaZupan
Copy link
Member

@MihaZupan MihaZupan commented Jul 3, 2025

Moves the ScopeId field from UriInfo to UriInfo.MoreInfo under the assumption that scopes are much rared compared to DNS / IPv4 / regular IPv6 hosts.
Saves 8 bytes for those common cases.

Method Toolchain Mean Error Ratio Allocated Alloc Ratio
Dns main 136.6 ns 0.90 ns 1.00 168 B 1.00
Dns pr 131.3 ns 0.67 ns 0.96 160 B 0.95
IPv4 main 180.0 ns 1.97 ns 1.00 176 B 1.00
IPv4 pr 174.4 ns 0.87 ns 0.97 168 B 0.95
IPv6 main 163.5 ns 2.21 ns 1.00 168 B 1.00
IPv6 pr 155.7 ns 0.46 ns 0.95 160 B 0.95
WithScopeId main 266.7 ns 0.86 ns 1.00 376 B 1.00
WithScopeId pr 283.4 ns 3.00 ns 1.06 432 B 1.15 
[MemoryDiagnoser(false)]
public class UriBench
{
    [Benchmark]
    public string Dns() => new Uri("http://host/").Host;

    [Benchmark]
    public string IPv4() => new Uri("http://127.0.0.1/").Host;

    [Benchmark]
    public string IPv6() => new Uri("http://[::]/").Host;

    [Benchmark]
    public string WithScopeId() => new Uri("http://[::%42]/").Host;
}

@MihaZupan MihaZupan added this to the 10.0.0 milestone Jul 3, 2025
@MihaZupan MihaZupan self-assigned this Jul 3, 2025
@MihaZupan MihaZupan marked this pull request as ready for review July 3, 2025 21:28
Copilot AI review requested due to automatic review settings July 3, 2025 21:28
@dotnet-policy-service
Copy link
Contributor

dotnet-policy-service bot commented Jul 3, 2025

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

Copilot AI reviewed Jul 3, 2025
View reviewed changes
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 relocates the ScopeId field from UriInfo into the nested MoreInfo class to save memory for the common (non-IPv6-with-scope) cases and updates all host-parsing and serialization logic accordingly.

  • Moved the ScopeId storage slot into MoreInfo
  • Updated Host property, CreateHostString, GetHostViaCustomSyntax, CreateHostStringHelper, and RecreateParts to reference the new location
  • Changed CreateHostStringHelper signature to accept a UriInfo instance instead of a ref string? scopeId
Comments suppressed due to low confidence (2)

src/libraries/System.Private.Uri/src/System/Uri.cs:2526

  • Add or update unit tests to cover parsing and serialization of IPv6 URIs with scope IDs, ensuring MoreInfo.ScopeId is correctly set and serialized.
                        info.MoreInfo.ScopeId = new string(scopeIdSpan);

src/libraries/System.Private.Uri/src/System/Uri.cs:159

  • The _moreInfo field was made public but is only used internally; revert it to private to preserve encapsulation within UriInfo.
            public MoreInfo? _moreInfo;

This was referenced Jul 4, 2025
Closed
Closed
Open
Closed
Open
Open
Open
rzikm
rzikm approved these changes Jul 4, 2025
View reviewed changes
Copy link
Member

@rzikm rzikm left a comment

Choose a reason for hiding this comment

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

LGTM

@MihaZupan MihaZupan merged commit 47378ac into dotnet:main Jul 4, 2025
75 of 89 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Aug 4, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

Copilot code review Copilot Copilot left review comments

@rzikm rzikm rzikm approved these changes

Assignees

@MihaZupan MihaZupan

Labels

area-System.Net

Projects

None yet

Milestone

10.0.0

Development

Successfully merging this pull request may close these issues.

2 participants

@MihaZupan @rzikm