Reduce number of string clones when compiling python source code (#8103)

Lib/test/test_thread.py

@@ -115,7 +115,7 @@ class ThreadRunningTests(BasicThreadTest):
 
         thread.stack_size(0)
 
-    @unittest.skipIf(__import__("sys").platform == "linux", "TODO: RUSTPYTHON; Flakey on CI")
+    @unittest.skipIf(__import__("sys").platform in ("linux", "win32"), "TODO: RUSTPYTHON; Flakey on CI")
     def test__count(self):
         # Test the _count() function.
         orig = thread._count()

benches/execution.rs

@@ -30,7 +30,7 @@ fn bench_rustpython_code(b: &mut Bencher, name: &str, source: &str) {
         // Note: bench_cpython is both compiling and executing the code.
         // As such we compile the code in the benchmark loop as well.
         b.iter(|| {
-            let code = vm.compile(source, Mode::Exec, name.to_owned()).unwrap();
+            let code = vm.compile(source, Mode::Exec, name).unwrap();
             let scope = vm.new_scope_with_builtins();
             let res: PyResult = vm.run_code_obj(code.clone(), scope);
             vm.unwrap_pyresult(res);

benches/microbenchmarks.rs

@@ -118,10 +118,10 @@ fn bench_rustpython_code(group: &mut BenchmarkGroup<WallTime>, bench: &MicroBenc
     let interp = builder.add_native_modules(&defs).build();
     interp.enter(|vm| {
         let setup_code = vm
-            .compile(&bench.setup, Mode::Exec, bench.name.to_owned())
+            .compile(&bench.setup, Mode::Exec, &bench.name)
             .expect("Error compiling setup code");
         let bench_code = vm
-            .compile(&bench.code, Mode::Exec, bench.name.to_owned())
+            .compile(&bench.code, Mode::Exec, &bench.name)
             .expect("Error compiling bench code");
 
         let bench_func = |scope| {

crates/capi/src/ceval.rs

@@ -38,7 +38,7 @@ pub unsafe extern "C" fn Py_CompileString(
             }
         };
 
-        vm.compile(code, mode, filename.to_owned())
+        vm.compile(code, mode, filename)
             .map_err(|err| vm.new_syntax_error(&err, Some(code)))
     })
 }

crates/stdlib/src/_opcode.rs

@@ -192,7 +192,7 @@ mod tests {
     ///
     /// Memory addresses in the output are replaced with `0xdeadbeef` for consistency.
     fn dis(source: &str) -> String {
-        let fname = String::from("<?>");
+        const FNAME: &str = "<?>";
 
         let builder = vm::Interpreter::builder(Default::default());
         let stdlib_defs = crate::stdlib_module_defs(&builder.ctx);
@@ -204,7 +204,7 @@ mod tests {
         interp.enter(|vm| {
             let scope = vm.new_scope_with_builtins();
             let code_obj = vm
-                .compile(source.trim(), Mode::Exec, fname.clone())
+                .compile(source.trim(), Mode::Exec, FNAME)
                 .map_err(|err| vm.new_syntax_error(&err, Some(source)))
                 .unwrap();
             scope.globals.set_item("code", code_obj.into(), vm).unwrap();
@@ -227,7 +227,7 @@ output = re.sub(r'(<code object \w+ at )0x[0-9a-fA-F]+', r'\g<1>0xdeadbeef', tmp
 "#;
 
             let py_code_obj = vm
-                .compile(py_source, Mode::Exec, fname)
+                .compile(py_source, Mode::Exec, FNAME)
                 .map_err(|err| vm.new_syntax_error(&err, Some(py_source)))
                 .unwrap();
 

crates/vm/src/builtins/getset.rs

@@ -1,6 +1,5 @@
-/*! Python `attribute` descriptor class. (PyGetSet)
+//! Python `attribute` descriptor class. (PyGetSet)
 
-*/
 use super::PyType;
 use crate::{
     AsObject, Context, Py, PyObject, PyObjectRef, PyPayload, PyResult, VirtualMachine,
@@ -71,9 +70,9 @@ impl GetDescriptor for PyGetSet {
 
 impl PyGetSet {
     #[must_use]
-    pub fn new(name: String, class: &'static Py<PyType>) -> Self {
+    pub fn new(name: &str, class: &'static Py<PyType>) -> Self {
         Self {
-            name,
+            name: name.into(),
             class: PointerSlot::from(class),
             getter: None,
             setter: None,

crates/vm/src/eval.rs

@@ -1,7 +1,7 @@
 use crate::{PyResult, VirtualMachine, compiler, scope::Scope};
 
 pub fn eval(vm: &VirtualMachine, source: &str, scope: Scope, source_path: &str) -> PyResult {
-    match vm.compile(source, compiler::Mode::Eval, source_path.to_owned()) {
+    match vm.compile(source, compiler::Mode::Eval, source_path) {
         Ok(bytecode) => {
             debug!("Code object: {bytecode:?}");
             vm.run_code_obj(bytecode, scope)

crates/vm/src/import.rs

@@ -131,7 +131,7 @@ pub fn import_builtin(vm: &VirtualMachine, module_name: &str) -> PyResult {
 pub fn import_file(
     vm: &VirtualMachine,
     module_name: &str,
-    file_path: String,
+    file_path: &str,
     content: &str,
 ) -> PyResult {
     let code = vm
@@ -151,7 +151,7 @@ pub fn import_source(vm: &VirtualMachine, module_name: &str, content: &str) -> P
         .compile_with_opts(
             content,
             crate::compiler::Mode::Exec,
-            "<source>".to_owned(),
+            "<source>",
             vm.compile_opts(),
         )
         .map_err(|err| vm.new_syntax_error(&err, Some(content)))?;

crates/vm/src/stdlib/builtins.rs

@@ -424,12 +424,7 @@ mod builtins {
                         opts.optimize = optimize;
 
                         let code = vm
-                            .compile_with_opts(
-                                source,
-                                mode,
-                                filename.to_string_lossy().into_owned(),
-                                opts,
-                            )
+                            .compile_with_opts(source, mode, &filename.to_string_lossy(), opts)
                             .map_err(|err| {
                                 (err, Some(source), allow_incomplete).to_pyexception(vm)
                             })?;
@@ -632,7 +627,7 @@ mod builtins {
             #[cfg(feature = "rustpython-compiler")]
             Either::A(string) => {
                 let source = string.as_str();
-                vm.compile(source, mode, "<string>".to_owned())
+                vm.compile(source, mode, "<string>")
                     .map_err(|err| vm.new_syntax_error(&err, Some(source)))?
             }
             #[cfg(not(feature = "rustpython-compiler"))]

crates/vm/src/stdlib/sys.rs

@@ -750,7 +750,7 @@ pub mod sys {
         handle
             .read_to_string(&mut source)
             .map_err(|e| vm.new_os_error(format!("Error reading from stdin: {e}")))?;
-        vm.compile(&source, crate::compiler::Mode::Single, "<stdin>".to_owned())
+        vm.compile(&source, crate::compiler::Mode::Single, "<stdin>")
             .map_err(|e| vm.new_os_error(format!("Error running stdin: {e}")))?;
         Ok(())
     }

crates/vm/src/vm/compile.rs

@@ -13,7 +13,7 @@ impl VirtualMachine {
         &self,
         source: &str,
         mode: compiler::Mode,
-        source_path: String,
+        source_path: &str,
     ) -> Result<PyRef<PyCode>, CompileError> {
         self.compile_with_opts(source, mode, source_path, self.compile_opts())
     }
@@ -22,15 +22,17 @@ impl VirtualMachine {
         &self,
         source: &str,
         mode: compiler::Mode,
-        source_path: String,
+        source_path: &str,
         opts: CompileOpts,
     ) -> Result<PyRef<PyCode>, CompileError> {
-        let code = compiler::compile(source, mode, &source_path, opts)
+        let code = compiler::compile(source, mode, source_path, opts)
             .map(|code| PyCode::new_ref_from_bytecode(self, code));
+
         #[cfg(feature = "parser")]
         if code.is_ok() {
-            self.emit_string_escape_warnings(source, &source_path);
+            self.emit_string_escape_warnings(source, source_path);
         }
+
         code
     }
 }

crates/vm/src/vm/context.rs

@@ -333,9 +333,11 @@ impl Context {
                 )
             })
             .collect();
-        let latin1_char_cache: Vec<PyRef<PyStr>> = (0u8..=255)
+
+        let latin1_char_cache = (u8::MIN..=u8::MAX)
             .map(|b| create_object(PyStr::from(char::from(b)), types.str_type))
-            .collect();
+            .collect::<Vec<PyRef<PyStr>>>();
+
         let ascii_char_cache = latin1_char_cache[..128].to_vec();
 
         let true_value = create_object(PyBool(PyInt::from(1)), types.bool_type);
@@ -519,19 +521,20 @@ impl Context {
     fn latin1_singleton_index(s: &PyStr) -> Option<u8> {
         let mut cps = s.as_wtf8().code_points();
         let cp = cps.next()?;
-        if cps.next().is_some() {
-            return None;
-        }
-        u8::try_from(cp.to_u32()).ok()
+
+        cps.next()
+            .is_none()
+            .then(|| u8::try_from(cp.to_u32()).ok())?
     }
 
     #[inline]
     pub fn new_str(&self, s: impl Into<pystr::PyStr>) -> PyRef<PyStr> {
         let s = s.into();
         if let Some(ch) = Self::latin1_singleton_index(&s) {
-            return self.latin1_char(ch);
+            self.latin1_char(ch)
+        } else {
+            s.into_ref(self)
         }
-        s.into_ref(self)
     }
 
     #[inline]
@@ -544,9 +547,10 @@ impl Context {
         S: Into<PyStr> + AsRef<M>,
         M: MaybeInternedString,
     {
-        match self.interned_str(s.as_ref()) {
-            Some(s) => s.to_owned(),
-            None => self.new_str(s),
+        if let Some(s) = self.interned_str(s.as_ref()) {
+            s.to_owned()
+        } else {
+            self.new_str(s)
         }
     }
 
@@ -617,11 +621,7 @@ impl Context {
         name: &str,
         bases: Option<Vec<PyTypeRef>>,
     ) -> PyTypeRef {
-        let bases = if let Some(bases) = bases {
-            bases
-        } else {
-            vec![self.exceptions.exception_type.to_owned()]
-        };
+        let bases = bases.unwrap_or_else(|| vec![self.exceptions.exception_type.to_owned()]);
         let mut attrs = PyAttributes::default();
         attrs.insert(identifier!(self, __module__), self.new_str(module).into());
 
@@ -692,21 +692,20 @@ impl Context {
 
     pub fn new_readonly_getset<F, T>(
         &self,
-        name: impl Into<String>,
+        name: &str,
         class: &'static Py<PyType>,
         f: F,
     ) -> PyRef<PyGetSet>
     where
         F: IntoPyGetterFunc<T>,
     {
-        let name = name.into();
         let getset = PyGetSet::new(name, class).with_get(f);
         PyRef::new_ref(getset, self.types.getset_type.to_owned(), None)
     }
 
     pub fn new_static_getset<G, S, T, U>(
         &self,
-        name: impl Into<String>,
+        name: &str,
         class: &'static Py<PyType>,
         g: G,
         s: S,
@@ -715,19 +714,18 @@ impl Context {
         G: IntoPyGetterFunc<T>,
         S: IntoPySetterFunc<U>,
     {
-        let name = name.into();
         let getset = PyGetSet::new(name, class).with_get(g).with_set(s);
         PyRef::new_ref(getset, self.types.getset_type.to_owned(), None)
     }
 
-    /// Creates a new `PyGetSet` with a heap type.
+    /// Creates a new [`PyGetSet`] with a heap type.
     ///
     /// # Safety
     /// In practice, this constructor is safe because a getset is always owned by its `class` type.
     /// However, it can be broken if used unconventionally.
     pub unsafe fn new_getset<G, S, T, U>(
         &self,
-        name: impl Into<String>,
+        name: &str,
         class: &Py<PyType>,
         g: G,
         s: S,

crates/vm/src/vm/interpreter.rs

@@ -300,9 +300,7 @@ impl Default for InterpreterBuilder {
 ///     let scope = vm.new_scope_with_builtins();
 ///     let source = r#"print("Hello World!")"#;
 ///     let code_obj = vm.compile(
-///             source,
-///             Mode::Exec,
-///             "<embedded>".to_owned(),
+///         source, Mode::Exec, "<embedded>"
 ///     ).map_err(|err| vm.new_syntax_error(&err, Some(source))).unwrap();
 ///     vm.run_code_obj(code_obj, scope).unwrap();
 /// });

crates/vm/src/vm/mod.rs

@@ -2285,7 +2285,7 @@ mod tests {
 
                 let source = "from dir_module.dir_module_inner import value2";
                 let code_obj = vm
-                    .compile(source, vm::compiler::Mode::Exec, "<embedded>".to_owned())
+                    .compile(source, vm::compiler::Mode::Exec, "<embedded>")
                     .map_err(|err| vm.new_syntax_error(&err, Some(source)))
                     .unwrap();
 

crates/vm/src/vm/python_run.rs

@@ -13,13 +13,13 @@ impl VirtualMachine {
     /// Execute a string of Python code in a new scope with builtins.
     pub fn run_simple_string(&self, source: &str) -> PyResult {
         let scope = self.new_scope_with_builtins();
-        self.run_string(scope, source, "<string>".to_owned())
+        self.run_string(scope, source, "<string>")
     }
 
     /// PyRun_String
     ///
     /// Execute a string of Python code with explicit scope and source path.
-    pub fn run_string(&self, scope: Scope, source: &str, source_path: String) -> PyResult {
+    pub fn run_string(&self, scope: Scope, source: &str, source_path: &str) -> PyResult {
         let code_obj = self
             .compile(source, compiler::Mode::Exec, source_path)
             .map_err(|err| self.new_syntax_error(&err, Some(source)))?;
@@ -40,13 +40,13 @@ impl VirtualMachine {
     }
 
     #[deprecated(note = "use run_string instead")]
-    pub fn run_code_string(&self, scope: Scope, source: &str, source_path: String) -> PyResult {
+    pub fn run_code_string(&self, scope: Scope, source: &str, source_path: &str) -> PyResult {
         self.run_string(scope, source, source_path)
     }
 
     pub fn run_block_expr(&self, scope: Scope, source: &str) -> PyResult {
         let code_obj = self
-            .compile(source, compiler::Mode::BlockExpr, "<embedded>".to_owned())
+            .compile(source, compiler::Mode::BlockExpr, "<embedded>")
             .map_err(|err| self.new_syntax_error(&err, Some(source)))?;
         self.run_code_obj(code_obj, scope)
     }
@@ -108,7 +108,7 @@ mod file_run {
                 match crate::host_env::fs::read_to_string(path) {
                     Ok(source) => {
                         let code_obj = self
-                            .compile(&source, compiler::Mode::Exec, path.to_owned())
+                            .compile(&source, compiler::Mode::Exec, path)
                             .map_err(|err| self.new_syntax_error(&err, Some(&source)))?;
                         self.run_code_obj(code_obj, scope)?;
                     }

crates/vm/src/vm/thread.rs

@@ -2,10 +2,11 @@
 use super::FramePtr;
 #[cfg(feature = "threading")]
 use crate::builtins::PyBaseExceptionRef;
-use crate::frame::Frame;
-use crate::{AsObject, PyObject, VirtualMachine};
 #[cfg(feature = "threading")]
 use alloc::sync::Arc;
+
+use crate::frame::Frame;
+use crate::{AsObject, PyObject, VirtualMachine};
 use core::{
     cell::{Cell, RefCell},
     ptr::NonNull,
@@ -129,6 +130,7 @@ pub fn enter_vm<R>(vm: &VirtualMachine, f: impl FnOnce() -> R) -> R {
             detach_thread();
         }
     }
+
     set_current_vm(vm, f)
 }
 
@@ -538,6 +540,7 @@ pub fn cleanup_current_thread_frames(vm: &VirtualMachine) {
     } else {
         None
     };
+
     #[cfg(all(unix, feature = "threading"))]
     if let Some(slot) = &_removed
         && vm.state.stop_the_world.requested.load(Ordering::Acquire)

crates/wasm/src/vm_class.rs

@@ -263,7 +263,7 @@ impl WASMVirtualMachine {
     ) -> Result<(), JsValue> {
         self.with_vm(|vm, _| {
             let code = vm
-                .compile(source, Mode::Exec, name.clone())
+                .compile(source, Mode::Exec, &name)
                 .map_err(convert::syntax_err)?;
             let attrs = vm.ctx.new_dict();
             attrs
@@ -327,7 +327,7 @@ impl WASMVirtualMachine {
     ) -> Result<JsValue, JsValue> {
         self.with_vm(|vm, StoredVirtualMachine { scope, .. }| {
             let source_path = source_path.unwrap_or_else(|| "<wasm>".to_owned());
-            let code = vm.compile(source, mode, source_path);
+            let code = vm.compile(source, mode, &source_path);
             let code = code.map_err(convert::syntax_err)?;
             let result = vm.run_code_obj(code, scope.clone());
             convert::pyresult_to_js_result(vm, result)

examples/hello_embed.rs

@@ -5,7 +5,7 @@ fn main() -> vm::PyResult<()> {
         let scope = vm.new_scope_with_builtins();
         let source = r#"print("Hello World!")"#;
         let code_obj = vm
-            .compile(source, vm::compiler::Mode::Exec, "<embedded>".to_owned())
+            .compile(source, vm::compiler::Mode::Exec, "<embedded>")
             .map_err(|err| vm.new_syntax_error(&err, Some(source)))?;
 
         vm.run_code_obj(code_obj, scope)?;

examples/mini_repl.rs

@@ -65,7 +65,7 @@ def fib(n):
         // this line also automatically prints the output
         // (note that this is only the case when compiler::Mode::Single is passed to vm.compile)
         match vm
-            .compile(&input, vm::compiler::Mode::Single, "<embedded>".to_owned())
+            .compile(&input, vm::compiler::Mode::Single, "<embedded>")
             .map_err(|err| vm.new_syntax_error(&err, Some(&input)))
             .and_then(|code_obj| vm.run_code_obj(code_obj, scope.clone()))
         {

src/lib.rs

@@ -146,7 +146,7 @@ __import__("io").TextIOWrapper(
         .downcast()
         .expect("TextIOWrapper.read() should return str");
     eprintln!("running get-pip.py...");
-    vm.run_string(scope, getpip_code.expect_str(), "get-pip.py".to_owned())?;
+    vm.run_string(scope, getpip_code.expect_str(), "get-pip.py")?;
     Ok(())
 }
 
@@ -190,7 +190,7 @@ fn run_file(vm: &VirtualMachine, scope: Scope, path: &str) -> PyResult<()> {
             // The VM itself has no filesystem access.
             let path = if path.is_empty() { "???" } else { path };
             match std::fs::read_to_string(path) {
-                Ok(source) => vm.run_string(scope, &source, path.to_owned()).map(drop),
+                Ok(source) => vm.run_string(scope, &source, path).map(drop),
                 Err(err) => Err(vm.new_os_error(err.to_string())),
             }
         }
@@ -294,8 +294,7 @@ fn run_rustpython(vm: &VirtualMachine, run_mode: RunMode) -> PyResult<()> {
     let res = match run_mode {
         RunMode::Command(command) => {
             debug!("Running command {command}");
-            vm.run_string(scope.clone(), &command, "<string>".to_owned())
-                .map(drop)
+            vm.run_string(scope.clone(), &command, "<string>").map(drop)
         }
         RunMode::Module(module) => {
             debug!("Running module {module}");

src/shell.rs

@@ -32,7 +32,7 @@ fn shell_exec(
     // was actually compiled.
     #[cfg(windows)]
     let source = &source.replace("\r\n", "\n");
-    match vm.compile(source, compiler::Mode::Single, "<stdin>".to_owned()) {
+    match vm.compile(source, compiler::Mode::Single, "<stdin>") {
         Ok(code) => {
             if empty_line_given || !continuing_block {
                 // We want to execute the full code