Optimize MediaType::Display, ContentType::Into<Header>, and precheck.

Differential and causal profiling determined that 35% of `Hello, world!`
dispatch time was spent rendering `Content-Type` due to many calls to `fmt` in
`MediaType::Display` and an allocation in `ContentType::Into<Header>`. This
change reduces the number of calls to `fmt` to 1 in `MediaType::Display` and
removes the allocation in `Into<Header>` for known media types.

This change also caches a `Rocket` "precheck" so that pre-dispatch checks are
done only a single time for a given `Rocket` instance, further reducing
`MockRequest::dispatch_with` time for "Hello, world!" by roughly 15%.
This commit is contained in:
Sergio Benitez 2017-05-23 16:41:38 -07:00
parent d93d366ce7
commit 299a422cbc
9 changed files with 76 additions and 44 deletions

View File

@ -26,7 +26,7 @@ fn media_type_to_expr(ecx: &ExtCtxt, ct: Option<MediaType>) -> Option<P<Expr>> {
ct.map(|ct| {
let (top, sub) = (ct.top().as_str(), ct.sub().as_str());
quote_expr!(ecx, ::rocket::http::MediaType {
source: None,
source: ::rocket::http::Source::None,
top: ::rocket::http::IndexedStr::Concrete(
::std::borrow::Cow::Borrowed($top)
),

View File

@ -95,6 +95,7 @@ impl Accept {
// self.0.push(media_type.into());
// }
/// TODO: Cache this?
pub fn preferred(&self) -> &WeightedMediaType {
static ANY: WeightedMediaType = WeightedMediaType(MediaType::Any, None);
@ -126,8 +127,6 @@ impl Accept {
preferred
}
// */html, text/plain
#[inline(always)]
pub fn first(&self) -> Option<&WeightedMediaType> {
self.iter().next()

View File

@ -4,7 +4,7 @@ use std::str::FromStr;
use std::fmt;
use ext::IntoCollection;
use http::{Header, MediaType};
use http::{Header, MediaType, Source};
use http::hyper::mime::Mime;
/// Representation of HTTP Content-Types.
@ -285,6 +285,10 @@ impl Into<Header<'static>> for ContentType {
//
// We could also use an `enum` for MediaType. But that kinda sucks. But
// maybe it's what we want.
Header::new("Content-Type", self.to_string())
if let Source::Known(src) = self.0.source {
Header::new("Content-Type", src)
} else {
Header::new("Content-Type", self.to_string())
}
}
}

View File

@ -1,4 +1,4 @@
use std::borrow::Cow;
use std::borrow::{Cow, Borrow};
use std::str::FromStr;
use std::fmt;
use std::hash::{Hash, Hasher};
@ -22,15 +22,31 @@ pub enum MediaParams {
Dynamic(SmallVec<[(IndexedStr, IndexedStr); 2]>)
}
#[derive(Debug, Clone, PartialEq, Eq)]
pub enum Source {
Known(&'static str),
Custom(Cow<'static, str>),
None
}
impl Source {
#[inline]
fn as_str(&self) -> Option<&str> {
match *self {
Source::Known(s) => Some(s),
Source::Custom(ref s) => Some(s.borrow()),
Source::None => None
}
}
}
// Describe a media type. In particular, describe its comparison and hashing
// semantics.
#[derive(Debug, Clone)]
pub struct MediaType {
/// Storage for the entire media type string. This will be `Some` when the
/// media type was parsed from a string and `None` when it was created
/// manually.
/// Storage for the entire media type string.
#[doc(hidden)]
pub source: Option<Cow<'static, str>>,
pub source: Source,
/// The top-level type.
#[doc(hidden)]
pub top: IndexedStr,
@ -56,7 +72,7 @@ macro_rules! media_types {
#[doc="</i>"]
#[allow(non_upper_case_globals)]
pub const $name: MediaType = MediaType {
source: None,
source: Source::Known(concat!($t, "/", $s, $("; ", $k, "=", $v),*)),
top: media_str!($t),
sub: media_str!($s),
params: MediaParams::Static(&[$((media_str!($k), media_str!($v))),*])
@ -139,7 +155,7 @@ impl MediaType {
where T: Into<Cow<'static, str>>, S: Into<Cow<'static, str>>
{
MediaType {
source: None,
source: Source::None,
top: IndexedStr::Concrete(top.into()),
sub: IndexedStr::Concrete(sub.into()),
params: MediaParams::Static(&[]),
@ -183,7 +199,7 @@ impl MediaType {
MediaType {
source: None,
source: Source::None,
top: IndexedStr::Concrete(top.into()),
sub: IndexedStr::Concrete(sub.into()),
params: MediaParams::Dynamic(params)
@ -207,7 +223,7 @@ impl MediaType {
/// ```
#[inline]
pub fn top(&self) -> &UncasedStr {
self.top.to_str(self.source.as_ref()).into()
self.top.to_str(self.source.as_str()).into()
}
/// Returns the subtype for this media type. The return type,
@ -225,7 +241,7 @@ impl MediaType {
/// ```
#[inline]
pub fn sub(&self) -> &UncasedStr {
self.sub.to_str(self.source.as_ref()).into()
self.sub.to_str(self.source.as_str()).into()
}
/// Returns a `u8` representing how specific the top-level type and subtype
@ -333,7 +349,7 @@ impl MediaType {
param_slice.iter()
.map(move |&(ref key, ref val)| {
let source_str = self.source.as_ref();
let source_str = self.source.as_str();
(key.to_str(source_str), val.to_str(source_str))
})
}
@ -374,11 +390,15 @@ impl Hash for MediaType {
impl fmt::Display for MediaType {
#[inline]
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
write!(f, "{}/{}", self.top(), self.sub())?;
for (key, val) in self.params() {
write!(f, "; {}={}", key, val)?;
}
if let Source::Known(src) = self.source {
src.fmt(f)
} else {
write!(f, "{}/{}", self.top(), self.sub())?;
for (key, val) in self.params() {
write!(f, "; {}={}", key, val)?;
}
Ok(())
Ok(())
}
}
}

View File

@ -26,7 +26,7 @@ pub(crate) mod parse;
// TODO: Expose a `const fn` from ContentType when possible. (see RFC#1817)
pub mod uncased;
#[doc(hidden)] pub use self::parse::IndexedStr;
#[doc(hidden)] pub use self::media_type::MediaParams;
#[doc(hidden)] pub use self::media_type::{MediaParams, Source};
pub use self::method::Method;
pub use self::content_type::ContentType;

View File

@ -38,13 +38,6 @@ mod test {
use http::MediaType;
use super::parse_accept;
macro_rules! assert_no_parse {
($string:expr) => ({
let result: Result<_, _> = parse_accept($string).into();
if result.is_ok() { panic!("{:?} parsed unexpectedly.", $string) }
});
}
macro_rules! assert_parse {
($string:expr) => ({
match parse_accept($string) {
@ -64,16 +57,6 @@ mod test {
});
}
macro_rules! assert_quality_eq {
($string:expr, [$($mt:expr),*]) => ({
let expected = vec![$($mt),*];
let result = assert_parse!($string);
for (i, wmt) in result.iter().enumerate() {
assert_eq!(wmt.media_type(), &expected[i]);
}
});
}
#[test]
fn check_does_parse() {
assert_parse!("text/html");

View File

@ -24,7 +24,7 @@ impl IndexedStr {
/// # Panics
///
/// Panics if `self` is an indexed string and `string` is None.
pub fn to_str<'a>(&'a self, string: Option<&'a Cow<str>>) -> &'a str {
pub fn to_str<'a>(&'a self, string: Option<&'a str>) -> &'a str {
if self.is_indexed() && string.is_none() {
panic!("Cannot convert indexed str to str without base string!")
}

View File

@ -5,7 +5,7 @@ use pear::parsers::*;
use pear::combinators::*;
use smallvec::SmallVec;
use http::{MediaType, MediaParams};
use http::{MediaType, Source, MediaParams};
use http::parse::checkers::{is_whitespace, is_valid_token};
use http::parse::IndexedStr;
@ -51,7 +51,7 @@ pub fn media_type<'a>(input: &mut &'a str) -> ParseResult<&'a str, MediaType> {
}
MediaType {
source: Some(Cow::Owned(source.to_string())),
source: Source::Custom(Cow::Owned(source.to_string())),
top: IndexedStr::from(top, source).expect("top in source"),
sub: IndexedStr::from(sub, source).expect("sub in source"),
params: MediaParams::Dynamic(params)

View File

@ -76,12 +76,14 @@
//! ```
use ::{Rocket, Request, Response, Data};
use error::LaunchError;
use http::{Method, Status, Header, Cookie};
use std::net::SocketAddr;
/// A type for mocking requests for testing Rocket applications.
pub struct MockRequest<'r> {
prechecked: Option<&'r Rocket>,
request: Request<'r>,
data: Data
}
@ -91,6 +93,7 @@ impl<'r> MockRequest<'r> {
#[inline]
pub fn new<S: AsRef<str>>(method: Method, uri: S) -> Self {
MockRequest {
prechecked: None,
request: Request::new(method, uri.as_ref().to_string()),
data: Data::local(vec![])
}
@ -199,6 +202,28 @@ impl<'r> MockRequest<'r> {
self
}
/// Returns `Some` if there an error checking `rocket`. Returns `None` if
/// there's no error and dispatching can continue.
fn precheck(&mut self, rocket: &'r Rocket) -> Option<LaunchError> {
// Check if we've already prechecked some `Rocket` instance.
if let Some(r) = self.prechecked {
// Check if the one we've prechecked is indeed `rocket`. This does a
// straight pointer comparison. If they're the same, then we know
// that the instance must not have changed since we kept an
// immutable borrow to it from the precheck.
if (r as *const Rocket) == (rocket as *const Rocket) {
return None
}
}
if let Some(err) = rocket.prelaunch_check() {
return Some(err);
}
self.prechecked = Some(rocket);
None
}
/// Dispatch this request using a given instance of Rocket.
///
/// It is possible that the supplied `rocket` instance contains malformed
@ -234,11 +259,12 @@ impl<'r> MockRequest<'r> {
/// assert_eq!(response.body_string(), Some("Hello, world!".into()));
/// # }
/// ```
#[inline]
pub fn dispatch_with<'s>(&'s mut self, rocket: &'r Rocket) -> Response<'s> {
if let Some(error) = rocket.prelaunch_check() {
if let Some(err) = self.precheck(rocket) {
return Response::build()
.status(Status::InternalServerError)
.sized_body(::std::io::Cursor::new(error.to_string()))
.sized_body(::std::io::Cursor::new(err.to_string()))
.finalize()
}