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 string.Create to build absolute URI #29448

Merged
2 commits merged into from
Jan 26, 2021

Conversation

paulomorgado
Copy link
Contributor

@paulomorgado paulomorgado commented Jan 20, 2021

PR Title

Use string.Creat to build absolute URI.

PR Description

UriHelper.BuildAbsolute creates an intermediary string for the combined path that is used only for concatenating with the other components to create the final URL.

It also uses a non-pooled StringBuilder that is instantiated on every invocation. Although optimized in size, it is a heap allocation with an intermediary buffer.

Using string.Create will bring a significant performance improvement.

Fixes #28905

@Tratcher
Copy link
Member

Doesn't compile?

@paulomorgado paulomorgado force-pushed the paulomorgado/issue-28905 branch from 4bc7fce to aaa5ecc Compare January 20, 2021 20:35
@paulomorgado paulomorgado changed the title Use string.Creat to build absolute URI Use string.Create to build absolute URI Jan 20, 2021
@paulomorgado
Copy link
Contributor Author

Sorry about that.

It was a bad copy+past and I still haven't mastered the art of working with this code base.

Base automatically changed from master to main January 22, 2021 01:33
@BrennanConroy BrennanConroy added the community-contribution Indicates that the PR has been added by a community member label Jan 22, 2021
@paulomorgado paulomorgado force-pushed the paulomorgado/issue-28905 branch 2 times, most recently from 42c84ea to 9a21cd8 Compare January 25, 2021 19:57
@paulomorgado paulomorgado force-pushed the paulomorgado/issue-28905 branch from 9a21cd8 to e82f0b2 Compare January 25, 2021 23:10
@ghost
Copy link

ghost commented Jan 26, 2021

Hello @Tratcher!

Because this pull request has the auto-merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

@ghost ghost merged commit 5cdb135 into dotnet:main Jan 26, 2021
@Tratcher
Copy link
Member

Thanks.

@ghost
Copy link

ghost commented Jan 26, 2021

Hi @Tratcher. It looks like you just commented on a closed PR. The team will most probably miss it. If you'd like to bring something important up to their attention, consider filing a new issue and add enough details to build context.

@Tratcher Tratcher added this to the 6.0-preview1 milestone Jan 26, 2021
@amcasey amcasey added area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed area-runtime labels Aug 24, 2023
@paulomorgado paulomorgado mentioned this pull request Jun 5, 2024
4 tasks
This pull request was closed.
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 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.

UriHelper.BuildAbsolute: opportunity for performance improvement
4 participants