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.

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

More allocator friendly Header definitions #1104

Closed
arthurprs opened this issue Mar 27, 2017 · 4 comments
Closed

More allocator friendly Header definitions #1104

arthurprs opened this issue Mar 27, 2017 · 4 comments
Labels
A-headers Area: headers. E-easy Effort: easy. A task that would be a great starting point for a new contributor.

Comments

@arthurprs
Copy link
Contributor

arthurprs commented Mar 27, 2017 •
edited
Loading

I was looking at TechEmpower/FrameworkBenchmarks#2611 the other day and I noticed there's no way of using some Header types without allocating, like the Server header in the linked example.

It's not a problem per se but it's probably worth discussing as setting N headers could potentially allocate N times.
@seanmonstar
Copy link
Member

seanmonstar commented Mar 27, 2017 •
edited
Loading

Agreed. I starting with the most common, Host, but others should follow. Since changing the signature would be a breaking change anyways, it also makes sense to make the fields private and use constructors instead. That way we can start with Cow but we can switch to Bytes or something later in the future and not break. Some common headers that could benefit:

  • Location
  • Origin
  • Referer
  • Server
  • UserAgent

@seanmonstar seanmonstar added E-easy Effort: easy. A task that would be a great starting point for a new contributor. A-headers Area: headers. labels Mar 27, 2017
@mcarlin
Copy link

mcarlin commented Mar 31, 2017

@seanmonstar, I'm looking to start contributing here. Mind if I take a stab at it and convert over those headers you mentioned if you're not already working on it?

@seanmonstar
Copy link
Member

@mcarlin That'd be great! I turned the list into checkboxes, ideally we can keep them up to date.

hannesg pushed a commit to hannesg/hyper that referenced this issue Apr 8, 2017
header: turned several String headers into Cow<'static,str> headers
f8d7162 
This change was suggested in hyperium#1104 . As suggested I've also made the
fields private and added a constructor.
hannesg pushed a commit to hannesg/hyper that referenced this issue Apr 8, 2017
refactor(header): make some headers more allocator friendly and add c…
d9644b4 
…onstructors

Namely:

- Location
- Referrer
- Server
- UserAgent

This change was suggested in [hyperium#1104].

BREAKING CHANGES: Old code that creates the header structs directly will
stop working.
hannesg pushed a commit to hannesg/hyper that referenced this issue Apr 8, 2017
refactor(header): make some headers more allocator friendly
f25db1c 
Change the internal implementation of some simple headers to make them
more allocator friendly. Also add a constructor method to allow changing
the implementation in the future again.

The headers are:

- Location
- Referrer
- Server
- UserAgent

This change was suggested in [hyperium#1104].

BREAKING CHANGES:
- Old code that creates the header structs directly will stop working.
- It's not possible to implement DerefMut for a Cow<'static,str>. Code
that needs to modify header after creation will stop working.
@hannesg hannesg mentioned this issue Apr 8, 2017
Merged
hannesg pushed a commit to hannesg/hyper that referenced this issue Apr 10, 2017
refactor(header): make some headers more allocator friendly
c8d5bf5 
Change the internal implementation of some simple headers to make them
more allocator friendly. Also add a constructor method to allow changing
the implementation in the future again.

The headers are:

- Location
- Referrer
- Server
- UserAgent

This change was suggested in [hyperium#1104].

BREAKING CHANGES:
- Old code that creates the header structs directly will stop working.
- It's not possible to implement DerefMut for a Cow<'static,str>. Code
that needs to modify header after creation will stop working.
@hannesg hannesg mentioned this issue Apr 10, 2017
Merged
1 task
@seanmonstar
Copy link
Member

The list of headers I made have all been converted. If there's changes want to a specific header, lets open specific issues. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-headers Area: headers. E-easy Effort: easy. A task that would be a great starting point for a new contributor.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@seanmonstar @arthurprs @mcarlin