From e3dc6f0ca80d91038e7089bd78f447ba11d2982e Mon Sep 17 00:00:00 2001
From: Nikolay Kim <fafhrd91@gmail.com>
Date: Sat, 23 Jun 2018 12:28:55 +0600
Subject: [PATCH] refactor h1decoder

---
 src/client/parser.rs    | 37 +++++++++++++++--------------
 src/server/h1decoder.rs | 52 ++++++++++++++++++++++++++++++-----------
 2 files changed, 57 insertions(+), 32 deletions(-)

diff --git a/src/client/parser.rs b/src/client/parser.rs
index 4668f58a..f5390cc3 100644
--- a/src/client/parser.rs
+++ b/src/client/parser.rs
@@ -1,13 +1,14 @@
+use std::mem;
+
 use bytes::{Bytes, BytesMut};
 use futures::{Async, Poll};
 use http::header::{self, HeaderName, HeaderValue};
-use http::{HeaderMap, HttpTryFrom, StatusCode, Version};
+use http::{HeaderMap, StatusCode, Version};
 use httparse;
-use std::mem;
 
 use error::{ParseError, PayloadError};
 
-use server::h1decoder::EncodingDecoder;
+use server::h1decoder::{EncodingDecoder, HeaderIndex};
 use server::IoStream;
 
 use super::response::ClientMessage;
@@ -117,24 +118,23 @@ impl HttpResponseParser {
     fn parse_message(
         buf: &mut BytesMut,
     ) -> Poll<(ClientResponse, Option<EncodingDecoder>), ParseError> {
-        // Parse http message
-        let bytes_ptr = buf.as_ref().as_ptr() as usize;
-        let mut headers: [httparse::Header; MAX_HEADERS] =
-            unsafe { mem::uninitialized() };
+        // Unsafe: we read only this data only after httparse parses headers into.
+        // performance bump for pipeline benchmarks.
+        let mut headers: [HeaderIndex; MAX_HEADERS] = unsafe { mem::uninitialized() };
 
         let (len, version, status, headers_len) = {
-            let b = unsafe {
-                let b: &[u8] = buf;
-                &*(b as *const _)
-            };
-            let mut resp = httparse::Response::new(&mut headers);
-            match resp.parse(b)? {
+            let mut parsed: [httparse::Header; MAX_HEADERS] =
+                unsafe { mem::uninitialized() };
+
+            let mut resp = httparse::Response::new(&mut parsed);
+            match resp.parse(buf)? {
                 httparse::Status::Complete(len) => {
                     let version = if resp.version.unwrap_or(1) == 1 {
                         Version::HTTP_11
                     } else {
                         Version::HTTP_10
                     };
+                    HeaderIndex::record(buf, resp.headers, &mut headers);
                     let status = StatusCode::from_u16(resp.code.unwrap())
                         .map_err(|_| ParseError::Status)?;
 
@@ -148,12 +148,13 @@ impl HttpResponseParser {
 
         // convert headers
         let mut hdrs = HeaderMap::new();
-        for header in headers[..headers_len].iter() {
-            if let Ok(name) = HeaderName::try_from(header.name) {
-                let v_start = header.value.as_ptr() as usize - bytes_ptr;
-                let v_end = v_start + header.value.len();
+        for idx in headers[..headers_len].iter() {
+            if let Ok(name) = HeaderName::from_bytes(&slice[idx.name.0..idx.name.1]) {
+                // Unsafe: httparse check header value for valid utf-8
                 let value = unsafe {
-                    HeaderValue::from_shared_unchecked(slice.slice(v_start, v_end))
+                    HeaderValue::from_shared_unchecked(
+                        slice.slice(idx.value.0, idx.value.1),
+                    )
                 };
                 hdrs.append(name, value);
             } else {
diff --git a/src/server/h1decoder.rs b/src/server/h1decoder.rs
index 77da36af..9815b936 100644
--- a/src/server/h1decoder.rs
+++ b/src/server/h1decoder.rs
@@ -91,17 +91,17 @@ impl H1Decoder {
         let mut content_length = None;
 
         let msg = {
-            let bytes_ptr = buf.as_ref().as_ptr() as usize;
-            let mut headers: [httparse::Header; MAX_HEADERS] =
+            // Unsafe: we read only this data only after httparse parses headers into.
+            // performance bump for pipeline benchmarks.
+            let mut headers: [HeaderIndex; MAX_HEADERS] =
                 unsafe { mem::uninitialized() };
 
             let (len, method, path, version, headers_len) = {
-                let b = unsafe {
-                    let b: &[u8] = buf;
-                    &*(b as *const [u8])
-                };
-                let mut req = httparse::Request::new(&mut headers);
-                match req.parse(b)? {
+                let mut parsed: [httparse::Header; MAX_HEADERS] =
+                    unsafe { mem::uninitialized() };
+
+                let mut req = httparse::Request::new(&mut parsed);
+                match req.parse(buf)? {
                     httparse::Status::Complete(len) => {
                         let method = Method::from_bytes(req.method.unwrap().as_bytes())
                             .map_err(|_| ParseError::Method)?;
@@ -111,6 +111,8 @@ impl H1Decoder {
                         } else {
                             Version::HTTP_10
                         };
+                        HeaderIndex::record(buf, req.headers, &mut headers);
+
                         (len, method, path, version, req.headers.len())
                     }
                     httparse::Status::Partial => return Ok(Async::NotReady),
@@ -127,15 +129,15 @@ impl H1Decoder {
                     .flags
                     .set(MessageFlags::KEEPALIVE, version != Version::HTTP_10);
 
-                for header in headers[..headers_len].iter() {
-                    if let Ok(name) = HeaderName::from_bytes(header.name.as_bytes()) {
+                for idx in headers[..headers_len].iter() {
+                    if let Ok(name) =
+                        HeaderName::from_bytes(&slice[idx.name.0..idx.name.1])
+                    {
                         has_upgrade = has_upgrade || name == header::UPGRADE;
-
-                        let v_start = header.value.as_ptr() as usize - bytes_ptr;
-                        let v_end = v_start + header.value.len();
+                        // Unsafe: httparse check header value for valid utf-8
                         let value = unsafe {
                             HeaderValue::from_shared_unchecked(
-                                slice.slice(v_start, v_end),
+                                slice.slice(idx.value.0, idx.value.1),
                             )
                         };
                         match name {
@@ -211,6 +213,28 @@ impl H1Decoder {
     }
 }
 
+#[derive(Clone, Copy)]
+pub(crate) struct HeaderIndex {
+    pub(crate) name: (usize, usize),
+    pub(crate) value: (usize, usize),
+}
+
+impl HeaderIndex {
+    pub(crate) fn record(
+        bytes: &[u8], headers: &[httparse::Header], indices: &mut [HeaderIndex],
+    ) {
+        let bytes_ptr = bytes.as_ptr() as usize;
+        for (header, indices) in headers.iter().zip(indices.iter_mut()) {
+            let name_start = header.name.as_ptr() as usize - bytes_ptr;
+            let name_end = name_start + header.name.len();
+            indices.name = (name_start, name_end);
+            let value_start = header.value.as_ptr() as usize - bytes_ptr;
+            let value_end = value_start + header.value.len();
+            indices.value = (value_start, value_end);
+        }
+    }
+}
+
 /// Decoders to handle different Transfer-Encodings.
 ///
 /// If a message body does not include a Transfer-Encoding, it *should*