Repository: vercel/next.js
Pull request: [turbopack] Simplify `Request::parse(Pattern)` the recursive portion doesn't need to be async

## Optimize Request parsing in Turbopack core resolver

### What?
Refactored the `Request::parse_ref` method and related functions to improve performance by:
- Converting async functions to synchronous where possible
- Normalizing patterns before parsing to improve cache hits
- Handling alternatives pattern separately in the async `parse_inner` function
- Adopt `rcstr!` in a few places

### Why?
This should be a small perf enhancement, basically just noticed the `Box::pin` calls and decided to track down what was fundamentally async.

Closes PACK-4841

Changed files:
filename: turbopack/crates/turbopack-core/src/resolve/parse.rs
patch: @@ -142,88 +142,91 @@ impl Request {
         })
     }
 
-    pub async fn parse_ref(mut request: Pattern) -> Result<Self> {
-        request.normalize();
-        Ok(match request {
+    /// Internal construction function.  Should only be called with normalized patterns, or
+    /// recursively. Most users should call [Self::parse] instead.
+    fn parse_ref(request: Pattern) -> Self {
+        match request {
             Pattern::Dynamic => Request::Dynamic,
-            Pattern::Constant(r) => Request::parse_constant_pattern(r).await?,
-            Pattern::Concatenation(list) => Request::parse_concatenation_pattern(list).await?,
-            Pattern::Alternatives(list) => Request::parse_alternatives_pattern(list).await?,
-        })
+            Pattern::Constant(r) => Request::parse_constant_pattern(r),
+            Pattern::Concatenation(list) => Request::parse_concatenation_pattern(list),
+            Pattern::Alternatives(_) => panic!(
+                "request should be normalized and alternatives should have already been handled.",
+            ),
+        }
     }
 
-    async fn parse_constant_pattern(r: RcStr) -> Result<Self> {
+    fn parse_constant_pattern(r: RcStr) -> Self {
         if r.is_empty() {
-            return Ok(Request::Empty);
+            return Request::Empty;
         }
 
         if let Some(remainder) = r.strip_prefix("//") {
-            return Ok(Request::Uri {
+            return Request::Uri {
                 protocol: rcstr!("//"),
                 remainder: remainder.into(),
                 query: RcStr::default(),
                 fragment: RcStr::default(),
-            });
+            };
         }
 
         if r.starts_with('/') {
             let (path, query, fragment) = split_off_query_fragment(&r);
 
-            return Ok(Request::ServerRelative {
+            return Request::ServerRelative {
                 path,
                 query,
                 fragment,
-            });
+            };
         }
 
         if r.starts_with('#') {
-            return Ok(Request::PackageInternal {
+            return Request::PackageInternal {
                 path: Pattern::Constant(r),
-            });
+            };
         }
 
         if r.starts_with("./") || r.starts_with("../") || r == "." || r == ".." {
             let (path, query, fragment) = split_off_query_fragment(&r);
 
-            return Ok(Request::Relative {
+            return Request::Relative {
                 path,
                 force_in_lookup_dir: false,
                 query,
                 fragment,
-            });
+            };
         }
 
         if WINDOWS_PATH.is_match(&r) {
             let (path, query, fragment) = split_off_query_fragment(&r);
 
-            return Ok(Request::Windows {
+            return Request::Windows {
                 path,
                 query,
                 fragment,
-            });
+            };
         }
 
         if let Some(caps) = URI_PATH.captures(&r)
             && let (Some(protocol), Some(remainder)) = (caps.get(1), caps.get(2))
         {
             if let Some(caps) = DATA_URI_REMAINDER.captures(remainder.as_str()) {
-                let media_type = caps.get(1).map_or("", |m| m.as_str()).into();
-                let encoding = caps.get(2).map_or("", |e| e.as_str()).into();
-                let data = caps.get(3).map_or("", |d| d.as_str()).into();
+                let media_type = caps.get(1).map_or(RcStr::default(), |m| m.as_str().into());
+                let encoding = caps.get(2).map_or(RcStr::default(), |e| e.as_str().into());
+                let data = caps.get(3).map_or(RcStr::default(), |d| d.as_str().into());
 
-                return Ok(Request::DataUri {
+                return Request::DataUri {
                     media_type,
                     encoding,
                     data: ResolvedVc::cell(data),
-                });
+                };
             }
 
-            return Ok(Request::Uri {
+            return Request::Uri {
                 protocol: protocol.as_str().into(),
                 remainder: remainder.as_str().into(),
                 query: RcStr::default(),
                 fragment: RcStr::default(),
-            });
+            };
         }
 
         if let Some((module, path)) = MODULE_PATH
@@ -232,25 +235,25 @@ impl Request {
         {
             let (path, query, fragment) = split_off_query_fragment(path.as_str());
 
-            return Ok(Request::Module {
+            return Request::Module {
                 module: module.as_str().into(),
                 path,
                 query,
                 fragment,
-            });
+            };
         }
 
-        Ok(Request::Unknown {
+        Request::Unknown {
             path: Pattern::Constant(r),
-        })
+        }
     }
 
-    async fn parse_concatenation_pattern(list: Vec<Pattern>) -> Result<Self> {
+    fn parse_concatenation_pattern(list: Vec<Pattern>) -> Self {
         if list.is_empty() {
-            return Ok(Request::Empty);
+            return Request::Empty;
         }
 
-        let mut result = Box::pin(Self::parse_ref(list[0].clone())).await?;
+        let mut result = Self::parse_ref(list[0].clone());
 
         for item in list.into_iter().skip(1) {
             match &mut result {
@@ -270,7 +273,7 @@ impl Request {
                     path.push(item);
                 }
                 Request::Empty => {
-                    result = Box::pin(Self::parse_ref(item)).await?;
+                    result = Self::parse_ref(item);
                 }
                 Request::PackageInternal { path } => {
                     path.push(item);
@@ -289,30 +292,39 @@ impl Request {
             };
         }
 
-        Ok(result)
-    }
-
-    async fn parse_alternatives_pattern(list: Vec<Pattern>) -> Result<Self> {
-        Ok(Request::Alternatives {
-            requests: list
-                .into_iter()
-                .map(Request::parse)
-                .map(|v| async move { v.to_resolved().await })
-                .try_join()
-                .await?,
-        })
+        result
     }
 
     pub fn parse_string(request: RcStr) -> Vc<Self> {
         Self::parse(request.into())
     }
+
+    pub fn parse(mut request: Pattern) -> Vc<Self> {
+        // Call normalize before parse_inner to improve cache hits.
+        request.normalize();
+        Self::parse_inner(request)
+    }
 }
 
 #[turbo_tasks::value_impl]
 impl Request {
     #[turbo_tasks::function]
-    pub async fn parse(request: Pattern) -> Result<Vc<Self>> {
-        Ok(Self::cell(Request::parse_ref(request).await?))
+    async fn parse_inner(request: Pattern) -> Result<Vc<Self>> {
+        // Because we are normalized, we should handle alternatives here
+        if let Pattern::Alternatives(alts) = request {
+            Ok(Self::cell(Self::Alternatives {
+                requests: alts
+                    .into_iter()
+                    // We can call parse_inner directly because these patterns are already
+                    // normalized.  We don't call `Self::parse_ref` so we can try to get a cache hit
+                    // on the sub-patterns
+                    .map(|p| Self::parse_inner(p).to_resolved())
+                    .try_join()
+                    .await?,
+            }))
+        } else {
+            Ok(Self::cell(Self::parse_ref(request)))
+        }
     }
 
     #[turbo_tasks::function]

filename: turbopack/crates/turbopack-core/src/resolve/pattern.rs
patch: @@ -10,7 +10,7 @@ use regex::Regex;
 use rustc_hash::{FxHashMap, FxHashSet};
 use serde::{Deserialize, Serialize};
 use tracing::Instrument;
-use turbo_rcstr::RcStr;
+use turbo_rcstr::{RcStr, rcstr};
 use turbo_tasks::{
     NonLocalValue, ResolvedVc, TaskInput, ValueToString, Vc, debug::ValueDebugFormat,
     trace::TraceRawVcs,
@@ -448,13 +448,20 @@ impl Pattern {
         current
     }
 
+    /// Normalizes paths by
+    /// - processing path segments: `.` and `..`
+    /// - normalizing windows filepaths by replacing `\` with `/`
+    ///
+    /// The Pattern must have already been processed by [Self::normalize].
+    /// Returns [Option::None] if any of the patterns attempt to navigate out of the root.
     pub fn with_normalized_path(&self) -> Option<Pattern> {
         let mut new = self.clone();
 
         fn normalize_path_internal(pattern: &mut Pattern) -> Option<()> {
             match pattern {
                 Pattern::Constant(c) => {
-                    *c = (*(normalize_path(c)?)).into();
+                    let normalized = c.replace('\\', "/");
+                    *c = (*(normalize_path(normalized.as_str())?)).into();
                     Some(())
                 }
                 Pattern::Dynamic => Some(()),
@@ -490,7 +497,7 @@ impl Pattern {
                             }
                         }
                     }
-                    let separator: RcStr = "/".into();
+                    let separator = rcstr!("/");
                     *list = segments
                         .into_iter()
                         .flat_map(|c| {
@@ -1091,7 +1098,7 @@ impl Pattern {
 
     pub fn or_any_nested_file(&self) -> Self {
         let mut new = self.clone();
-        new.push(Pattern::Constant("/".into()));
+        new.push(Pattern::Constant(rcstr!("/")));
         new.push(Pattern::Dynamic);
         new.normalize();
         Pattern::alternatives([self.clone(), new])
@@ -1548,10 +1555,10 @@ pub async fn read_matches(
             }
             if prefix.is_empty() {
                 if let Some(pos) = pat.match_position("./") {
-                    results.push((pos, PatternMatch::Directory("./".into(), lookup_dir)));
+                    results.push((pos, PatternMatch::Directory(rcstr!("./"), lookup_dir)));
                 }
                 if let Some(pos) = pat.could_match_position("./") {
-                    nested.push((pos, read_matches(*lookup_dir, "./".into(), false, pattern)));
+                    nested.push((pos, read_matches(*lookup_dir, rcstr!("./"), false, pattern)));
                 }
             } else {
                 prefix.push('/');