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

refactor(header): make some headers more allocator friendly #1119

Merged
merged 1 commit into from
Apr 10, 2017

Conversation

hannesg
Copy link

@hannesg hannesg commented Apr 8, 2017

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 #1104.

I've left out the Origin header for now because the internal implementation is different.

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.

Copy link
Member

@seanmonstar seanmonstar left a comment

Choose a reason for hiding this comment

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

Hey, this looks spot on! Just one thought regarding the Deref, but we can definitely get this in to the 0.11 release!

}
}
impl ::std::ops::Deref for $id {
type Target = ::std::borrow::Cow<'static,$value>;
Copy link
Member

Choose a reason for hiding this comment

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

So that we don't leak that it is actually a Cow inside, let's just Deref to the $value type instead. This will let us change Cow to a Bytes or something later, and it should all still just work.

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 force-pushed the allocator_friendly_header branch from f25db1c to c8d5bf5 Compare April 10, 2017 08:48
@hannesg
Copy link
Author

hannesg commented Apr 10, 2017

@seanmonstar sure! I've updated that.

@seanmonstar seanmonstar merged commit 52d3ff1 into hyperium:master Apr 10, 2017
@seanmonstar
Copy link
Member

Fantastic, thanks!

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