PySliceableSequence for PyString reuse len cache

youknowone · youknowone · commit 234c015146aa · 2020-07-25T22:08:09.000+09:00
diff --git a/vm/src/obj/objstr.rs b/vm/src/obj/objstr.rs
@@ -253,7 +253,7 @@ impl PyString {
                 }
             }
             SequenceIndex::Slice(slice) => {
-                let string = self.value.get_slice_items(vm, &slice)?;
+                let string = self.get_slice_items(vm, &slice)?;
                 Ok(vm.new_str(string))
             }
         }
@@ -1206,38 +1206,40 @@ pub fn borrow_value(obj: &PyObjectRef) -> &str {
     &obj.payload::<PyString>().unwrap().value
 }
 
-impl PySliceableSequence for str {
+impl PySliceableSequence for PyString {
     type Sliced = String;
 
     fn do_slice(&self, range: Range<usize>) -> Self::Sliced {
-        self.chars()
+        self.value
+            .chars()
             .skip(range.start)
             .take(range.end - range.start)
             .collect()
     }
 
     fn do_slice_reverse(&self, range: Range<usize>) -> Self::Sliced {
-        let count = self.chars().count();
-
-        self.chars()
+        let count = self.len();
+        self.value
+            .chars()
             .rev()
             .skip(count - range.end)
             .take(range.end - range.start)
             .collect()
     }
 
     fn do_stepped_slice(&self, range: Range<usize>, step: usize) -> Self::Sliced {
-        self.chars()
+        self.value
+            .chars()
             .skip(range.start)
             .take(range.end - range.start)
             .step_by(step)
             .collect()
     }
 
     fn do_stepped_slice_reverse(&self, range: Range<usize>, step: usize) -> Self::Sliced {
-        let count = self.chars().count();
-
-        self.chars()
+        let count = self.len();
+        self.value
+            .chars()
             .rev()
             .skip(count - range.end)
             .take(range.end - range.start)
@@ -1246,15 +1248,15 @@ impl PySliceableSequence for str {
     }
 
     fn empty() -> Self::Sliced {
-        String::default()
+        String::new()
     }
 
     fn len(&self) -> usize {
-        self.chars().count()
+        self.len()
     }
 
     fn is_empty(&self) -> bool {
-        self.is_empty()
+        self.value.is_empty()
     }
 }
 

Original file line number	Diff line number	Diff line change
`@@ -253,7 +253,7 @@ impl PyString {`
`253`	`253`	`}`
`254`	`254`	`}`
`255`	`255`	`SequenceIndex::Slice(slice) => {`
`256`		`- let string = self.value.get_slice_items(vm, &slice)?;`
	`256`	`+ let string = self.get_slice_items(vm, &slice)?;`
`257`	`257`	`Ok(vm.new_str(string))`
`258`	`258`	`}`
`259`	`259`	`}`
`@@ -1206,38 +1206,40 @@ pub fn borrow_value(obj: &PyObjectRef) -> &str {`
`1206`	`1206`	`&obj.payload::<PyString>().unwrap().value`
`1207`	`1207`	`}`
`1208`	`1208`
`1209`		`-impl PySliceableSequence for str {`
	`1209`	`+impl PySliceableSequence for PyString {`
`1210`	`1210`	`type Sliced = String;`
`1211`	`1211`
`1212`	`1212`	`fn do_slice(&self, range: Range<usize>) -> Self::Sliced {`
`1213`		`- self.chars()`
	`1213`	`+ self.value`
	`1214`	`+ .chars()`
`1214`	`1215`	`.skip(range.start)`
`1215`	`1216`	`.take(range.end - range.start)`
`1216`	`1217`	`.collect()`
`1217`	`1218`	`}`
`1218`	`1219`
`1219`	`1220`	`fn do_slice_reverse(&self, range: Range<usize>) -> Self::Sliced {`
`1220`		`- let count = self.chars().count();`
`1221`		`-`
`1222`		`- self.chars()`
	`1221`	`+ let count = self.len();`
	`1222`	`+ self.value`
	`1223`	`+ .chars()`
`1223`	`1224`	`.rev()`
`1224`	`1225`	`.skip(count - range.end)`
`1225`	`1226`	`.take(range.end - range.start)`
`1226`	`1227`	`.collect()`
`1227`	`1228`	`}`
`1228`	`1229`
`1229`	`1230`	`fn do_stepped_slice(&self, range: Range<usize>, step: usize) -> Self::Sliced {`
`1230`		`- self.chars()`
	`1231`	`+ self.value`
	`1232`	`+ .chars()`
`1231`	`1233`	`.skip(range.start)`
`1232`	`1234`	`.take(range.end - range.start)`
`1233`	`1235`	`.step_by(step)`
`1234`	`1236`	`.collect()`
`1235`	`1237`	`}`
`1236`	`1238`
`1237`	`1239`	`fn do_stepped_slice_reverse(&self, range: Range<usize>, step: usize) -> Self::Sliced {`
`1238`		`- let count = self.chars().count();`
`1239`		`-`
`1240`		`- self.chars()`
	`1240`	`+ let count = self.len();`
	`1241`	`+ self.value`
	`1242`	`+ .chars()`
`1241`	`1243`	`.rev()`
`1242`	`1244`	`.skip(count - range.end)`
`1243`	`1245`	`.take(range.end - range.start)`
`@@ -1246,15 +1248,15 @@ impl PySliceableSequence for str {`
`1246`	`1248`	`}`
`1247`	`1249`
`1248`	`1250`	`fn empty() -> Self::Sliced {`
`1249`		`- String::default()`
	`1251`	`+ String::new()`
`1250`	`1252`	`}`
`1251`	`1253`
`1252`	`1254`	`fn len(&self) -> usize {`
`1253`		`- self.chars().count()`
	`1255`	`+ self.len()`
`1254`	`1256`	`}`
`1255`	`1257`
`1256`	`1258`	`fn is_empty(&self) -> bool {`
`1257`		`- self.is_empty()`
	`1259`	`+ self.value.is_empty()`
`1258`	`1260`	`}`
`1259`	`1261`	`}`
`1260`	`1262`