Skip to content

Commit 44831bb

Browse files
youknowoneyouknowone
committed
AsMapping only with static reference
1 parent 6857384 commit 44831bb

File tree

6 files changed

+92
-82
lines changed

6 files changed

+92
-82
lines changed

‎vm/src/builtins/mappingproxy.rs

+1-1
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

+3-3
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

+36-42
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

+17-12
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

+2-2
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),

‎vm/src/types/slot.rs

+33-22
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ impl Default for PyTypeFlags {
140140
}
141141

142142
pub(crate) type GenericMethod = fn(&PyObject, FuncArgs, &VirtualMachine) -> PyResult;
143-
pub(crate) type AsMappingFunc = fn(&PyObject, &VirtualMachine) -> PyMappingMethods;
143+
pub(crate) type AsMappingFunc = fn(&PyObject, &VirtualMachine) -> &'static PyMappingMethods;
144144
pub(crate) type HashFunc = fn(&PyObject, &VirtualMachine) -> PyResult<PyHash>;
145145
// CallFunc = GenericMethod
146146
pub(crate) type GetattroFunc = fn(&PyObject, PyStrRef, &VirtualMachine) -> PyResult;
@@ -192,14 +192,14 @@ fn length_wrapper(obj: &PyObject, vm: &VirtualMachine) -> PyResult<usize> {
192192
Ok(len as usize)
193193
}
194194

195-
const fn new_as_mapping_wrapper(
195+
const fn new_as_mapping_generic(
196196
has_length: bool,
197197
has_subscript: bool,
198198
has_ass_subscript: bool,
199199
) -> PyMappingMethods {
200200
PyMappingMethods {
201201
length: if has_length {
202-
Some(|mapping, vm| length_wrapper(&mapping.obj, vm))
202+
Some(|mapping, vm| length_wrapper(mapping.obj, vm))
203203
} else {
204204
None
205205
},
@@ -237,34 +237,44 @@ const fn new_as_mapping_wrapper(
237237
}
238238
}
239239

240-
fn as_mapping_wrapper(zelf: &PyObject, vm: &VirtualMachine) -> PyMappingMethods {
241-
static MAPPING_METHODS: &[PyMappingMethods] = &[
242-
new_as_mapping_wrapper(false, false, false),
243-
new_as_mapping_wrapper(true, false, false),
244-
new_as_mapping_wrapper(false, true, false),
245-
new_as_mapping_wrapper(true, true, false),
246-
new_as_mapping_wrapper(false, false, true),
247-
new_as_mapping_wrapper(true, false, true),
248-
new_as_mapping_wrapper(false, true, true),
249-
new_as_mapping_wrapper(true, true, true),
250-
];
240+
pub(crate) fn static_as_mapping_generic(
241+
has_length: bool,
242+
has_subscript: bool,
243+
has_ass_subscript: bool,
244+
) -> &'static PyMappingMethods {
251245
const fn bool_int(v: bool) -> usize {
252246
if v {
253247
1
254248
} else {
255249
0
256250
}
257251
}
252+
253+
static MAPPING_METHODS: &[PyMappingMethods] = &[
254+
new_as_mapping_generic(false, false, false),
255+
new_as_mapping_generic(true, false, false),
256+
new_as_mapping_generic(false, true, false),
257+
new_as_mapping_generic(true, true, false),
258+
new_as_mapping_generic(false, false, true),
259+
new_as_mapping_generic(true, false, true),
260+
new_as_mapping_generic(false, true, true),
261+
new_as_mapping_generic(true, true, true),
262+
];
263+
264+
let key =
265+
bool_int(has_length) | (bool_int(has_subscript) << 1) | (bool_int(has_ass_subscript) << 2);
266+
267+
&MAPPING_METHODS[key]
268+
}
269+
270+
pub fn as_mapping_generic(zelf: &PyObject, vm: &VirtualMachine) -> &'static PyMappingMethods {
258271
let (has_length, has_subscript, has_ass_subscript) = (
259272
zelf.class().has_attr(identifier!(vm, __len__)),
260273
zelf.class().has_attr(identifier!(vm, __getitem__)),
261274
zelf.class().has_attr(identifier!(vm, __setitem__))
262275
| zelf.class().has_attr(identifier!(vm, __delitem__)),
263276
);
264-
let key = (bool_int(has_length) << 0)
265-
| (bool_int(has_subscript) << 1)
266-
| (bool_int(has_ass_subscript) << 2);
267-
MAPPING_METHODS[key].clone()
277+
static_as_mapping_generic(has_length, has_subscript, has_ass_subscript)
268278
}
269279

270280
fn as_sequence_wrapper(zelf: &PyObject, vm: &VirtualMachine) -> Cow<'static, PySequenceMethods> {
@@ -275,7 +285,7 @@ fn as_sequence_wrapper(zelf: &PyObject, vm: &VirtualMachine) -> Cow<'static, PyS
275285
Cow::Owned(PySequenceMethods {
276286
length: then_some_closure!(
277287
zelf.class().has_attr(identifier!(vm, __len__)),
278-
|seq, vm| { length_wrapper(&seq.obj, vm) }
288+
|seq, vm| { length_wrapper(seq.obj, vm) }
279289
),
280290
item: Some(|seq, i, vm| {
281291
vm.call_special_method(
@@ -420,7 +430,7 @@ impl PyType {
420430
}
421431
match name.as_str() {
422432
"__len__" | "__getitem__" | "__setitem__" | "__delitem__" => {
423-
update_slot!(as_mapping, as_mapping_wrapper);
433+
update_slot!(as_mapping, as_mapping_generic);
424434
update_slot!(as_sequence, as_sequence_wrapper);
425435
}
426436
"__hash__" => {
@@ -936,10 +946,11 @@ pub trait AsMapping: PyPayload {
936946

937947
#[inline]
938948
#[pyslot]
939-
fn as_mapping(_zelf: &PyObject, _vm: &VirtualMachine) -> PyMappingMethods {
940-
Self::AS_MAPPING
949+
fn as_mapping(_zelf: &PyObject, _vm: &VirtualMachine) -> &'static PyMappingMethods {
950+
&Self::AS_MAPPING
941951
}
942952

953+
#[inline]
943954
fn mapping_downcast<'a>(mapping: &'a PyMapping) -> &'a Py<Self> {
944955
unsafe { mapping.obj.downcast_unchecked_ref() }
945956
}

0 commit comments

Comments
 (0)