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(header): Make Uri use MemSlice internally #1054

Merged
merged 1 commit into from Feb 9, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/client/request.rs
Expand Up @@ -7,6 +7,7 @@ use http::{Body, RequestHead};
use method::Method;
use uri::Uri;
use version::HttpVersion;
use std::str::FromStr;

/// A client request to a remote server.
pub struct Request {
Expand Down Expand Up @@ -79,7 +80,7 @@ impl fmt::Debug for Request {
}

pub fn split(req: Request) -> (RequestHead, Option<Body>) {
let uri = Uri::new(&req.url[::url::Position::BeforePath..::url::Position::AfterQuery]).expect("url is uri");
let uri = Uri::from_str(&req.url[::url::Position::BeforePath..::url::Position::AfterQuery]).expect("url is uri");
let head = RequestHead {
subject: ::http::RequestLine(req.method, uri),
headers: req.headers,
Expand Down
16 changes: 15 additions & 1 deletion src/http/buf.rs
Expand Up @@ -4,6 +4,7 @@ use std::fmt;
use std::io::{self, Read};
use std::ops::{Index, Range, RangeFrom, RangeTo, RangeFull};
use std::ptr;
use std::str;
use std::sync::Arc;

pub struct MemBuf {
Expand Down Expand Up @@ -49,8 +50,8 @@ impl MemBuf {
}

pub fn slice(&self, len: usize) -> MemSlice {
assert!(self.end - self.start.get() >= len);
let start = self.start.get();
assert!(!(self.end - start < len));
let end = start + len;
self.start.set(end);
MemSlice {
Expand Down Expand Up @@ -196,6 +197,19 @@ impl From<Vec<u8>> for MemBuf {
}
}

#[derive(Debug, Clone, PartialEq, Eq)]
pub struct MemStr(MemSlice);

impl MemStr {
pub unsafe fn from_utf8_unchecked(slice: MemSlice) -> MemStr {
MemStr(slice)
}

pub fn as_str(&self) -> &str {
unsafe { str::from_utf8_unchecked(self.0.as_ref()) }
}
}

pub struct MemSlice {
buf: Arc<UnsafeCell<Vec<u8>>>,
start: usize,
Expand Down
4 changes: 3 additions & 1 deletion src/http/conn.rs
Expand Up @@ -577,6 +577,8 @@ mod tests {
use super::{Conn, Writing};
use ::uri::Uri;

use std::str::FromStr;

#[test]
fn test_conn_init_read() {
let good_message = b"GET / HTTP/1.1\r\n\r\n".to_vec();
Expand All @@ -587,7 +589,7 @@ mod tests {
match conn.poll().unwrap() {
Async::Ready(Some(Frame::Message { message, body: false })) => {
assert_eq!(message, MessageHead {
subject: ::http::RequestLine(::Get, Uri::new("/").unwrap()),
subject: ::http::RequestLine(::Get, Uri::from_str("/").unwrap()),
.. MessageHead::default()
})
},
Expand Down
23 changes: 15 additions & 8 deletions src/http/h1/parse.rs
Expand Up @@ -6,7 +6,7 @@ use httparse;
use header::{self, Headers, ContentLength, TransferEncoding};
use http::{MessageHead, RawStatus, Http1Transaction, ParseResult, ServerTransaction, ClientTransaction, RequestLine};
use http::h1::{Encoder, Decoder};
use http::buf::{MemBuf, MemSlice};
use http::buf::{MemBuf, MemSlice, MemStr};
use method::Method;
use status::StatusCode;
use version::HttpVersion::{Http10, Http11};
Expand All @@ -33,18 +33,26 @@ impl Http1Transaction for ServerTransaction {
Ok(match try!(req.parse(buf.bytes())) {
httparse::Status::Complete(len) => {
trace!("Request.parse Complete({})", len);
let mut headers = Headers::with_capacity(req.headers.len());
let slice = buf.slice(len);
let path = req.path.unwrap();
let path_start = path.as_ptr() as usize - slice.as_ref().as_ptr() as usize;
let path_end = path_start + path.len();
let path = slice.slice(path_start..path_end);
// path was found to be utf8 by httparse
let path = unsafe { MemStr::from_utf8_unchecked(path) };
let subject = RequestLine(
try!(req.method.unwrap().parse()),
try!(::uri::from_mem_str(path)),
);
let mut headers = Headers::with_capacity(req.headers.len());
headers.extend(HeadersAsMemSliceIter {
headers: req.headers.iter(),
slice: slice,
});

Some((MessageHead {
version: if req.version.unwrap() == 1 { Http11 } else { Http10 },
subject: RequestLine(
try!(req.method.unwrap().parse()),
try!(req.path.unwrap().parse())
),
subject: subject,
headers: headers,
}, len))
}
Expand Down Expand Up @@ -143,7 +151,7 @@ impl Http1Transaction for ClientTransaction {
headers: headers,
}, len))
},
httparse::Status::Partial => None
httparse::Status::Partial => None,
})
}

Expand Down Expand Up @@ -326,5 +334,4 @@ mod tests {
raw.restart();
});
}

}
113 changes: 80 additions & 33 deletions src/uri.rs
@@ -1,6 +1,8 @@
use std::borrow::Cow;
use std::fmt::{Display, self};
use std::str::FromStr;
use std::ops::Deref;
use std::str::{self, FromStr};
use http::buf::MemStr;
use Url;
use url::ParseError as UrlError;

Expand Down Expand Up @@ -30,7 +32,7 @@ use Error;
/// scheme authority path query fragment
#[derive(Clone)]
pub struct Uri {
source: Cow<'static, str>,
source: InternalUri,
scheme_end: Option<usize>,
authority_end: Option<usize>,
query: Option<usize>,
Expand All @@ -39,31 +41,32 @@ pub struct Uri {

impl Uri {
/// Parse a string into a `Uri`.
pub fn new(s: &str) -> Result<Uri, Error> {
let bytes = s.as_bytes();
if bytes.len() == 0 {
fn new(s: InternalUri) -> Result<Uri, Error> {
if s.len() == 0 {
Err(Error::Uri(UrlError::RelativeUrlWithoutBase))
} else if bytes == b"*" {
} else if s.as_bytes() == b"*" {
Ok(Uri {
source: "*".into(),
source: InternalUri::Cow("*".into()),
scheme_end: None,
authority_end: None,
query: None,
fragment: None,
})
} else if bytes == b"/" {
} else if s.as_bytes() == b"/" {
Ok(Uri::default())
} else if bytes.starts_with(b"/") {
} else if s.as_bytes()[0] == b'/' {
let query = parse_query(&s);
let fragment = parse_fragment(&s);
Ok(Uri {
source: s.to_owned().into(),
source: s,
scheme_end: None,
authority_end: None,
query: parse_query(s),
fragment: parse_fragment(s),
query: query,
fragment: fragment,
})
} else if s.contains("://") {
let scheme = parse_scheme(s);
let auth = parse_authority(s);
let scheme = parse_scheme(&s);
let auth = parse_authority(&s);
if let Some(end) = scheme {
match &s[..end] {
"ftp" | "gopher" | "http" | "https" | "ws" | "wss" => {},
Expand All @@ -79,20 +82,23 @@ impl Uri {
None => return Err(Error::Method),
}
}
let query = parse_query(&s);
let fragment = parse_fragment(&s);
Ok(Uri {
source: s.to_owned().into(),
source: s,
scheme_end: scheme,
authority_end: auth,
query: parse_query(s),
fragment: parse_fragment(s),
query: query,
fragment: fragment,
})
} else if (s.contains("/") || s.contains("?")) && !s.contains("://") {
return Err(Error::Method)
} else {
let len = s.len();
Ok(Uri {
source: s.to_owned().into(),
source: s,
scheme_end: None,
authority_end: Some(s.len()),
authority_end: Some(len),
query: None,
fragment: None,
})
Expand All @@ -108,9 +114,10 @@ impl Uri {
if fragment_len > 0 { fragment_len + 1 } else { 0 };
if index >= end {
if self.scheme().is_some() {
return "/" // absolute-form MUST have path
"/" // absolute-form MUST have path
} else {
""
}
""
} else {
&self.source[index..end]
}
Expand Down Expand Up @@ -158,7 +165,8 @@ impl Uri {
let fragment_len = self.fragment.unwrap_or(0);
let fragment_len = if fragment_len > 0 { fragment_len + 1 } else { 0 };
if let Some(len) = self.query {
Some(&self.source[self.source.len() - len - fragment_len..self.source.len() - fragment_len])
Some(&self.source[self.source.len() - len - fragment_len..
self.source.len() - fragment_len])
} else {
None
}
Expand All @@ -167,7 +175,7 @@ impl Uri {
#[cfg(test)]
fn fragment(&self) -> Option<&str> {
if let Some(len) = self.fragment {
Some(&self.source[self.source.len() - len..])
Some(&self.source[self.source.len() - len..self.source.len()])
} else {
None
}
Expand All @@ -180,7 +188,7 @@ fn parse_scheme(s: &str) -> Option<usize> {

fn parse_authority(s: &str) -> Option<usize> {
let i = s.find("://").and_then(|p| Some(p + 3)).unwrap_or(0);

Some(&s[i..].split("/")
.next()
.unwrap_or(s)
Expand Down Expand Up @@ -209,32 +217,43 @@ impl FromStr for Uri {
type Err = Error;

fn from_str(s: &str) -> Result<Uri, Error> {
Uri::new(s)
//TODO: refactor such that the to_owned() is only required at the end
//of successful parsing, so an Err doesn't needlessly clone the string.
Uri::new(InternalUri::Cow(Cow::Owned(s.to_owned())))
}
}

impl From<Url> for Uri {
fn from(url: Url) -> Uri {
Uri::new(url.as_str()).expect("Uri::From<Url> failed")
let s = InternalUri::Cow(Cow::Owned(url.into_string()));
// TODO: we could build the indices with some simple subtraction,
// instead of re-parsing an already parsed string.
// For example:
// let scheme_end = url[..url::Position::AfterScheme].as_ptr() as usize
// - url.as_str().as_ptr() as usize;
// etc
Uri::new(s).expect("Uri::From<Url> failed")
}
}

impl PartialEq for Uri {
fn eq(&self, other: &Uri) -> bool {
self.source == other.source
self.source.as_str() == other.source.as_str()
}
}

impl Eq for Uri {}

impl AsRef<str> for Uri {
fn as_ref(&self) -> &str {
&self.source
self.source.as_str()
}
}

impl Default for Uri {
fn default() -> Uri {
Uri {
source: "/".into(),
source: InternalUri::Cow("/".into()),
scheme_end: None,
authority_end: None,
query: None,
Expand All @@ -245,16 +264,44 @@ impl Default for Uri {

impl fmt::Debug for Uri {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
fmt::Debug::fmt(&self.source.as_ref(), f)
fmt::Debug::fmt(self.as_ref(), f)
}
}

impl Display for Uri {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
f.write_str(&self.source)
f.write_str(self.as_ref())
}
}

pub fn from_mem_str(s: MemStr) -> Result<Uri, Error> {
Uri::new(InternalUri::Shared(s))
}

#[derive(Clone)]
enum InternalUri {
Cow(Cow<'static, str>),
Shared(MemStr),
}

impl InternalUri {
fn as_str(&self) -> &str {
match *self {
InternalUri::Cow(ref s) => s.as_ref(),
InternalUri::Shared(ref m) => m.as_str(),
}
}
}

impl Deref for InternalUri {
type Target = str;

fn deref(&self) -> &str {
self.as_str()
}
}


macro_rules! test_parse {
(
$test_name:ident,
Expand All @@ -263,7 +310,7 @@ macro_rules! test_parse {
) => (
#[test]
fn $test_name() {
let uri = Uri::new($str).unwrap();
let uri = Uri::from_str($str).unwrap();
$(
assert_eq!(uri.$method(), $value);
)+
Expand Down Expand Up @@ -368,7 +415,7 @@ test_parse! {
#[test]
fn test_uri_parse_error() {
fn err(s: &str) {
Uri::new(s).unwrap_err();
Uri::from_str(s).unwrap_err();
}

err("http://");
Expand Down