fixup! perf(*): cache regex predicates with rc using router attribute

switch Rc to Arc and add static assertions

Author: nowNick

src/ast.rs

@@ -1,7 +1,7 @@
 use crate::schema::Schema;
 use cidr::IpCidr;
 use regex::Regex;
-use std::{net::IpAddr, rc::Rc};
+use std::{net::IpAddr, sync::Arc};
 
 #[cfg(feature = "serde")]
 use serde::{Deserialize, Serialize};
@@ -53,7 +53,7 @@ pub enum Value {
     IpAddr(IpAddr),
     Int(i64),
     #[cfg_attr(feature = "serde", serde(with = "serde_regex"))]
-    Regex(Rc<Regex>),
+    Regex(Arc<Regex>),
 }
 
 impl PartialEq for Value {

src/parser.rs

@@ -13,7 +13,7 @@ use pest::Parser;
 use regex::Regex;
 use std::collections::HashMap;
 use std::net::{IpAddr, Ipv4Addr, Ipv6Addr};
-use std::rc::Rc;
+use std::sync::Arc;
 
 type ParseResult<T> = Result<T, ParseError<Rule>>;
 
@@ -66,7 +66,7 @@ impl ATCParser {
     fn parse_matcher(
         &mut self,
         source: &str,
-        regex_cache: &mut HashMap<String, Rc<Regex>>,
+        regex_cache: &mut HashMap<String, Arc<Regex>>,
     ) -> ParseResult<Expression> {
         let pairs = ATCParser::parse(Rule::matcher, source)?;
         let expr_pair = pairs.peek().unwrap().into_inner().peek().unwrap();
@@ -212,7 +212,7 @@ fn parse_int_literal(pair: Pair<Rule>) -> ParseResult<i64> {
 #[allow(clippy::result_large_err)] // it's fine as parsing is not the hot path
 fn parse_predicate(
     pair: Pair<Rule>,
-    regex_cache: &mut HashMap<String, Rc<Regex>>,
+    regex_cache: &mut HashMap<String, Arc<Regex>>,
 ) -> ParseResult<Predicate> {
     let mut pairs = pair.into_inner();
     let lhs = parse_lhs(pairs.next().unwrap())?;
@@ -236,7 +236,7 @@ fn parse_predicate(
                 None => {
                     let r = Regex::new(&s).into_parse_result(&rhs_pair)?;
 
-                    let rc = Rc::new(r);
+                    let rc = Arc::new(r);
 
                     regex_cache.insert(s, rc.clone());
                     rc
@@ -302,7 +302,7 @@ fn parse_binary_operator(pair: Pair<Rule>) -> BinaryOperator {
 fn parse_parenthesised_expression(
     pair: Pair<Rule>,
     pratt: &PrattParser<Rule>,
-    regex_cache: &mut HashMap<String, Rc<Regex>>,
+    regex_cache: &mut HashMap<String, Arc<Regex>>,
 ) -> ParseResult<Expression> {
     let mut pairs = pair.into_inner();
     let pair = pairs.next().unwrap();
@@ -321,7 +321,7 @@ fn parse_parenthesised_expression(
 fn parse_term(
     pair: Pair<Rule>,
     pratt: &PrattParser<Rule>,
-    regex_cache: &mut HashMap<String, Rc<Regex>>,
+    regex_cache: &mut HashMap<String, Arc<Regex>>,
 ) -> ParseResult<Expression> {
     let pairs = pair.into_inner();
     let inner_rule = pairs.peek().unwrap();
@@ -343,7 +343,7 @@ fn parse_term(
 fn parse_expression(
     pair: Pair<Rule>,
     pratt: &PrattParser<Rule>,
-    regex_cache: &mut HashMap<String, Rc<Regex>>,
+    regex_cache: &mut HashMap<String, Arc<Regex>>,
 ) -> ParseResult<Expression> {
     let pairs = pair.into_inner();
     pratt
@@ -364,7 +364,7 @@ fn parse_expression(
 #[allow(clippy::result_large_err)] // it's fine as parsing is not the hot path
 pub fn parse(
     source: &str,
-    regex_cache: &mut HashMap<String, Rc<Regex>>,
+    regex_cache: &mut HashMap<String, Arc<Regex>>,
 ) -> ParseResult<Expression> {
     ATCParser::new().parse_matcher(source, regex_cache)
 }

src/router.rs

@@ -6,7 +6,7 @@ use crate::schema::Schema;
 use crate::semantics::{FieldCounter, Validate};
 use regex::Regex;
 use std::collections::{BTreeMap, HashMap};
-use std::rc::Rc;
+use std::sync::Arc;
 use uuid::Uuid;
 
 #[derive(Debug, PartialEq, Eq, PartialOrd, Ord)]
@@ -17,7 +17,7 @@ pub struct Router<'a> {
     schema: &'a Schema,
     matchers: BTreeMap<MatcherKey, Expression>,
     pub fields: HashMap<String, usize>,
-    regex_cache: HashMap<String, Rc<Regex>>,
+    regex_cache: HashMap<String, Arc<Regex>>,
 }
 
 fn release_cache(expr: &Expression, router: &mut Router) {
@@ -30,10 +30,10 @@ fn release_cache(expr: &Expression, router: &mut Router) {
             LogicalExpression::Not(r) => release_cache(r, router),
         },
         Expression::Predicate(p) => {
-            if let Value::Regex(rc) = &p.rhs {
-                if Rc::strong_count(rc) == 2 {
+            if let Value::Regex(arc) = &p.rhs {
+                if Arc::strong_count(arc) == 2 {
                     // about to be dropped and the only Rc left is in the map
-                    router.regex_cache.remove(rc.as_str());
+                    router.regex_cache.remove(arc.as_str());
                 }
             }
         }
@@ -170,4 +170,13 @@ mod tests {
         ctx.add_value("http.path", "/not-dev".to_owned().into());
         router.try_match(&ctx).ok_or(()).expect_err("should fail");
     }
+
+    // Router might be used in async context therefore we need to assert that it implements Send and Sync traits.
+    fn _assert_send<T: Send>() {}
+    fn _assert_sync<T: Sync>() {}
+
+    fn _assertions() {
+        _assert_send::<Router>();
+        _assert_sync::<Router>();
+    }
 }