Skip to content

Commit 83a146d

Browse files
committed
AsMapping only with static reference
1 parent 6857384 commit 83a146d

File tree

6 files changed

+137
-122
lines changed

6 files changed

+137
-122
lines changed

vm/src/builtins/mappingproxy.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ impl Constructor for PyMappingProxy {
3939
type Args = PyObjectRef;
4040

4141
fn py_new(cls: PyTypeRef, mapping: Self::Args, vm: &VirtualMachine) -> PyResult {
42-
if !PyMapping::from(mapping.as_ref()).check(vm)
42+
if !PyMapping::check(mapping.as_ref(), vm)
4343
|| mapping.payload_if_subclass::<PyList>(vm).is_some()
4444
|| mapping.payload_if_subclass::<PyTuple>(vm).is_some()
4545
{

vm/src/function/protocol.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -103,15 +103,15 @@ where
103103
#[derive(Clone)]
104104
pub struct ArgMapping {
105105
obj: PyObjectRef,
106-
mapping_methods: PyMappingMethods,
106+
mapping_methods: &'static PyMappingMethods,
107107
}
108108

109109
impl ArgMapping {
110110
#[inline(always)]
111111
pub fn from_dict_exact(dict: PyDictRef) -> Self {
112112
Self {
113113
obj: dict.into(),
114-
mapping_methods: PyDict::AS_MAPPING,
114+
mapping_methods: &PyDict::AS_MAPPING,
115115
}
116116
}
117117

@@ -152,7 +152,7 @@ impl ToPyObject for ArgMapping {
152152
impl TryFromObject for ArgMapping {
153153
fn try_from_object(vm: &VirtualMachine, obj: PyObjectRef) -> PyResult<Self> {
154154
let mapping = PyMapping::try_protocol(&obj, vm)?;
155-
let mapping_methods = *mapping.methods(vm);
155+
let mapping_methods = mapping.methods;
156156
Ok(Self {
157157
obj,
158158
mapping_methods,

vm/src/protocol/mapping.rs

Lines changed: 36 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -3,36 +3,30 @@ use crate::{
33
dict::{PyDictItems, PyDictKeys, PyDictValues},
44
PyDict, PyStrInterned,
55
},
6-
common::lock::OnceCell,
76
convert::ToPyResult,
87
AsObject, PyObject, PyObjectRef, PyResult, VirtualMachine,
98
};
109

1110
// Mapping protocol
1211
// https://docs.python.org/3/c-api/mapping.html
1312
#[allow(clippy::type_complexity)]
14-
#[derive(Default, Copy, Clone)]
1513
pub struct PyMappingMethods {
1614
pub length: Option<fn(&PyMapping, &VirtualMachine) -> PyResult<usize>>,
1715
pub subscript: Option<fn(&PyMapping, &PyObject, &VirtualMachine) -> PyResult>,
1816
pub ass_subscript:
1917
Option<fn(&PyMapping, &PyObject, Option<PyObjectRef>, &VirtualMachine) -> PyResult<()>>,
2018
}
2119

20+
impl PyMappingMethods {
21+
fn check(&self) -> bool {
22+
self.subscript.is_some()
23+
}
24+
}
25+
2226
#[derive(Clone)]
2327
pub struct PyMapping<'a> {
2428
pub obj: &'a PyObject,
25-
methods: OnceCell<PyMappingMethods>,
26-
}
27-
28-
impl<'a> From<&'a PyObject> for PyMapping<'a> {
29-
#[inline(always)]
30-
fn from(obj: &'a PyObject) -> Self {
31-
Self {
32-
obj,
33-
methods: OnceCell::new(),
34-
}
35-
}
29+
pub methods: &'static PyMappingMethods,
3630
}
3731

3832
impl AsRef<PyObject> for PyMapping<'_> {
@@ -43,47 +37,47 @@ impl AsRef<PyObject> for PyMapping<'_> {
4337
}
4438

4539
impl<'a> PyMapping<'a> {
40+
#[inline]
41+
pub fn new(obj: &'a PyObject, vm: &VirtualMachine) -> Option<Self> {
42+
let methods = Self::find_methods(obj, vm)?;
43+
Some(Self { obj, methods })
44+
}
45+
4646
#[inline(always)]
47-
pub fn with_methods(obj: &'a PyObject, methods: PyMappingMethods) -> Self {
48-
Self {
49-
obj,
50-
methods: OnceCell::from(methods),
51-
}
47+
pub fn with_methods(obj: &'a PyObject, methods: &'static PyMappingMethods) -> Self {
48+
Self { obj, methods }
5249
}
5350

5451
pub fn try_protocol(obj: &'a PyObject, vm: &VirtualMachine) -> PyResult<Self> {
55-
let zelf = Self::from(obj);
56-
if zelf.check(vm) {
57-
Ok(zelf)
58-
} else {
59-
Err(vm.new_type_error(format!("{} is not a mapping object", zelf.obj.class())))
52+
if let Some(methods) = Self::find_methods(obj, vm) {
53+
if methods.check() {
54+
return Ok(Self::with_methods(obj, methods));
55+
}
6056
}
57+
58+
Err(vm.new_type_error(format!("{} is not a mapping object", obj.class())))
6159
}
6260
}
6361

6462
impl PyMapping<'_> {
6563
// PyMapping::Check
6664
#[inline]
67-
pub fn check(&self, vm: &VirtualMachine) -> bool {
68-
self.methods(vm).subscript.is_some()
69-
}
70-
71-
pub fn methods(&self, vm: &VirtualMachine) -> &PyMappingMethods {
72-
self.methods.get_or_init(|| {
73-
if let Some(f) = self
74-
.obj
75-
.class()
76-
.mro_find_map(|cls| cls.slots.as_mapping.load())
77-
{
78-
f(self.obj, vm)
79-
} else {
80-
PyMappingMethods::default()
81-
}
82-
})
65+
pub fn check(obj: &PyObject, vm: &VirtualMachine) -> bool {
66+
Self::find_methods(obj, vm)
67+
.and_then(|m| m.subscript)
68+
.is_some()
69+
}
70+
71+
pub fn find_methods(obj: &PyObject, vm: &VirtualMachine) -> Option<&'static PyMappingMethods> {
72+
if let Some(f) = obj.class().mro_find_map(|cls| cls.slots.as_mapping.load()) {
73+
Some(f(obj, vm))
74+
} else {
75+
None
76+
}
8377
}
8478

8579
pub fn length_opt(&self, vm: &VirtualMachine) -> Option<PyResult<usize>> {
86-
self.methods(vm).length.map(|f| f(self, vm))
80+
self.methods.length.map(|f| f(self, vm))
8781
}
8882

8983
pub fn length(&self, vm: &VirtualMachine) -> PyResult<usize> {
@@ -110,7 +104,7 @@ impl PyMapping<'_> {
110104

111105
fn _subscript(&self, needle: &PyObject, vm: &VirtualMachine) -> PyResult {
112106
let f = self
113-
.methods(vm)
107+
.methods
114108
.subscript
115109
.ok_or_else(|| vm.new_type_error(format!("{} is not a mapping", self.obj.class())))?;
116110
f(self, needle, vm)
@@ -122,7 +116,7 @@ impl PyMapping<'_> {
122116
value: Option<PyObjectRef>,
123117
vm: &VirtualMachine,
124118
) -> PyResult<()> {
125-
let f = self.methods(vm).ass_subscript.ok_or_else(|| {
119+
let f = self.methods.ass_subscript.ok_or_else(|| {
126120
vm.new_type_error(format!(
127121
"'{}' object does not support item assignment",
128122
self.obj.class()

vm/src/protocol/object.rs

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -524,7 +524,7 @@ impl PyObject {
524524
pub fn length_opt(&self, vm: &VirtualMachine) -> Option<PyResult<usize>> {
525525
PySequence::from(self)
526526
.length_opt(vm)
527-
.or_else(|| PyMapping::from(self).length_opt(vm))
527+
.or_else(|| PyMapping::new(self, vm).and_then(|mapping| mapping.length_opt(vm)))
528528
}
529529

530530
pub fn length(&self, vm: &VirtualMachine) -> PyResult<usize> {
@@ -570,13 +570,15 @@ impl PyObject {
570570
return dict.set_item(needle, value, vm);
571571
}
572572

573-
let mapping = PyMapping::from(self);
574-
let seq = PySequence::from(self);
573+
if let Some(mapping) = PyMapping::new(self, vm) {
574+
if let Some(f) = mapping.methods.ass_subscript {
575+
let needle = needle.to_pyobject(vm);
576+
return f(&mapping, &needle, Some(value), vm);
577+
}
578+
}
575579

576-
if let Some(f) = mapping.methods(vm).ass_subscript {
577-
let needle = needle.to_pyobject(vm);
578-
f(&mapping, &needle, Some(value), vm)
579-
} else if let Some(f) = seq.methods(vm).ass_item {
580+
let seq = PySequence::from(self);
581+
if let Some(f) = seq.methods(vm).ass_item {
580582
let i = needle.key_as_isize(vm)?;
581583
f(&seq, i, Some(value), vm)
582584
} else {
@@ -592,13 +594,16 @@ impl PyObject {
592594
return dict.del_item(needle, vm);
593595
}
594596

595-
let mapping = PyMapping::from(self);
597+
if let Some(mapping) = PyMapping::new(self, vm) {
598+
if let Some(f) = mapping.methods.ass_subscript {
599+
let needle = needle.to_pyobject(vm);
600+
return f(&mapping, &needle, None, vm);
601+
}
602+
}
603+
596604
let seq = PySequence::from(self);
597605

598-
if let Some(f) = mapping.methods(vm).ass_subscript {
599-
let needle = needle.to_pyobject(vm);
600-
f(&mapping, &needle, None, vm)
601-
} else if let Some(f) = seq.methods(vm).ass_item {
606+
if let Some(f) = seq.methods(vm).ass_item {
602607
let i = needle.key_as_isize(vm)?;
603608
f(&seq, i, None, vm)
604609
} else {

vm/src/protocol/sequence.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -242,8 +242,8 @@ impl PySequence<'_> {
242242
value: Option<PyObjectRef>,
243243
vm: &VirtualMachine,
244244
) -> PyResult<()> {
245-
let mapping = PyMapping::from(self.obj);
246-
if let Some(f) = mapping.methods(vm).ass_subscript {
245+
let mapping = PyMapping::new(self.obj, vm).unwrap();
246+
if let Some(f) = mapping.methods.ass_subscript {
247247
let slice = PySlice {
248248
start: Some(start.to_pyobject(vm)),
249249
stop: stop.to_pyobject(vm),

0 commit comments

Comments
 (0)