From eba68330969f0da264766d71c46a71d8d9bbf70d Mon Sep 17 00:00:00 2001 From: Dennis Zhuang Date: Sat, 14 May 2022 16:18:28 +0800 Subject: [PATCH 1/2] PyStr::mul optimization for i=0 or str='' --- Lib/test/string_tests.py | 2 ++ vm/src/builtins/str.rs | 9 +++++++-- vm/src/vm/context.rs | 4 ++++ 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/Lib/test/string_tests.py b/Lib/test/string_tests.py index c82962195f..9051c6f258 100644 --- a/Lib/test/string_tests.py +++ b/Lib/test/string_tests.py @@ -1188,6 +1188,8 @@ class MixinStrUnicodeUserStringTest: slice(start, stop, step)) def test_mul(self): + self.assertTrue("('' * 3) is ''"); + self.assertTrue("('a' * 0) is ''"); self.checkequal('', 'abc', '__mul__', -1) self.checkequal('', 'abc', '__mul__', 0) self.checkequal('abc', 'abc', '__mul__', 1) diff --git a/vm/src/builtins/str.rs b/vm/src/builtins/str.rs index e24f700a13..911d19dda7 100644 --- a/vm/src/builtins/str.rs +++ b/vm/src/builtins/str.rs @@ -509,8 +509,13 @@ impl PyStr { #[pymethod(name = "__rmul__")] #[pymethod(magic)] fn mul(zelf: PyRef, value: isize, vm: &VirtualMachine) -> PyResult> { - if value == 1 && zelf.class().is(&vm.ctx.types.str_type) { - // Special case: when some `str` is multiplied by `1`, + if value == 0 && zelf.class().is(&vm.ctx.types.str_type) { + // Special case: when some `str` is multiplied by `0`, + // returns the empty `str`. + return Ok(vm.ctx.empty_str.clone()); + } + if (value == 1 || zelf.is_empty()) && zelf.class().is(&vm.ctx.types.str_type) { + // Special case: when some `str` is multiplied by `1` or is the empty `str`, // nothing really happens, we need to return an object itself // with the same `id()` to be compatible with CPython. // This only works for `str` itself, not its subclasses. diff --git a/vm/src/vm/context.rs b/vm/src/vm/context.rs index c08d809c24..d3d69dcf2a 100644 --- a/vm/src/vm/context.rs +++ b/vm/src/vm/context.rs @@ -25,6 +25,7 @@ pub struct Context { pub none: PyRef, pub empty_tuple: PyTupleRef, pub empty_frozenset: PyRef, + pub empty_str: PyRef, pub ellipsis: PyRef, pub not_implemented: PyRef, @@ -83,6 +84,7 @@ impl Context { let true_str = unsafe { string_pool.intern("True", types.str_type.clone()) }.into_pyref(); let false_str = unsafe { string_pool.intern("False", types.str_type.clone()) }.into_pyref(); + let empty_str = unsafe { string_pool.intern("", types.str_type.clone()) }.into_pyref(); let context = Context { true_value, @@ -90,6 +92,8 @@ impl Context { none, empty_tuple, empty_frozenset, + empty_str, + ellipsis, not_implemented, From 7cb391e140323d5a0330242c22d43e30cba74a2e Mon Sep 17 00:00:00 2001 From: Dennis Zhuang Date: Sat, 14 May 2022 21:45:55 +0800 Subject: [PATCH 2/2] Repalce all PyStr::from().into_ref(vm) with vm.ctx.empty_str --- vm/src/exceptions.rs | 2 +- vm/src/frame.rs | 2 +- vm/src/stdlib/builtins.rs | 2 +- vm/src/stdlib/io.rs | 6 +++--- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/vm/src/exceptions.rs b/vm/src/exceptions.rs index b4a9a85930..91bf73f160 100644 --- a/vm/src/exceptions.rs +++ b/vm/src/exceptions.rs @@ -507,7 +507,7 @@ impl PyBaseException { pub(super) fn str(&self, vm: &VirtualMachine) -> PyStrRef { let str_args = vm.exception_args_as_string(self.args(), true); match str_args.into_iter().exactly_one() { - Err(i) if i.len() == 0 => PyStr::from("").into_ref(vm), + Err(i) if i.len() == 0 => vm.ctx.empty_str.clone(), Ok(s) => s, Err(i) => PyStr::from(format!("({})", i.format(", "))).into_ref(vm), } diff --git a/vm/src/frame.rs b/vm/src/frame.rs index cd0d4cd5b9..6949ff1d8c 100644 --- a/vm/src/frame.rs +++ b/vm/src/frame.rs @@ -1125,7 +1125,7 @@ impl ExecutingFrame<'_> { #[cfg_attr(feature = "flame-it", flame("Frame"))] fn import(&mut self, vm: &VirtualMachine, module: Option) -> FrameResult { - let module = module.unwrap_or_else(|| PyStr::from("").into_ref(vm)); + let module = module.unwrap_or_else(|| vm.ctx.empty_str.clone()); let from_list = >>::try_from_object(vm, self.pop_value())?; let level = usize::try_from_object(vm, self.pop_value())?; diff --git a/vm/src/stdlib/builtins.rs b/vm/src/stdlib/builtins.rs index d7bfc94c70..33513b9ee6 100644 --- a/vm/src/stdlib/builtins.rs +++ b/vm/src/stdlib/builtins.rs @@ -305,7 +305,7 @@ mod builtins { ) -> PyResult { let format_spec = format_spec .into_option() - .unwrap_or_else(|| PyStr::from("").into_ref(vm)); + .unwrap_or_else(|| vm.ctx.empty_str.clone()); call_object_format(vm, value, None, format_spec.as_str()) } diff --git a/vm/src/stdlib/io.rs b/vm/src/stdlib/io.rs index a18dabb9b6..e7f9ca2dd3 100644 --- a/vm/src/stdlib/io.rs +++ b/vm/src/stdlib/io.rs @@ -2588,7 +2588,7 @@ mod _io { } } if chunks.is_empty() { - PyStr::from("").into_ref(vm) + vm.ctx.empty_str.clone() } else if chunks.len() == 1 { chunks.pop().unwrap() } else { @@ -2850,7 +2850,7 @@ mod _io { } else if let Some(cur_line) = cur_line { cur_line.slice_pystr(vm) } else { - PyStr::from("").into_ref(vm) + vm.ctx.empty_str.clone() }; Ok(line) } @@ -3009,7 +3009,7 @@ mod _io { append: Option, vm: &VirtualMachine, ) -> PyStrRef { - let empty_str = || PyStr::from("").into_ref(vm); + let empty_str = || vm.ctx.empty_str.clone(); let chars_pos = std::mem::take(&mut self.decoded_chars_used).bytes; let decoded_chars = match std::mem::take(&mut self.decoded_chars) { None => return append.unwrap_or_else(empty_str),