rework IntoPatterns trait and codegen (#372)

Cargo.toml

@@ -27,3 +27,8 @@ actix-utils = { path = "actix-utils" }
 bytestring = { path = "bytestring" }
 local-channel = { path = "local-channel" }
 local-waker = { path = "local-waker" }
+
+[profile.release]
+lto = true
+opt-level = 3
+codegen-units = 1

actix-router/CHANGES.md

@@ -6,6 +6,7 @@
 * Fix segment interpolation leaving `Path` in unintended state after matching. [#368]
 * Path tail pattern now works as expected after a dynamic segment (e.g. `/user/{uid}/*`). [#366]
 * Fixed a bug where, in multi-patterns, static patterns are interpreted as regex. [#366]
+* Re-work `IntoPatterns` trait. [#372]
 * Rename `Path::{len => segment_count}` to be more descriptive of it's purpose. [#370]
 * Alias `ResourceDef::{resource_path => resource_path_from_iter}` pending eventual deprecation. [#371]
 * Alias `ResourceDef::{resource_path_named => resource_path_from_map}` pending eventual deprecation. [#371]
@@ -15,6 +16,7 @@
 [#368]: https://github.com/actix/actix-net/pull/368
 [#370]: https://github.com/actix/actix-net/pull/370
 [#371]: https://github.com/actix/actix-net/pull/371
+[#372]: https://github.com/actix/actix-net/pull/372
 
 
 ## 0.4.0 - 2021-06-06

actix-router/Cargo.toml

@@ -20,12 +20,17 @@ path = "src/lib.rs"
 default = ["http"]
 
 [dependencies]
+bytestring = ">=0.1.5, <2"
+http = { version = "0.2.3", optional = true }
+log = "0.4"
 regex = "1.5"
 serde = "1"
-bytestring = ">=0.1.5, <2"
-log = "0.4"
-http = { version = "0.2.3", optional = true }
 
 [dev-dependencies]
 http = "0.2.3"
 serde = { version = "1", features = ["derive"] }
+criterion = { version = "0.3", features = ["html_reports"] }
+
+[[bench]]
+name = "router"
+harness = false

actix-router/benches/router.rs

@@ -0,0 +1,194 @@
+//! Based on https://github.com/ibraheemdev/matchit/blob/master/benches/bench.rs
+
+use criterion::{black_box, criterion_group, criterion_main, Criterion};
+
+macro_rules! register {
+    (colon) => {{
+        register!(finish => ":p1", ":p2", ":p3", ":p4")
+    }};
+    (brackets) => {{
+        register!(finish => "{p1}", "{p2}", "{p3}", "{p4}")
+    }};
+    (regex) => {{
+        register!(finish => "(.*)", "(.*)", "(.*)", "(.*)")
+    }};
+    (finish => $p1:literal, $p2:literal, $p3:literal, $p4:literal) => {{
+        let arr = [
+            concat!("/authorizations"),
+            concat!("/authorizations/", $p1),
+            concat!("/applications/", $p1, "/tokens/", $p2),
+            concat!("/events"),
+            concat!("/repos/", $p1, "/", $p2, "/events"),
+            concat!("/networks/", $p1, "/", $p2, "/events"),
+            concat!("/orgs/", $p1, "/events"),
+            concat!("/users/", $p1, "/received_events"),
+            concat!("/users/", $p1, "/received_events/public"),
+            concat!("/users/", $p1, "/events"),
+            concat!("/users/", $p1, "/events/public"),
+            concat!("/users/", $p1, "/events/orgs/", $p2),
+            concat!("/feeds"),
+            concat!("/notifications"),
+            concat!("/repos/", $p1, "/", $p2, "/notifications"),
+            concat!("/notifications/threads/", $p1),
+            concat!("/notifications/threads/", $p1, "/subscription"),
+            concat!("/repos/", $p1, "/", $p2, "/stargazers"),
+            concat!("/users/", $p1, "/starred"),
+            concat!("/user/starred"),
+            concat!("/user/starred/", $p1, "/", $p2),
+            concat!("/repos/", $p1, "/", $p2, "/subscribers"),
+            concat!("/users/", $p1, "/subscriptions"),
+            concat!("/user/subscriptions"),
+            concat!("/repos/", $p1, "/", $p2, "/subscription"),
+            concat!("/user/subscriptions/", $p1, "/", $p2),
+            concat!("/users/", $p1, "/gists"),
+            concat!("/gists"),
+            concat!("/gists/", $p1),
+            concat!("/gists/", $p1, "/star"),
+            concat!("/repos/", $p1, "/", $p2, "/git/blobs/", $p3),
+            concat!("/repos/", $p1, "/", $p2, "/git/commits/", $p3),
+            concat!("/repos/", $p1, "/", $p2, "/git/refs"),
+            concat!("/repos/", $p1, "/", $p2, "/git/tags/", $p3),
+            concat!("/repos/", $p1, "/", $p2, "/git/trees/", $p3),
+            concat!("/issues"),
+            concat!("/user/issues"),
+            concat!("/orgs/", $p1, "/issues"),
+            concat!("/repos/", $p1, "/", $p2, "/issues"),
+            concat!("/repos/", $p1, "/", $p2, "/issues/", $p3),
+            concat!("/repos/", $p1, "/", $p2, "/assignees"),
+            concat!("/repos/", $p1, "/", $p2, "/assignees/", $p3),
+            concat!("/repos/", $p1, "/", $p2, "/issues/", $p3, "/comments"),
+            concat!("/repos/", $p1, "/", $p2, "/issues/", $p3, "/events"),
+            concat!("/repos/", $p1, "/", $p2, "/labels"),
+            concat!("/repos/", $p1, "/", $p2, "/labels/", $p3),
+            concat!("/repos/", $p1, "/", $p2, "/issues/", $p3, "/labels"),
+            concat!("/repos/", $p1, "/", $p2, "/milestones/", $p3, "/labels"),
+            concat!("/repos/", $p1, "/", $p2, "/milestones/"),
+            concat!("/repos/", $p1, "/", $p2, "/milestones/", $p3),
+            concat!("/emojis"),
+            concat!("/gitignore/templates"),
+            concat!("/gitignore/templates/", $p1),
+            concat!("/meta"),
+            concat!("/rate_limit"),
+            concat!("/users/", $p1, "/orgs"),
+            concat!("/user/orgs"),
+            concat!("/orgs/", $p1),
+            concat!("/orgs/", $p1, "/members"),
+            concat!("/orgs/", $p1, "/members", $p2),
+            concat!("/orgs/", $p1, "/public_members"),
+            concat!("/orgs/", $p1, "/public_members/", $p2),
+            concat!("/orgs/", $p1, "/teams"),
+            concat!("/teams/", $p1),
+            concat!("/teams/", $p1, "/members"),
+            concat!("/teams/", $p1, "/members", $p2),
+            concat!("/teams/", $p1, "/repos"),
+            concat!("/teams/", $p1, "/repos/", $p2, "/", $p3),
+            concat!("/user/teams"),
+            concat!("/repos/", $p1, "/", $p2, "/pulls"),
+            concat!("/repos/", $p1, "/", $p2, "/pulls/", $p3),
+            concat!("/repos/", $p1, "/", $p2, "/pulls/", $p3, "/commits"),
+            concat!("/repos/", $p1, "/", $p2, "/pulls/", $p3, "/files"),
+            concat!("/repos/", $p1, "/", $p2, "/pulls/", $p3, "/merge"),
+            concat!("/repos/", $p1, "/", $p2, "/pulls/", $p3, "/comments"),
+            concat!("/user/repos"),
+            concat!("/users/", $p1, "/repos"),
+            concat!("/orgs/", $p1, "/repos"),
+            concat!("/repositories"),
+            concat!("/repos/", $p1, "/", $p2),
+            concat!("/repos/", $p1, "/", $p2, "/contributors"),
+            concat!("/repos/", $p1, "/", $p2, "/languages"),
+            concat!("/repos/", $p1, "/", $p2, "/teams"),
+            concat!("/repos/", $p1, "/", $p2, "/tags"),
+            concat!("/repos/", $p1, "/", $p2, "/branches"),
+            concat!("/repos/", $p1, "/", $p2, "/branches/", $p3),
+            concat!("/repos/", $p1, "/", $p2, "/collaborators"),
+            concat!("/repos/", $p1, "/", $p2, "/collaborators/", $p3),
+            concat!("/repos/", $p1, "/", $p2, "/comments"),
+            concat!("/repos/", $p1, "/", $p2, "/commits/", $p3, "/comments"),
+            concat!("/repos/", $p1, "/", $p2, "/commits"),
+            concat!("/repos/", $p1, "/", $p2, "/commits/", $p3),
+            concat!("/repos/", $p1, "/", $p2, "/readme"),
+            concat!("/repos/", $p1, "/", $p2, "/keys"),
+            concat!("/repos/", $p1, "/", $p2, "/keys", $p3),
+            concat!("/repos/", $p1, "/", $p2, "/downloads"),
+            concat!("/repos/", $p1, "/", $p2, "/downloads", $p3),
+            concat!("/repos/", $p1, "/", $p2, "/forks"),
+            concat!("/repos/", $p1, "/", $p2, "/hooks"),
+            concat!("/repos/", $p1, "/", $p2, "/hooks", $p3),
+            concat!("/repos/", $p1, "/", $p2, "/releases"),
+            concat!("/repos/", $p1, "/", $p2, "/releases/", $p3),
+            concat!("/repos/", $p1, "/", $p2, "/releases/", $p3, "/assets"),
+            concat!("/repos/", $p1, "/", $p2, "/stats/contributors"),
+            concat!("/repos/", $p1, "/", $p2, "/stats/commit_activity"),
+            concat!("/repos/", $p1, "/", $p2, "/stats/code_frequency"),
+            concat!("/repos/", $p1, "/", $p2, "/stats/participation"),
+            concat!("/repos/", $p1, "/", $p2, "/stats/punch_card"),
+            concat!("/repos/", $p1, "/", $p2, "/statuses/", $p3),
+            concat!("/search/repositories"),
+            concat!("/search/code"),
+            concat!("/search/issues"),
+            concat!("/search/users"),
+            concat!("/legacy/issues/search/", $p1, "/", $p2, "/", $p3, "/", $p4),
+            concat!("/legacy/repos/search/", $p1),
+            concat!("/legacy/user/search/", $p1),
+            concat!("/legacy/user/email/", $p1),
+            concat!("/users/", $p1),
+            concat!("/user"),
+            concat!("/users"),
+            concat!("/user/emails"),
+            concat!("/users/", $p1, "/followers"),
+            concat!("/user/followers"),
+            concat!("/users/", $p1, "/following"),
+            concat!("/user/following"),
+            concat!("/user/following/", $p1),
+            concat!("/users/", $p1, "/following", $p2),
+            concat!("/users/", $p1, "/keys"),
+            concat!("/user/keys"),
+            concat!("/user/keys/", $p1),
+        ];
+        std::array::IntoIter::new(arr)
+    }};
+}
+
+fn call() -> impl Iterator<Item = &'static str> {
+    let arr = [
+        "/authorizations",
+        "/user/repos",
+        "/repos/rust-lang/rust/stargazers",
+        "/orgs/rust-lang/public_members/nikomatsakis",
+        "/repos/rust-lang/rust/releases/1.51.0",
+    ];
+
+    std::array::IntoIter::new(arr)
+}
+
+fn compare_routers(c: &mut Criterion) {
+    let mut group = c.benchmark_group("Compare Routers");
+
+    let mut actix = actix_router::Router::<bool>::build();
+    for route in register!(brackets) {
+        actix.path(route, true);
+    }
+    let actix = actix.finish();
+    group.bench_function("actix", |b| {
+        b.iter(|| {
+            for route in call() {
+                let mut path = actix_router::Path::new(route);
+                black_box(actix.recognize(&mut path).unwrap());
+            }
+        });
+    });
+
+    let regex_set = regex::RegexSet::new(register!(regex)).unwrap();
+    group.bench_function("regex", |b| {
+        b.iter(|| {
+            for route in call() {
+                black_box(regex_set.matches(route));
+            }
+        });
+    });
+
+    group.finish();
+}
+
+criterion_group!(benches, compare_routers);
+criterion_main!(benches);

actix-router/src/lib.rs

@@ -40,98 +40,71 @@ impl ResourcePath for bytestring::ByteString {
     }
 }
 
-/// Helper trait for type that could be converted to path pattern
+/// One or many patterns.
-pub trait IntoPattern {
+#[derive(Debug, Clone)]
-    fn is_single(&self) -> bool;
+pub enum Patterns {
-
+    Single(String),
-    fn patterns(&self) -> Vec<String>;
+    List(Vec<String>),
 }
 
-impl IntoPattern for String {
+/// Helper trait for type that could be converted to one or more path pattern.
-    fn is_single(&self) -> bool {
+pub trait IntoPatterns {
-        true
+    fn patterns(&self) -> Patterns;
 }
 
-    fn patterns(&self) -> Vec<String> {
+impl IntoPatterns for String {
-        vec![self.clone()]
+    fn patterns(&self) -> Patterns {
+        Patterns::Single(self.clone())
     }
 }
 
-impl<'a> IntoPattern for &'a String {
+impl<'a> IntoPatterns for &'a String {
-    fn is_single(&self) -> bool {
+    fn patterns(&self) -> Patterns {
-        true
+        Patterns::Single((*self).clone())
-    }
-
-    fn patterns(&self) -> Vec<String> {
-        vec![self.as_str().to_string()]
     }
 }
 
-impl<'a> IntoPattern for &'a str {
+impl<'a> IntoPatterns for &'a str {
-    fn is_single(&self) -> bool {
+    fn patterns(&self) -> Patterns {
-        true
+        Patterns::Single((*self).to_owned())
-    }
-
-    fn patterns(&self) -> Vec<String> {
-        vec![(*self).to_string()]
     }
 }
 
-impl<T: AsRef<str>> IntoPattern for Vec<T> {
+impl<T: AsRef<str>> IntoPatterns for Vec<T> {
-    fn is_single(&self) -> bool {
+    fn patterns(&self) -> Patterns {
-        self.len() == 1
+        let mut patterns = self.iter().map(|v| v.as_ref().to_owned());
-    }
 
-    fn patterns(&self) -> Vec<String> {
+        match patterns.size_hint() {
-        self.iter().map(|v| v.as_ref().to_string()).collect()
+            (1, _) => Patterns::Single(patterns.next().unwrap()),
+            _ => Patterns::List(patterns.collect()),
+        }
     }
 }
 
-macro_rules! array_patterns (($tp:ty, $num:tt) => {
+macro_rules! array_patterns_single (($tp:ty) => {
-    impl IntoPattern for [$tp; $num] {
+    impl IntoPatterns for [$tp; 1] {
-        fn is_single(&self) -> bool {
+        fn patterns(&self) -> Patterns {
-            $num == 1
+            Patterns::Single(self[0].to_owned())
-        }
-
-        fn patterns(&self) -> Vec<String> {
-            self.iter().map(|v| v.to_string()).collect()
         }
     }
 });
 
-array_patterns!(&str, 1);
+macro_rules! array_patterns_multiple (($tp:ty, $str_fn:expr, $($num:tt) +) => {
-array_patterns!(&str, 2);
+    // for each array length specified in $num
-array_patterns!(&str, 3);
+    $(
-array_patterns!(&str, 4);
+        impl IntoPatterns for [$tp; $num] {
-array_patterns!(&str, 5);
+            fn patterns(&self) -> Patterns {
-array_patterns!(&str, 6);
+                Patterns::List(self.iter().map($str_fn).collect())
-array_patterns!(&str, 7);
+            }
-array_patterns!(&str, 8);
+        }
-array_patterns!(&str, 9);
+    )+
-array_patterns!(&str, 10);
+});
-array_patterns!(&str, 11);
-array_patterns!(&str, 12);
-array_patterns!(&str, 13);
-array_patterns!(&str, 14);
-array_patterns!(&str, 15);
-array_patterns!(&str, 16);
 
-array_patterns!(String, 1);
+array_patterns_single!(&str);
-array_patterns!(String, 2);
+array_patterns_multiple!(&str, |&v| v.to_owned(), 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16);
-array_patterns!(String, 3);
+
-array_patterns!(String, 4);
+array_patterns_single!(String);
-array_patterns!(String, 5);
+array_patterns_multiple!(String, |v| v.clone(), 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16);
-array_patterns!(String, 6);
-array_patterns!(String, 7);
-array_patterns!(String, 8);
-array_patterns!(String, 9);
-array_patterns!(String, 10);
-array_patterns!(String, 11);
-array_patterns!(String, 12);
-array_patterns!(String, 13);
-array_patterns!(String, 14);
-array_patterns!(String, 15);
-array_patterns!(String, 16);
 
 #[cfg(feature = "http")]
 mod url;
@@ -140,10 +113,11 @@ mod url;
 pub use self::url::{Quoter, Url};
 
 #[cfg(feature = "http")]
-mod http_support {
+mod http_impls {
-    use super::ResourcePath;
     use http::Uri;
 
+    use super::ResourcePath;
+
     impl ResourcePath for Uri {
         fn path(&self) -> &str {
             self.path()

actix-router/src/resource.rs

@@ -8,8 +8,10 @@ use std::{
 
 use regex::{escape, Regex, RegexSet};
 
-use crate::path::{Path, PathItem};
+use crate::{
-use crate::{IntoPattern, Resource, ResourcePath};
+    path::{Path, PathItem},
+    IntoPatterns, Patterns, Resource, ResourcePath,
+};
 
 const MAX_DYNAMIC_SEGMENTS: usize = 16;
 
@@ -25,9 +27,6 @@ const REGEX_FLAGS: &str = "(?s-m)";
 pub struct ResourceDef {
     id: u16,
 
-    /// Pattern type.
-    pat_type: PatternType,
-
     /// Optional name of resource definition. Defaults to "".
     name: String,
 
@@ -35,6 +34,9 @@ pub struct ResourceDef {
     // TODO: Sort of, in dynamic set pattern type it is blank, consider change to option.
     pattern: String,
 
+    /// Pattern type.
+    pat_type: PatternType,
+
     /// List of elements that compose the pattern, in order.
     ///
     /// `None` with pattern type is DynamicSet.
@@ -75,29 +77,43 @@ impl ResourceDef {
     /// Parse path pattern and create new `Pattern` instance.
     ///
     /// Panics if path pattern is malformed.
-    pub fn new<T: IntoPattern>(path: T) -> Self {
+    pub fn new<T: IntoPatterns>(path: T) -> Self {
-        if path.is_single() {
+        match path.patterns() {
-            ResourceDef::from_single_pattern(&path.patterns()[0], false)
+            Patterns::Single(pattern) => ResourceDef::from_single_pattern(&pattern, false),
-        } else {
-            let mut data = Vec::new();
-            let mut re_set = Vec::new();
 
-            for pattern in path.patterns() {
+            // since zero length pattern sets are possible
+            // just return a useless `ResourceDef`
+            Patterns::List(patterns) if patterns.is_empty() => ResourceDef {
+                id: 0,
+                name: String::new(),
+                pattern: String::new(),
+                pat_type: PatternType::DynamicSet(RegexSet::empty(), Vec::new()),
+                elements: None,
+            },
+
+            Patterns::List(patterns) => {
+                let mut re_set = Vec::with_capacity(patterns.len());
+                let mut pattern_data = Vec::new();
+
+                for pattern in patterns {
                     match ResourceDef::parse(&pattern, false, true) {
                         (PatternType::Dynamic(re, names), _) => {
                             re_set.push(re.as_str().to_owned());
-                        data.push((re, names));
+                            pattern_data.push((re, names));
                         }
                         _ => unreachable!(),
                     }
                 }
 
+                let pattern_re_set = RegexSet::new(re_set).unwrap();
+
                 ResourceDef {
                     id: 0,
-                pat_type: PatternType::DynamicSet(RegexSet::new(re_set).unwrap(), data),
-                elements: None,
                     name: String::new(),
-                pattern: "".to_owned(),
+                    pattern: String::new(),
+                    pat_type: PatternType::DynamicSet(pattern_re_set, pattern_data),
+                    elements: None,
+                }
             }
         }
     }

actix-router/src/router.rs

@@ -1,4 +1,4 @@
-use crate::{IntoPattern, Resource, ResourceDef, ResourcePath};
+use crate::{IntoPatterns, Resource, ResourceDef, ResourcePath};
 
 #[derive(Debug, Copy, Clone, PartialEq)]
 pub struct ResourceId(pub u16);
@@ -88,7 +88,7 @@ pub struct RouterBuilder<T, U = ()> {
 
 impl<T, U> RouterBuilder<T, U> {
     /// Register resource for specified path.
-    pub fn path<P: IntoPattern>(
+    pub fn path<P: IntoPatterns>(
         &mut self,
         path: P,
         resource: T,