Merge pull request #3107 from spinframework/less-allocations-in-router

Less allocations in router

Author: rylev

crates/http/src/routes.rs

@@ -4,7 +4,7 @@
 
 use anyhow::{anyhow, Result};
 use indexmap::IndexMap;
-use std::{collections::HashMap, fmt};
+use std::{borrow::Cow, collections::HashMap, fmt};
 
 use crate::config::HttpTriggerRouteConfig;
 
@@ -22,9 +22,9 @@ struct RouteHandler {
     /// The component ID that the route maps to.
     component_id: String,
     /// The route, including any application base.
-    based_route: String,
+    based_route: Cow<'static, str>,
     /// The route, not including any application base.
-    raw_route: String,
+    raw_route: Cow<'static, str>,
     /// The route, including any application base and capturing information about whether it has a trailing wildcard.
     /// (This avoids re-parsing the route string.)
     parsed_based_route: ParsedRoute,
@@ -111,8 +111,8 @@ impl Router {
 
             let handler = RouteHandler {
                 component_id: re.component_id.to_string(),
-                based_route: re.based_route,
-                raw_route: re.raw_route.to_string(),
+                based_route: re.based_route.into(),
+                raw_route: re.raw_route.to_string().into(),
                 parsed_based_route: parsed,
             };
 
@@ -163,37 +163,24 @@ impl Router {
     /// If multiple components could potentially handle the same request based on their
     /// defined routes, components with matching exact routes take precedence followed
     /// by matching wildcard patterns with the longest matching prefix.
-    pub fn route(&self, p: &str) -> Result<RouteMatch> {
+    pub fn route<'path, 'router: 'path>(
+        &'router self,
+        path: &'path str,
+    ) -> Result<RouteMatch<'router, 'path>> {
         let best_match = self
             .router
-            .best_match(p)
-            .ok_or_else(|| anyhow!("Cannot match route for path {p}"))?;
+            .best_match(path)
+            .ok_or_else(|| anyhow!("Cannot match route for path {path}"))?;
 
-        let route_handler = best_match.handler().clone();
-        let named_wildcards = best_match
-            .captures()
-            .iter()
-            .map(|(k, v)| (k.to_owned(), v.to_owned()))
-            .collect();
-        let trailing_wildcard = best_match.captures().wildcard().map(|s|
-            // Backward compatibility considerations - Spin has traditionally
-            // captured trailing slashes, but routefinder does not.
-            match (s.is_empty(), p.ends_with('/')) {
-                // route: /foo/..., path: /foo
-                (true, false) => s.to_owned(),
-                // route: /foo/..., path: /foo/
-                (true, true) => "/".to_owned(),
-                // route: /foo/..., path: /foo/bar
-                (false, false) => format!("/{s}"),
-                // route: /foo/..., path: /foo/bar/
-                (false, true) => format!("/{s}/"),
-            }
-        );
+        let route_handler = best_match.handler();
+        let captures = best_match.captures();
 
         Ok(RouteMatch {
-            route_handler,
-            named_wildcards,
-            trailing_wildcard,
+            inner: RouteMatchKind::Real {
+                route_handler,
+                captures,
+                path,
+            },
         })
     }
 }
@@ -235,69 +222,136 @@ impl fmt::Display for ParsedRoute {
 }
 
 /// A routing match for a URL.
-pub struct RouteMatch {
-    route_handler: RouteHandler,
-    named_wildcards: HashMap<String, String>,
-    trailing_wildcard: Option<String>,
+pub struct RouteMatch<'router, 'path> {
+    inner: RouteMatchKind<'router, 'path>,
 }
 
-impl RouteMatch {
+impl RouteMatch<'_, '_> {
     /// A synthetic match as if the given path was matched against the wildcard route.
     /// Used in service chaining.
-    pub fn synthetic(component_id: &str, path: &str) -> Self {
+    pub fn synthetic(component_id: String, path: String) -> Self {
         Self {
-            route_handler: RouteHandler {
-                component_id: component_id.to_string(),
-                based_route: "/...".to_string(),
-                raw_route: "/...".to_string(),
-                parsed_based_route: ParsedRoute::TrailingWildcard(String::new()),
+            inner: RouteMatchKind::Synthetic {
+                route_handler: RouteHandler {
+                    component_id,
+                    based_route: "/...".into(),
+                    raw_route: "/...".into(),
+                    parsed_based_route: ParsedRoute::TrailingWildcard(String::new()),
+                },
+                trailing_wildcard: path,
             },
-            named_wildcards: Default::default(),
-            trailing_wildcard: Some(path.to_string()),
         }
     }
 
     /// The matched component.
     pub fn component_id(&self) -> &str {
-        &self.route_handler.component_id
+        &self.inner.route_handler().component_id
     }
 
     /// The matched route, as originally written in the manifest, combined with the base.
     pub fn based_route(&self) -> &str {
-        &self.route_handler.based_route
+        &self.inner.route_handler().based_route
     }
 
     /// The matched route, excluding any trailing wildcard, combined with the base.
-    pub fn based_route_or_prefix(&self) -> String {
-        self.route_handler
+    pub fn based_route_or_prefix(&self) -> &str {
+        self.inner
+            .route_handler()
             .based_route
             .strip_suffix("/...")
-            .unwrap_or(&self.route_handler.based_route)
-            .to_string()
+            .unwrap_or(&self.inner.route_handler().based_route)
     }
 
     /// The matched route, as originally written in the manifest.
     pub fn raw_route(&self) -> &str {
-        &self.route_handler.raw_route
+        &self.inner.route_handler().raw_route
     }
 
     /// The matched route, excluding any trailing wildcard.
-    pub fn raw_route_or_prefix(&self) -> String {
-        self.route_handler
+    pub fn raw_route_or_prefix(&self) -> &str {
+        self.inner
+            .route_handler()
             .raw_route
             .strip_suffix("/...")
-            .unwrap_or(&self.route_handler.raw_route)
-            .to_string()
+            .unwrap_or(&self.inner.route_handler().raw_route)
+    }
+
+    /// The named wildcards captured from the path, if any
+    pub fn named_wildcards(&self) -> HashMap<&str, &str> {
+        self.inner.named_wildcards()
+    }
+
+    /// The trailing wildcard part of the path, if any
+    pub fn trailing_wildcard(&self) -> Cow<'_, str> {
+        self.inner.trailing_wildcard()
+    }
+}
+
+/// The kind of route match that was made.
+///
+/// Can either be real based on the routefinder or synthetic based on hardcoded results.
+enum RouteMatchKind<'router, 'path> {
+    /// A synthetic match as if the given path was matched against the wildcard route.
+    Synthetic {
+        /// The route handler that matched the path.
+        route_handler: RouteHandler,
+        /// The trailing wildcard part of the path
+        trailing_wildcard: String,
+    },
+    /// A real match.
+    Real {
+        /// The route handler that matched the path.
+        route_handler: &'router RouteHandler,
+        /// The best match for the path.
+        captures: routefinder::Captures<'router, 'path>,
+        /// The path that was matched.
+        path: &'path str,
+    },
+}
+
+impl RouteMatchKind<'_, '_> {
+    /// The route handler that matched the path.
+    fn route_handler(&self) -> &RouteHandler {
+        match self {
+            RouteMatchKind::Synthetic { route_handler, .. } => route_handler,
+            RouteMatchKind::Real { route_handler, .. } => route_handler,
+        }
     }
 
     /// The named wildcards captured from the path, if any
-    pub fn named_wildcards(&self) -> &HashMap<String, String> {
-        &self.named_wildcards
+    pub fn named_wildcards(&self) -> HashMap<&str, &str> {
+        let Self::Real { captures, .. } = &self else {
+            return HashMap::new();
+        };
+        captures.iter().collect()
     }
 
     /// The trailing wildcard part of the path, if any
-    pub fn trailing_wildcard(&self) -> String {
-        self.trailing_wildcard.clone().unwrap_or_default()
+    pub fn trailing_wildcard(&self) -> Cow<'_, str> {
+        let (captures, path) = match self {
+            // If we have a synthetic match, we already have the trailing wildcard.
+            Self::Synthetic {
+                trailing_wildcard, ..
+            } => return trailing_wildcard.into(),
+            Self::Real { captures, path, .. } => (captures, path),
+        };
+
+        captures
+            .wildcard()
+            .map(|s|
+            // Backward compatibility considerations - Spin has traditionally
+            // captured trailing slashes, but routefinder does not.
+            match (s.is_empty(), path.ends_with('/')) {
+                // route: /foo/..., path: /foo
+                (true, false) => s.into(),
+                // route: /foo/..., path: /foo/
+                (true, true) => "/".into(),
+                // route: /foo/..., path: /foo/bar
+                (false, false) => format!("/{s}").into(),
+                // route: /foo/..., path: /foo/bar/
+                (false, true) => format!("/{s}/").into(),
+            })
+            .unwrap_or_default()
     }
 }
 

crates/http/src/wagi/mod.rs

@@ -105,7 +105,7 @@ pub fn build_headers(
     // https://datatracker.ietf.org/doc/html/rfc3875#section-4.1.13
     headers.insert(
         "SCRIPT_NAME".to_owned(),
-        route_match.based_route_or_prefix(),
+        route_match.based_route_or_prefix().to_owned(),
     );
     // PATH_INFO is any path information after SCRIPT_NAME
     //
@@ -117,7 +117,10 @@ pub fn build_headers(
     // https://datatracker.ietf.org/doc/html/rfc3875#section-4.1.5
     let pathsegment = path_info;
     let pathinfo = percent_encoding::percent_decode_str(&pathsegment).decode_utf8_lossy();
-    headers.insert("X_RAW_PATH_INFO".to_owned(), pathsegment.clone());
+    headers.insert(
+        "X_RAW_PATH_INFO".to_owned(),
+        pathsegment.as_ref().to_owned(),
+    );
     headers.insert("PATH_INFO".to_owned(), pathinfo.to_string());
     // PATH_TRANSLATED is the url-decoded version of PATH_INFO
     // https://datatracker.ietf.org/doc/html/rfc3875#section-4.1.6

crates/oci/src/client.rs

@@ -801,8 +801,8 @@ pub async fn unpack_archive_layer(
 fn digest_from_url(manifest_url: &str) -> Option<String> {
     // The URL is in the form "https://host/v2/refname/manifests/sha256:..."
     let manifest_url = Url::parse(manifest_url).ok()?;
-    let segments = manifest_url.path_segments()?;
-    let last = segments.last()?;
+    let mut segments = manifest_url.path_segments()?;
+    let last = segments.next_back()?;
     if last.contains(':') {
         Some(last.to_owned())
     } else {

crates/trigger-http/src/headers.rs

@@ -1,4 +1,8 @@
-use std::{net::SocketAddr, str, str::FromStr};
+use std::{
+    borrow::Cow,
+    net::SocketAddr,
+    str::{self, FromStr},
+};
 
 use anyhow::Result;
 use http::Uri;
@@ -21,23 +25,27 @@ pub const RAW_COMPONENT_ROUTE: [&str; 2] = ["SPIN_RAW_COMPONENT_ROUTE", "X_RAW_C
 pub const BASE_PATH: [&str; 2] = ["SPIN_BASE_PATH", "X_BASE_PATH"];
 pub const CLIENT_ADDR: [&str; 2] = ["SPIN_CLIENT_ADDR", "X_CLIENT_ADDR"];
 
-pub fn compute_default_headers(
+// Header key/value pairs that use copy on write to avoid allocation
+pub type HeaderPair<'a> = ([Cow<'static, str>; 2], Cow<'a, str>);
+
+/// Compute the default headers to be passed to the component.
+pub fn compute_default_headers<'a>(
     uri: &Uri,
     host: &str,
-    route_match: &RouteMatch,
+    route_match: &'a RouteMatch,
     client_addr: SocketAddr,
-) -> anyhow::Result<Vec<([String; 2], String)>> {
-    fn owned(strs: &[&'static str; 2]) -> [String; 2] {
-        [strs[0].to_owned(), strs[1].to_owned()]
+) -> anyhow::Result<Vec<HeaderPair<'a>>> {
+    fn owned(strs: &[&'static str; 2]) -> [Cow<'static, str>; 2] {
+        [strs[0].into(), strs[1].into()]
     }
 
-    let owned_full_url: [String; 2] = owned(&FULL_URL);
-    let owned_path_info: [String; 2] = owned(&PATH_INFO);
-    let owned_matched_route: [String; 2] = owned(&MATCHED_ROUTE);
-    let owned_component_route: [String; 2] = owned(&COMPONENT_ROUTE);
-    let owned_raw_component_route: [String; 2] = owned(&RAW_COMPONENT_ROUTE);
-    let owned_base_path: [String; 2] = owned(&BASE_PATH);
-    let owned_client_addr: [String; 2] = owned(&CLIENT_ADDR);
+    let owned_full_url = owned(&FULL_URL);
+    let owned_path_info = owned(&PATH_INFO);
+    let owned_matched_route = owned(&MATCHED_ROUTE);
+    let owned_component_route = owned(&COMPONENT_ROUTE);
+    let owned_raw_component_route = owned(&RAW_COMPONENT_ROUTE);
+    let owned_base_path = owned(&BASE_PATH);
+    let owned_client_addr = owned(&CLIENT_ADDR);
 
     let mut res = vec![];
     let abs_path = uri
@@ -52,21 +60,21 @@ pub fn compute_default_headers(
     let full_url = format!("{}://{}{}", scheme, host, abs_path);
 
     res.push((owned_path_info, path_info));
-    res.push((owned_full_url, full_url));
-    res.push((owned_matched_route, route_match.based_route().to_string()));
+    res.push((owned_full_url, full_url.into()));
+    res.push((owned_matched_route, route_match.based_route().into()));
 
-    res.push((owned_base_path, "/".to_string()));
+    res.push((owned_base_path, "/".into()));
+    res.push((owned_raw_component_route, route_match.raw_route().into()));
     res.push((
-        owned_raw_component_route,
-        route_match.raw_route().to_string(),
+        owned_component_route,
+        route_match.raw_route_or_prefix().into(),
     ));
-    res.push((owned_component_route, route_match.raw_route_or_prefix()));
-    res.push((owned_client_addr, client_addr.to_string()));
+    res.push((owned_client_addr, client_addr.to_string().into()));
 
     for (wild_name, wild_value) in route_match.named_wildcards() {
-        let wild_header = format!("SPIN_PATH_MATCH_{}", wild_name.to_ascii_uppercase()); // TODO: safer
-        let wild_wagi_header = format!("X_PATH_MATCH_{}", wild_name.to_ascii_uppercase()); // TODO: safer
-        res.push(([wild_header, wild_wagi_header], wild_value.clone()));
+        let wild_header = format!("SPIN_PATH_MATCH_{}", wild_name.to_ascii_uppercase()).into();
+        let wild_wagi_header = format!("X_PATH_MATCH_{}", wild_name.to_ascii_uppercase()).into();
+        res.push(([wild_header, wild_wagi_header], wild_value.into()));
     }
 
     Ok(res)
@@ -110,7 +118,7 @@ pub fn prepare_request_headers(
     // In the future, we might want to have this information in a context
     // object as opposed to headers.
     for (keys, val) in compute_default_headers(req.uri(), host, route_match, client_addr)? {
-        res.push((prepare_header_key(&keys[0]), val));
+        res.push((prepare_header_key(&keys[0]), val.into_owned()));
     }
 
     Ok(res)
@@ -138,6 +146,8 @@ fn prepare_header_key(key: &str) -> String {
 
 #[cfg(test)]
 mod tests {
+    use std::borrow::Cow;
+
     use super::*;
     use anyhow::Result;
     use spin_http::routes::Router;
@@ -318,11 +328,14 @@ mod tests {
         assert!(req.headers().get("Host").is_some());
     }
 
-    fn search(keys: &[&str; 2], headers: &[([String; 2], String)]) -> Option<String> {
+    fn search(
+        keys: &[&str; 2],
+        headers: &[([Cow<'static, str>; 2], Cow<'_, str>)],
+    ) -> Option<String> {
         let mut res: Option<String> = None;
         for (k, v) in headers {
             if k[0] == keys[0] && k[1] == keys[1] {
-                res = Some(v.clone());
+                res = Some(v.as_ref().to_owned());
             }
         }