From e1683313ec239a9ff6ebb303f62121e2b6c2b40c Mon Sep 17 00:00:00 2001 From: fakeshadow <24548779@qq.com> Date: Mon, 4 Jan 2021 08:32:41 +0800 Subject: [PATCH] optimize ServiceRequest methods (#1870) Co-authored-by: Rob Ede --- src/app_service.rs | 2 +- src/request.rs | 61 ++++++++++++++++++++++++++-------------------- src/service.rs | 26 +++++++++++--------- 3 files changed, 50 insertions(+), 39 deletions(-) diff --git a/src/app_service.rs b/src/app_service.rs index f02bb831a..2f120cf13 100644 --- a/src/app_service.rs +++ b/src/app_service.rs @@ -238,7 +238,7 @@ where let (head, payload) = req.into_parts(); let req = if let Some(mut req) = self.pool.get_request() { - let inner = Rc::get_mut(&mut req.0).unwrap(); + let inner = Rc::get_mut(&mut req.inner).unwrap(); inner.path.get_mut().update(&head.uri); inner.path.reset(); inner.head = head; diff --git a/src/request.rs b/src/request.rs index 432134cd7..82304c1af 100644 --- a/src/request.rs +++ b/src/request.rs @@ -16,7 +16,12 @@ use crate::rmap::ResourceMap; #[derive(Clone)] /// An HTTP Request -pub struct HttpRequest(pub(crate) Rc); +pub struct HttpRequest { + // *. Rc is used exclusively and NO Weak + // is allowed anywhere in the code. Weak pointer is purposely ignored when + // doing Rc's ref counter check. + pub(crate) inner: Rc, +} pub(crate) struct HttpRequestInner { pub(crate) head: Message, @@ -42,15 +47,17 @@ impl HttpRequest { let mut data = SmallVec::<[Rc; 4]>::new(); data.push(app_data); - HttpRequest(Rc::new(HttpRequestInner { - head, - path, - payload, - rmap, - config, - app_data: data, - pool, - })) + HttpRequest { + inner: Rc::new(HttpRequestInner { + head, + path, + payload, + rmap, + config, + app_data: data, + pool, + }), + } } } @@ -58,14 +65,14 @@ impl HttpRequest { /// This method returns reference to the request head #[inline] pub fn head(&self) -> &RequestHead { - &self.0.head + &self.inner.head } /// This method returns mutable reference to the request head. /// panics if multiple references of http request exists. #[inline] pub(crate) fn head_mut(&mut self) -> &mut RequestHead { - &mut Rc::get_mut(&mut self.0).unwrap().head + &mut Rc::get_mut(&mut self.inner).unwrap().head } /// Request's uri. @@ -118,12 +125,12 @@ impl HttpRequest { /// access the matched value for that segment. #[inline] pub fn match_info(&self) -> &Path { - &self.0.path + &self.inner.path } #[inline] pub(crate) fn match_info_mut(&mut self) -> &mut Path { - &mut Rc::get_mut(&mut self.0).unwrap().path + &mut Rc::get_mut(&mut self.inner).unwrap().path } /// The resource definition pattern that matched the path. Useful for logging and metrics. @@ -134,7 +141,7 @@ impl HttpRequest { /// Returns a None when no resource is fully matched, including default services. #[inline] pub fn match_pattern(&self) -> Option { - self.0.rmap.match_pattern(self.path()) + self.inner.rmap.match_pattern(self.path()) } /// The resource name that matched the path. Useful for logging and metrics. @@ -142,7 +149,7 @@ impl HttpRequest { /// Returns a None when no resource is fully matched, including default services. #[inline] pub fn match_name(&self) -> Option<&str> { - self.0.rmap.match_name(self.path()) + self.inner.rmap.match_name(self.path()) } /// Request extensions @@ -184,7 +191,7 @@ impl HttpRequest { U: IntoIterator, I: AsRef, { - self.0.rmap.url_for(&self, name, elements) + self.inner.rmap.url_for(&self, name, elements) } /// Generate url for named resource @@ -199,7 +206,7 @@ impl HttpRequest { #[inline] /// Get a reference to a `ResourceMap` of current application. pub fn resource_map(&self) -> &ResourceMap { - &self.0.rmap + &self.inner.rmap } /// Peer socket address @@ -225,7 +232,7 @@ impl HttpRequest { /// App config #[inline] pub fn app_config(&self) -> &AppConfig { - &self.0.config + &self.inner.config } /// Get an application data object stored with `App::data` or `App::app_data` @@ -237,7 +244,7 @@ impl HttpRequest { /// let opt_t = req.app_data::>(); /// ``` pub fn app_data(&self) -> Option<&T> { - for container in self.0.app_data.iter().rev() { + for container in self.inner.app_data.iter().rev() { if let Some(data) = container.get::() { return Some(data); } @@ -259,13 +266,13 @@ impl HttpMessage for HttpRequest { /// Request extensions #[inline] fn extensions(&self) -> Ref<'_, Extensions> { - self.0.head.extensions() + self.inner.head.extensions() } /// Mutable reference to a the request's extensions #[inline] fn extensions_mut(&self) -> RefMut<'_, Extensions> { - self.0.head.extensions_mut() + self.inner.head.extensions_mut() } #[inline] @@ -279,7 +286,7 @@ impl Drop for HttpRequest { // if possible, contribute to current worker's HttpRequest allocation pool // This relies on no Weak exists anywhere.(There is none) - if let Some(inner) = Rc::get_mut(&mut self.0) { + if let Some(inner) = Rc::get_mut(&mut self.inner) { let v = &mut inner.pool.0.borrow_mut(); if v.len() < 128 { // clear additional app_data and keep the root one for reuse. @@ -287,7 +294,7 @@ impl Drop for HttpRequest { // inner is borrowed mut here. get head's Extension mutably // to reduce borrow check inner.head.extensions.get_mut().clear(); - v.push(self.0.clone()); + v.push(self.inner.clone()); } } } @@ -329,8 +336,8 @@ impl fmt::Debug for HttpRequest { writeln!( f, "\nHttpRequest {:?} {}:{}", - self.0.head.version, - self.0.head.method, + self.inner.head.version, + self.inner.head.method, self.path() )?; if !self.query_string().is_empty() { @@ -369,7 +376,7 @@ impl HttpRequestPool { /// Re-use a previously allocated (but now completed/discarded) HttpRequest object. #[inline] pub(crate) fn get_request(&self) -> Option { - self.0.borrow_mut().pop().map(HttpRequest) + self.0.borrow_mut().pop().map(|inner| HttpRequest { inner }) } /// Clears all allocated HttpRequest objects. diff --git a/src/service.rs b/src/service.rs index 85bc6123d..e6f71ed06 100644 --- a/src/service.rs +++ b/src/service.rs @@ -62,7 +62,7 @@ impl ServiceRequest { /// Deconstruct request into parts pub fn into_parts(mut self) -> (HttpRequest, Payload) { - let pl = Rc::get_mut(&mut (self.0).0).unwrap().payload.take(); + let pl = Rc::get_mut(&mut (self.0).inner).unwrap().payload.take(); (self.0, pl) } @@ -73,11 +73,12 @@ impl ServiceRequest { mut req: HttpRequest, pl: Payload, ) -> Result { - if Rc::strong_count(&req.0) == 1 && Rc::weak_count(&req.0) == 0 { - Rc::get_mut(&mut req.0).unwrap().payload = pl; - Ok(ServiceRequest(req)) - } else { - Err((req, pl)) + match Rc::get_mut(&mut req.inner) { + Some(p) => { + p.payload = pl; + Ok(ServiceRequest(req)) + } + None => Err((req, pl)), } } @@ -87,7 +88,10 @@ impl ServiceRequest { /// can be re-constructed only if rc's strong pointers count eq 1 and /// weak pointers count is 0. pub fn from_request(req: HttpRequest) -> Result { - if Rc::strong_count(&req.0) == 1 && Rc::weak_count(&req.0) == 0 { + // There is no weak pointer used on HttpRequest so intentionally + // ignore the check. + if Rc::strong_count(&req.inner) == 1 { + debug_assert!(Rc::weak_count(&req.inner) == 0); Ok(ServiceRequest(req)) } else { Err(req) @@ -227,7 +231,7 @@ impl ServiceRequest { /// Counterpart to [`HttpRequest::app_data`](super::HttpRequest::app_data()). pub fn app_data(&self) -> Option<&T> { - for container in (self.0).0.app_data.iter().rev() { + for container in (self.0).inner.app_data.iter().rev() { if let Some(data) = container.get::() { return Some(data); } @@ -238,13 +242,13 @@ impl ServiceRequest { /// Set request payload. pub fn set_payload(&mut self, payload: Payload) { - Rc::get_mut(&mut (self.0).0).unwrap().payload = payload; + Rc::get_mut(&mut (self.0).inner).unwrap().payload = payload; } #[doc(hidden)] /// Add app data container to request's resolution set. pub fn add_data_container(&mut self, extensions: Rc) { - Rc::get_mut(&mut (self.0).0) + Rc::get_mut(&mut (self.0).inner) .unwrap() .app_data .push(extensions); @@ -280,7 +284,7 @@ impl HttpMessage for ServiceRequest { #[inline] fn take_payload(&mut self) -> Payload { - Rc::get_mut(&mut (self.0).0).unwrap().payload.take() + Rc::get_mut(&mut (self.0).inner).unwrap().payload.take() } }