Skip to content

Commit fe34737

Browse files
skinnyBatskinnyBat
committed
Avoid cloning within isinstance/issubclass calls.
1 parent 82f79b5 commit fe34737

14 files changed

Lines changed: 82 additions & 55 deletions

File tree

‎vm/src/builtins.rs‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -587,8 +587,8 @@ fn builtin_pow(
587587
}
588588
OptionalArg::Present(m) => {
589589
// Check if the 3rd argument is defined and perform modulus on the result
590-
if !(objtype::isinstance(&x, &vm.ctx.int_type())
591-
&& objtype::isinstance(&y, &vm.ctx.int_type()))
590+
if !(objtype::isinstance(&x, &vm.ctx.types.int_type)
591+
&& objtype::isinstance(&y, &vm.ctx.types.int_type))
592592
{
593593
return Err(vm.new_type_error(
594594
"pow() 3rd argument not allowed unless all arguments are integers".to_owned(),
@@ -928,9 +928,10 @@ pub fn builtin_build_class_(
928928
};
929929

930930
for base in bases.clone() {
931-
if objtype::issubclass(&base.class(), &metaclass) {
931+
let base_class = base.lease_class();
932+
if objtype::issubclass(&base_class, &metaclass) {
932933
metaclass = base.class();
933-
} else if !objtype::issubclass(&metaclass, &base.class()) {
934+
} else if !objtype::issubclass(&metaclass, &base_class) {
934935
return Err(vm.new_type_error(
935936
"metaclass conflict: the metaclass of a derived class must be a (non-strict) \
936937
subclass of the metaclasses of all its bases"

‎vm/src/obj/objbool.rs‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ impl IntoPyObject for bool {
2020

2121
impl TryFromObject for bool {
2222
fn try_from_object(vm: &VirtualMachine, obj: PyObjectRef) -> PyResult<bool> {
23-
if objtype::isinstance(&obj, &vm.ctx.int_type()) {
23+
if objtype::isinstance(&obj, &vm.ctx.types.int_type) {
2424
Ok(get_value(&obj))
2525
} else {
2626
Err(vm.new_type_error(format!("Expected type bool, not {}", obj.lease_class().name)))
@@ -41,7 +41,7 @@ pub fn boolval(vm: &VirtualMachine, obj: PyObjectRef) -> PyResult<bool> {
4141
// If descriptor returns Error, propagate it further
4242
let method = method_or_err?;
4343
let bool_obj = vm.invoke(&method, PyFuncArgs::default())?;
44-
if !objtype::isinstance(&bool_obj, &vm.ctx.bool_type()) {
44+
if !objtype::isinstance(&bool_obj, &vm.ctx.types.bool_type) {
4545
return Err(vm.new_type_error(format!(
4646
"__bool__ should return bool, returned type {}",
4747
bool_obj.lease_class().name
@@ -110,8 +110,8 @@ impl PyBool {
110110
#[pymethod(name = "__ror__")]
111111
#[pymethod(magic)]
112112
fn or(lhs: PyObjectRef, rhs: PyObjectRef, vm: &VirtualMachine) -> PyResult {
113-
if objtype::isinstance(&lhs, &vm.ctx.bool_type())
114-
&& objtype::isinstance(&rhs, &vm.ctx.bool_type())
113+
if objtype::isinstance(&lhs, &vm.ctx.types.bool_type)
114+
&& objtype::isinstance(&rhs, &vm.ctx.types.bool_type)
115115
{
116116
let lhs = get_value(&lhs);
117117
let rhs = get_value(&rhs);
@@ -124,8 +124,8 @@ impl PyBool {
124124
#[pymethod(name = "__rand__")]
125125
#[pymethod(magic)]
126126
fn and(lhs: PyObjectRef, rhs: PyObjectRef, vm: &VirtualMachine) -> PyResult {
127-
if objtype::isinstance(&lhs, &vm.ctx.bool_type())
128-
&& objtype::isinstance(&rhs, &vm.ctx.bool_type())
127+
if objtype::isinstance(&lhs, &vm.ctx.types.bool_type)
128+
&& objtype::isinstance(&rhs, &vm.ctx.types.bool_type)
129129
{
130130
let lhs = get_value(&lhs);
131131
let rhs = get_value(&rhs);
@@ -138,8 +138,8 @@ impl PyBool {
138138
#[pymethod(name = "__rxor__")]
139139
#[pymethod(magic)]
140140
fn xor(lhs: PyObjectRef, rhs: PyObjectRef, vm: &VirtualMachine) -> PyResult {
141-
if objtype::isinstance(&lhs, &vm.ctx.bool_type())
142-
&& objtype::isinstance(&rhs, &vm.ctx.bool_type())
141+
if objtype::isinstance(&lhs, &vm.ctx.types.bool_type)
142+
&& objtype::isinstance(&rhs, &vm.ctx.types.bool_type)
143143
{
144144
let lhs = get_value(&lhs);
145145
let rhs = get_value(&rhs);
@@ -151,7 +151,7 @@ impl PyBool {
151151

152152
#[pyslot]
153153
fn tp_new(zelf: PyObjectRef, x: OptionalArg<PyObjectRef>, vm: &VirtualMachine) -> PyResult {
154-
if !objtype::isinstance(&zelf, &vm.ctx.type_type()) {
154+
if !objtype::isinstance(&zelf, &vm.ctx.types.type_type) {
155155
let zelf_typ = zelf.class();
156156
let actual_type = vm.to_pystr(&zelf_typ)?;
157157
return Err(vm.new_type_error(format!(
@@ -172,7 +172,7 @@ pub(crate) fn init(context: &PyContext) {
172172
}
173173

174174
pub fn not(vm: &VirtualMachine, obj: &PyObjectRef) -> PyResult<bool> {
175-
if objtype::isinstance(obj, &vm.ctx.bool_type()) {
175+
if objtype::isinstance(obj, &vm.ctx.types.bool_type) {
176176
let value = get_value(obj);
177177
Ok(!value)
178178
} else {

‎vm/src/obj/objint.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -681,9 +681,9 @@ impl IntOptions {
681681
// FIXME: unnessessary bigint clone/creation
682682
let base = if let OptionalArg::Present(base) = self.base {
683683
if ![
684-
&vm.ctx.str_type(),
685-
&vm.ctx.bytes_type(),
686-
&vm.ctx.bytearray_type(),
684+
&vm.ctx.types.str_type,
685+
&vm.ctx.types.bytes_type,
686+
&vm.ctx.types.bytearray_type,
687687
]
688688
.iter()
689689
.any(|&typ| objtype::isinstance(&val, typ))

‎vm/src/obj/objmemory.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ impl PyMemoryView {
2626
bytes_object: PyObjectRef,
2727
vm: &VirtualMachine,
2828
) -> PyResult<PyMemoryViewRef> {
29-
let object_type = bytes_object.class();
29+
let object_type = bytes_object.lease_class();
3030

3131
if issubclass(&object_type, &vm.ctx.types.memoryview_type)
3232
|| issubclass(&object_type, &vm.ctx.types.bytes_type)

‎vm/src/obj/objset.rs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -798,8 +798,9 @@ struct SetIterable {
798798

799799
impl TryFromObject for SetIterable {
800800
fn try_from_object(vm: &VirtualMachine, obj: PyObjectRef) -> PyResult<Self> {
801-
if objtype::issubclass(&obj.class(), &vm.ctx.set_type())
802-
|| objtype::issubclass(&obj.class(), &vm.ctx.frozenset_type())
801+
let class = obj.lease_class();
802+
if objtype::issubclass(&class, &vm.ctx.set_type())
803+
|| objtype::issubclass(&class, &vm.ctx.frozenset_type())
803804
{
804805
Ok(SetIterable {
805806
iterable: Args::new(vec![PyIterable::try_from_object(vm, obj)?]),

‎vm/src/obj/objstr.rs‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,7 @@ impl PyString {
205205
}
206206
#[pymethod(name = "__add__")]
207207
fn add(&self, rhs: PyObjectRef, vm: &VirtualMachine) -> PyResult<String> {
208-
if objtype::isinstance(&rhs, &vm.ctx.str_type()) {
208+
if objtype::isinstance(&rhs, &vm.ctx.types.str_type) {
209209
Ok(format!("{}{}", self.value, borrow_value(&rhs)))
210210
} else {
211211
Err(vm.new_type_error(format!("Cannot add {} and {}", self, rhs)))
@@ -219,7 +219,7 @@ impl PyString {
219219

220220
#[pymethod(name = "__eq__")]
221221
fn eq(&self, rhs: PyObjectRef, vm: &VirtualMachine) -> PyObjectRef {
222-
if objtype::isinstance(&rhs, &vm.ctx.str_type()) {
222+
if objtype::isinstance(&rhs, &vm.ctx.types.str_type) {
223223
vm.new_bool(self.value == borrow_value(&rhs))
224224
} else {
225225
vm.ctx.not_implemented()
@@ -228,7 +228,7 @@ impl PyString {
228228

229229
#[pymethod(name = "__ne__")]
230230
fn ne(&self, rhs: PyObjectRef, vm: &VirtualMachine) -> PyObjectRef {
231-
if objtype::isinstance(&rhs, &vm.ctx.str_type()) {
231+
if objtype::isinstance(&rhs, &vm.ctx.types.str_type) {
232232
vm.new_bool(self.value != borrow_value(&rhs))
233233
} else {
234234
vm.ctx.not_implemented()
@@ -609,7 +609,7 @@ impl PyString {
609609
}
610610

611611
let zelf = &args.args[0];
612-
if !objtype::isinstance(&zelf, &vm.ctx.str_type()) {
612+
if !objtype::isinstance(&zelf, &vm.ctx.types.str_type) {
613613
let zelf_typ = zelf.class();
614614
let actual_type = vm.to_pystr(&zelf_typ)?;
615615
return Err(vm.new_type_error(format!(
@@ -1287,7 +1287,7 @@ fn call_object_format(vm: &VirtualMachine, argument: PyObjectRef, format_spec: &
12871287
let returned_type = vm.ctx.new_str(new_format_spec.to_owned());
12881288

12891289
let result = vm.call_method(&argument, "__format__", vec![returned_type])?;
1290-
if !objtype::isinstance(&result, &vm.ctx.str_type()) {
1290+
if !objtype::isinstance(&result, &vm.ctx.types.str_type) {
12911291
let result_type = result.class();
12921292
let actual_type = vm.to_pystr(&result_type)?;
12931293
return Err(vm.new_type_error(format!("__format__ must return a str, not {}", actual_type)));
@@ -1315,7 +1315,7 @@ fn do_cformat_specifier(
13151315
Ok(format_spec.format_string(clone_value(&result)))
13161316
}
13171317
CFormatType::Number(_) => {
1318-
if !objtype::isinstance(&obj, &vm.ctx.int_type()) {
1318+
if !objtype::isinstance(&obj, &vm.ctx.types.int_type) {
13191319
let required_type_string = match format_type {
13201320
CFormatType::Number(Decimal) => "a number",
13211321
CFormatType::Number(_) => "an integer",
@@ -1330,9 +1330,9 @@ fn do_cformat_specifier(
13301330
}
13311331
Ok(format_spec.format_number(objint::get_value(&obj)))
13321332
}
1333-
CFormatType::Float(_) => if objtype::isinstance(&obj, &vm.ctx.float_type()) {
1333+
CFormatType::Float(_) => if objtype::isinstance(&obj, &vm.ctx.types.float_type) {
13341334
format_spec.format_float(objfloat::get_value(&obj))
1335-
} else if objtype::isinstance(&obj, &vm.ctx.int_type()) {
1335+
} else if objtype::isinstance(&obj, &vm.ctx.types.int_type) {
13361336
format_spec.format_float(objint::get_value(&obj).to_f64().unwrap())
13371337
} else {
13381338
let required_type_string = "an floating point or integer";
@@ -1346,15 +1346,15 @@ fn do_cformat_specifier(
13461346
.map_err(|e| vm.new_not_implemented_error(e)),
13471347
CFormatType::Character => {
13481348
let char_string = {
1349-
if objtype::isinstance(&obj, &vm.ctx.int_type()) {
1349+
if objtype::isinstance(&obj, &vm.ctx.types.int_type) {
13501350
// BigInt truncation is fine in this case because only the unicode range is relevant
13511351
match objint::get_value(&obj).to_u32().and_then(char::from_u32) {
13521352
Some(value) => Ok(value.to_string()),
13531353
None => {
13541354
Err(vm.new_overflow_error("%c arg not in range(0x110000)".to_owned()))
13551355
}
13561356
}
1357-
} else if objtype::isinstance(&obj, &vm.ctx.str_type()) {
1357+
} else if objtype::isinstance(&obj, &vm.ctx.types.str_type) {
13581358
let s = borrow_value(&obj);
13591359
let num_chars = s.chars().count();
13601360
if num_chars != 1 {
@@ -1384,7 +1384,7 @@ fn try_update_quantity_from_tuple(
13841384
match elements.next() {
13851385
Some(width_obj) => {
13861386
tuple_index += 1;
1387-
if !objtype::isinstance(&width_obj, &vm.ctx.int_type()) {
1387+
if !objtype::isinstance(&width_obj, &vm.ctx.types.int_type) {
13881388
Err(vm.new_type_error("* wants int".to_owned()))
13891389
} else {
13901390
// TODO: handle errors when truncating BigInt to usize
@@ -1423,7 +1423,7 @@ pub fn do_cformat_string(
14231423
.all(|(_, part)| CFormatPart::has_key(part));
14241424

14251425
let values = if mapping_required {
1426-
if !objtype::isinstance(&values_obj, &vm.ctx.dict_type()) {
1426+
if !objtype::isinstance(&values_obj, &vm.ctx.types.dict_type) {
14271427
return Err(vm.new_type_error("format requires a mapping".to_owned()));
14281428
}
14291429
values_obj.clone()
@@ -1440,7 +1440,7 @@ pub fn do_cformat_string(
14401440
}
14411441

14421442
// convert `values_obj` to a new tuple if it's not a tuple
1443-
if !objtype::isinstance(&values_obj, &vm.ctx.tuple_type()) {
1443+
if !objtype::isinstance(&values_obj, &vm.ctx.types.tuple_type) {
14441444
vm.ctx.new_tuple(vec![values_obj.clone()])
14451445
} else {
14461446
values_obj.clone()

‎vm/src/obj/objsuper.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ impl PySuper {
101101
};
102102

103103
// Check type argument:
104-
if !objtype::isinstance(typ.as_object(), &vm.get_type()) {
104+
if !objtype::isinstance(typ.as_object(), &vm.ctx.types.type_type) {
105105
return Err(vm.new_type_error(format!(
106106
"super() argument 1 must be type, not {}",
107107
typ.lease_class().name

‎vm/src/obj/objtype.rs‎

Lines changed: 29 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,14 +10,12 @@ use super::objstr::PyStringRef;
1010
use super::objtuple::PyTuple;
1111
use super::objweakref::PyWeak;
1212
use crate::function::{OptionalArg, PyFuncArgs};
13-
use crate::pyobject::{
14-
IdProtocol, PyAttributes, PyClassImpl, PyContext, PyIterable, PyObject, PyObjectRef, PyRef,
15-
PyResult, PyValue, TypeProtocol,
16-
};
13+
use crate::pyobject::{IdProtocol, PyAttributes, PyClassImpl, PyContext, PyIterable, PyObject, PyObjectRef, PyRef, PyResult, PyValue, TypeProtocol, PyLease};
1714
use crate::slots::{PyClassSlots, PyTpFlags};
1815
use crate::vm::VirtualMachine;
1916
use arc_swap::ArcSwap;
2017
use itertools::Itertools;
18+
use std::ops::Deref;
2119

2220
/// type(object_or_name, bases, dict)
2321
/// type(object) -> the object's type
@@ -364,18 +362,40 @@ pub(crate) fn init(ctx: &PyContext) {
364362
PyClassRef::extend_class(ctx, &ctx.types.type_type);
365363
}
366364

365+
pub trait DerefToPyClass {
366+
fn deref_to_class(&self) -> &PyClass;
367+
}
368+
369+
impl DerefToPyClass for PyClassRef {
370+
fn deref_to_class(&self) -> &PyClass {
371+
self.deref()
372+
}
373+
}
374+
375+
impl DerefToPyClass for PyLease<PyClass> {
376+
fn deref_to_class(&self) -> &PyClass {
377+
self.deref()
378+
}
379+
}
380+
381+
impl<T: DerefToPyClass> DerefToPyClass for &'_ T {
382+
fn deref_to_class(&self) -> &PyClass {
383+
(&**self).deref_to_class()
384+
}
385+
}
386+
367387
/// Determines if `obj` actually an instance of `cls`, this doesn't call __instancecheck__, so only
368388
/// use this if `cls` is known to have not overridden the base __instancecheck__ magic method.
369389
#[inline]
370390
pub fn isinstance<T: TypeProtocol>(obj: &T, cls: &PyClassRef) -> bool {
371-
issubclass(&obj.class(), &cls)
391+
issubclass(obj.lease_class(), &cls)
372392
}
373393

374394
/// Determines if `subclass` is actually a subclass of `cls`, this doesn't call __subclasscheck__,
375395
/// so only use this if `cls` is known to have not overridden the base __subclasscheck__ magic
376396
/// method.
377-
pub fn issubclass(subclass: &PyClassRef, cls: &PyClassRef) -> bool {
378-
subclass.iter_mro().any(|c| c.is(cls))
397+
pub fn issubclass<T: DerefToPyClass + IdProtocol, R: IdProtocol>(subclass: T, cls: R) -> bool {
398+
subclass.is(&cls) || subclass.deref_to_class().mro.iter().any(|c| c.is(&cls))
379399
}
380400

381401
fn call_tp_new(
@@ -570,11 +590,11 @@ fn calculate_meta_class(
570590
// = _PyType_CalculateMetaclass
571591
let mut winner = metatype;
572592
for base in bases {
573-
let base_type = base.class();
593+
let base_type = base.lease_class();
574594
if issubclass(&winner, &base_type) {
575595
continue;
576596
} else if issubclass(&base_type, &winner) {
577-
winner = base_type.clone();
597+
winner = PyLease::into_pyref(base_type);
578598
continue;
579599
}
580600

‎vm/src/py_serde.rs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -66,13 +66,13 @@ impl<'s> serde::Serialize for PyObjectSerializer<'s> {
6666
}
6767
seq.end()
6868
};
69-
if objtype::isinstance(self.pyobject, &self.vm.ctx.str_type()) {
69+
if objtype::isinstance(self.pyobject, &self.vm.ctx.types.str_type) {
7070
serializer.serialize_str(objstr::borrow_value(&self.pyobject))
71-
} else if objtype::isinstance(self.pyobject, &self.vm.ctx.float_type()) {
71+
} else if objtype::isinstance(self.pyobject, &self.vm.ctx.types.float_type) {
7272
serializer.serialize_f64(objfloat::get_value(self.pyobject))
73-
} else if objtype::isinstance(self.pyobject, &self.vm.ctx.bool_type()) {
73+
} else if objtype::isinstance(self.pyobject, &self.vm.ctx.types.bool_type) {
7474
serializer.serialize_bool(objbool::get_value(self.pyobject))
75-
} else if objtype::isinstance(self.pyobject, &self.vm.ctx.int_type()) {
75+
} else if objtype::isinstance(self.pyobject, &self.vm.ctx.types.int_type) {
7676
let v = objint::get_value(self.pyobject);
7777
let int_too_large = || serde::ser::Error::custom("int too large to serialize");
7878
// TODO: serialize BigInt when it does not fit into i64
@@ -88,7 +88,7 @@ impl<'s> serde::Serialize for PyObjectSerializer<'s> {
8888
serialize_seq_elements(serializer, &list.borrow_elements())
8989
} else if let Some(tuple) = self.pyobject.payload_if_subclass::<PyTuple>(self.vm) {
9090
serialize_seq_elements(serializer, tuple.as_slice())
91-
} else if objtype::isinstance(self.pyobject, &self.vm.ctx.dict_type()) {
91+
} else if objtype::isinstance(self.pyobject, &self.vm.ctx.types.dict_type) {
9292
let dict: PyDictRef = self.pyobject.clone().downcast().unwrap();
9393
let pairs: Vec<_> = dict.into_iter().collect();
9494
let mut map = serializer.serialize_map(Some(pairs.len()))?;

‎vm/src/pyobject.rs‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -865,6 +865,11 @@ impl<T: PyObjectPayload> IdProtocol for PyLease<T> {
865865
}
866866
}
867867

868+
impl<T: IdProtocol> IdProtocol for &'_ T {
869+
fn get_id(&self) -> usize {
870+
(&**self).get_id()
871+
}
872+
}
868873

869874
pub struct PyLease<T: PyObjectPayload> {
870875
inner: arc_swap::Guard<'static, Arc<PyObject<T>>>
@@ -1254,7 +1259,7 @@ impl PyObject<dyn PyObjectPayload> {
12541259
&self,
12551260
vm: &VirtualMachine,
12561261
) -> Option<&T> {
1257-
if objtype::issubclass(&self.class(), &T::class(vm)) {
1262+
if objtype::issubclass(self.lease_class(), &T::class(vm)) {
12581263
self.payload()
12591264
} else {
12601265
None

0 commit comments

Comments
 (0)