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

perf(http): use Cow<[u8]> for setting header #20112

Merged
merged 2 commits into from
Aug 10, 2023

Conversation

bartlomieju
Copy link
Member

No description provided.

@bartlomieju bartlomieju requested a review from mmastrac August 10, 2023 01:36
@bartlomieju bartlomieju enabled auto-merge (squash) August 10, 2023 02:49
Comment on lines +401 to +403
Cow::Owned(bytes_vec) => unsafe {
HeaderValue::from_maybe_shared_unchecked(bytes::Bytes::from(bytes_vec))
},
Copy link
Member

Choose a reason for hiding this comment

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

I think Cow::Owned is when strings are not latin-1

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah yeah this one is tricky, but #[string(onebyte)] asserts that it is latin-1 even in the owned case. It'll throw it not. You can safely assume borrowed and owned are latin-1

Copy link
Contributor

@mmastrac mmastrac left a comment

Choose a reason for hiding this comment

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

LGTM

@bartlomieju bartlomieju merged commit 634f5cc into denoland:main Aug 10, 2023
@bartlomieju bartlomieju deleted the http_use_cow branch August 11, 2023 06:57
littledivy pushed a commit to littledivy/deno that referenced this pull request Aug 21, 2023
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.

3 participants