Turbopack: chore: Avoid string clones in Glob::parse by using RcStr

Author: bgw

turbopack/crates/turbo-tasks-fs/src/glob.rs

@@ -24,19 +24,21 @@ use crate::globset::parse;
 #[derive(Debug, Clone)]
 #[serde(into = "GlobForm", try_from = "GlobForm")]
 pub struct Glob {
-    glob: String,
+    glob: RcStr,
     #[turbo_tasks(trace_ignore)]
     opts: GlobOptions,
     #[turbo_tasks(trace_ignore)]
     regex: Regex,
     #[turbo_tasks(trace_ignore)]
     directory_match_regex: Regex,
 }
+
 impl PartialEq for Glob {
     fn eq(&self, other: &Self) -> bool {
         self.glob == other.glob
     }
 }
+
 impl Eq for Glob {}
 
 impl Display for Glob {
@@ -47,6 +49,7 @@ impl Display for Glob {
 #[derive(
     Serialize, Deserialize, Copy, Clone, PartialEq, Eq, Hash, Default, TaskInput, TraceRawVcs, Debug,
 )]
+
 pub struct GlobOptions {
     /// Whether the glob is a partial match.
     /// Allows glob to match any part of the given string(s).
@@ -58,9 +61,10 @@ pub struct GlobOptions {
 
 #[derive(Serialize, Deserialize)]
 struct GlobForm {
-    glob: String,
+    glob: RcStr,
     opts: GlobOptions,
 }
+
 impl From<Glob> for GlobForm {
     fn from(value: Glob) -> Self {
         Self {
@@ -69,10 +73,11 @@ impl From<Glob> for GlobForm {
         }
     }
 }
+
 impl TryFrom<GlobForm> for Glob {
     type Error = anyhow::Error;
     fn try_from(value: GlobForm) -> Result<Self, Self::Error> {
-        Glob::parse(&value.glob, value.opts)
+        Glob::parse(value.glob, value.opts)
     }
 }
 
@@ -92,33 +97,25 @@ impl Glob {
         self.directory_match_regex.is_match(path.as_bytes())
     }
 
-    pub fn parse(input: &str, opts: GlobOptions) -> Result<Glob> {
-        let (glob_re, directory_match_re) = parse(input, opts)?;
+    pub fn parse(input: RcStr, opts: GlobOptions) -> Result<Glob> {
+        let (glob_re, directory_match_re) = parse(&input, opts)?;
         let regex = new_regex(glob_re.as_str());
         let directory_match_regex = new_regex(directory_match_re.as_str());
 
         Ok(Glob {
-            glob: input.to_string(),
+            glob: input,
             opts,
             regex,
             directory_match_regex,
         })
     }
 }
 
-impl TryFrom<&str> for Glob {
-    type Error = anyhow::Error;
-
-    fn try_from(value: &str) -> Result<Self, Self::Error> {
-        Glob::parse(value, GlobOptions::default())
-    }
-}
-
 #[turbo_tasks::value_impl]
 impl Glob {
     #[turbo_tasks::function]
     pub fn new(glob: RcStr, opts: GlobOptions) -> Result<Vc<Self>> {
-        Ok(Self::cell(Glob::parse(glob.as_str(), opts)?))
+        Ok(Self::cell(Glob::parse(glob, opts)?))
     }
 
     #[turbo_tasks::function]
@@ -166,8 +163,7 @@ fn new_regex(pattern: &str) -> Regex {
 mod tests {
     use rstest::*;
 
-    use super::Glob;
-    use crate::glob::GlobOptions;
+    use super::*;
 
     #[rstest]
     #[case::file("file.js", "file.js")]
@@ -238,7 +234,7 @@ mod tests {
     #[case::alternatives_empty2("react{,-dom}", "react-dom")]
     #[case::alternatives_chars("[abc]", "b")]
     fn glob_match(#[case] glob: &str, #[case] path: &str) {
-        let glob = Glob::parse(glob, GlobOptions::default()).unwrap();
+        let glob = Glob::parse(RcStr::from(glob), GlobOptions::default()).unwrap();
 
         println!("{glob:?} {path}");
 
@@ -253,7 +249,7 @@ mod tests {
     )]
     #[case::star("*", "/foo")]
     fn glob_not_matching(#[case] glob: &str, #[case] path: &str) {
-        let glob = Glob::parse(glob, GlobOptions::default()).unwrap();
+        let glob = Glob::parse(RcStr::from(glob), GlobOptions::default()).unwrap();
 
         println!("{glob:?} {path}");
 
@@ -272,7 +268,7 @@ mod tests {
     #[case::globstar_in_dir_partial("dir/**/sub/file.js", "dir/a/b/sub")]
     #[case::globstar_in_dir_partial("dir/**/sub/file.js", "dir/a/b/sub/file.js")]
     fn glob_can_match_directory(#[case] glob: &str, #[case] path: &str) {
-        let glob = Glob::parse(glob, GlobOptions::default()).unwrap();
+        let glob = Glob::parse(RcStr::from(glob), GlobOptions::default()).unwrap();
 
         println!("{glob:?} {path}");
 
@@ -282,7 +278,7 @@ mod tests {
     #[case::dir_and_file_partial("dir/file.js", "dir/file.js")] // even if there was a dir, named `file.js` we know the glob wasn't intended to match it.
     #[case::alternatives_chars("[abc]", "b")]
     fn glob_not_can_match_directory(#[case] glob: &str, #[case] path: &str) {
-        let glob = Glob::parse(glob, GlobOptions::default()).unwrap();
+        let glob = Glob::parse(RcStr::from(glob), GlobOptions::default()).unwrap();
 
         println!("{glob:?} {path}");
 
@@ -297,7 +293,7 @@ mod tests {
     // This is a possibly surprising case.
     #[case::dir_match("node_modules/foo", "my_node_modules/foobar")]
     fn partial_glob_match(#[case] glob: &str, #[case] path: &str) {
-        let glob = Glob::parse(glob, GlobOptions { contains: true }).unwrap();
+        let glob = Glob::parse(RcStr::from(glob), GlobOptions { contains: true }).unwrap();
 
         println!("{glob:?} {path}");
 
@@ -311,7 +307,7 @@ mod tests {
     // This is a possibly surprising case
     #[case::dir_match("/node_modules/", "node_modules/")]
     fn partial_glob_not_matching(#[case] glob: &str, #[case] path: &str) {
-        let glob = Glob::parse(glob, GlobOptions { contains: true }).unwrap();
+        let glob = Glob::parse(RcStr::from(glob), GlobOptions { contains: true }).unwrap();
 
         println!("{glob:?} {path}");