Skip to content

Commit 54c6c64

Browse files
Create properties hashmap once instead of on each call (#114)
This adds `once_cell` as a dependency, however, it is used for both handlers and properties. Better to be 'safe' than sorry ;)
1 parent 745cd13 commit 54c6c64

File tree

4 files changed

+61
-38
lines changed

4 files changed

+61
-38
lines changed

Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ exclude = ["/.github", "/.crates", "/guide"]
1515
bitflags = "1.2.1"
1616
parking_lot = "0.11.2"
1717
cfg-if = "1.0"
18+
once_cell = "1.8.0"
1819
anyhow = { version = "1", optional = true }
1920
ext-php-rs-derive = { version = "=0.7.2", path = "./crates/macros" }
2021

src/class.rs

Lines changed: 28 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,11 @@
33
use std::{
44
collections::HashMap,
55
marker::PhantomData,
6-
mem::MaybeUninit,
7-
sync::atomic::{AtomicBool, AtomicPtr, Ordering},
6+
sync::atomic::{AtomicPtr, Ordering},
87
};
98

9+
use once_cell::sync::OnceCell;
10+
1011
use crate::{
1112
builders::FunctionBuilder,
1213
exception::PhpException,
@@ -37,6 +38,10 @@ pub trait RegisteredClass: Sized + 'static {
3738
/// The key should be the name of the property and the value should be a
3839
/// reference to the property with reference to `self`. The value is a
3940
/// [`Property`].
41+
///
42+
/// Instead of using this method directly, you should access the properties
43+
/// through the [`ClassMetadata::get_properties`] function, which builds the
44+
/// hashmap one and stores it in memory.
4045
fn get_properties<'a>() -> HashMap<&'static str, Property<'a, Self>>;
4146
}
4247

@@ -81,8 +86,8 @@ impl<T> From<T> for ConstructorResult<T> {
8186
/// Stores the class entry and handlers for a Rust type which has been exported
8287
/// to PHP. Usually allocated statically.
8388
pub struct ClassMetadata<T> {
84-
handlers_init: AtomicBool,
85-
handlers: MaybeUninit<ZendObjectHandlers>,
89+
handlers: OnceCell<ZendObjectHandlers>,
90+
properties: OnceCell<HashMap<&'static str, Property<'static, T>>>,
8691
ce: AtomicPtr<ClassEntry>,
8792

8893
// `AtomicPtr` is used here because it is `Send + Sync`.
@@ -95,8 +100,8 @@ impl<T> ClassMetadata<T> {
95100
/// Creates a new class metadata instance.
96101
pub const fn new() -> Self {
97102
Self {
98-
handlers_init: AtomicBool::new(false),
99-
handlers: MaybeUninit::uninit(),
103+
handlers: OnceCell::new(),
104+
properties: OnceCell::new(),
100105
ce: AtomicPtr::new(std::ptr::null_mut()),
101106
phantom: PhantomData,
102107
}
@@ -107,10 +112,7 @@ impl<T: RegisteredClass> ClassMetadata<T> {
107112
/// Returns an immutable reference to the object handlers contained inside
108113
/// the class metadata.
109114
pub fn handlers(&self) -> &ZendObjectHandlers {
110-
self.check_handlers();
111-
112-
// SAFETY: `check_handlers` guarantees that `handlers` has been initialized.
113-
unsafe { &*self.handlers.as_ptr() }
115+
self.handlers.get_or_init(ZendObjectHandlers::new::<T>)
114116
}
115117

116118
/// Checks if the class entry has been stored, returning a boolean.
@@ -142,20 +144,23 @@ impl<T: RegisteredClass> ClassMetadata<T> {
142144
/// Panics if the class entry has already been set in the class metadata.
143145
/// This function should only be called once.
144146
pub fn set_ce(&self, ce: &'static mut ClassEntry) {
145-
if !self.ce.load(Ordering::SeqCst).is_null() {
146-
panic!("Class entry has already been set.");
147-
}
148-
149-
self.ce.store(ce, Ordering::SeqCst);
147+
self.ce
148+
.compare_exchange(
149+
std::ptr::null_mut(),
150+
ce,
151+
Ordering::SeqCst,
152+
Ordering::Relaxed,
153+
)
154+
.expect("Class entry has already been set");
150155
}
151156

152-
/// Checks if the handlers have been initialized, and initializes them if
153-
/// they are not.
154-
fn check_handlers(&self) {
155-
if !self.handlers_init.load(Ordering::SeqCst) {
156-
// SAFETY: `MaybeUninit` has the same size as the handlers.
157-
unsafe { ZendObjectHandlers::init::<T>(self.handlers.as_ptr() as *mut _) };
158-
self.handlers_init.store(true, Ordering::SeqCst);
159-
}
157+
/// Retrieves a reference to the hashmap storing the classes property
158+
/// accessors.
159+
///
160+
/// # Returns
161+
///
162+
/// Immutable reference to the properties hashmap.
163+
pub fn get_properties(&self) -> &HashMap<&'static str, Property<'static, T>> {
164+
self.properties.get_or_init(T::get_properties)
160165
}
161166
}

src/props.rs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -67,10 +67,10 @@ impl<'a, T: Clone + IntoZval + FromZval<'a>> Prop<'a> for T {
6767
/// * Method properties, where getter and/or setter functions are provided,
6868
/// which are used to get and set the value of the property.
6969
pub enum Property<'a, T> {
70-
Field(Box<dyn Fn(&mut T) -> &mut dyn Prop>),
70+
Field(Box<dyn (Fn(&mut T) -> &mut dyn Prop) + Send + Sync>),
7171
Method {
72-
get: Option<Box<dyn Fn(&T, &mut Zval) -> PhpResult + 'a>>,
73-
set: Option<Box<dyn Fn(&mut T, &Zval) -> PhpResult + 'a>>,
72+
get: Option<Box<dyn Fn(&T, &mut Zval) -> PhpResult + Send + Sync + 'a>>,
73+
set: Option<Box<dyn Fn(&mut T, &Zval) -> PhpResult + Send + Sync + 'a>>,
7474
},
7575
}
7676

@@ -93,9 +93,9 @@ impl<'a, T: 'a> Property<'a, T> {
9393
/// ```
9494
pub fn field<F>(f: F) -> Self
9595
where
96-
F: (Fn(&mut T) -> &mut dyn Prop) + 'static,
96+
F: (Fn(&mut T) -> &mut dyn Prop) + Send + Sync + 'static,
9797
{
98-
Self::Field(Box::new(f) as Box<dyn Fn(&mut T) -> &mut dyn Prop>)
98+
Self::Field(Box::new(f) as Box<dyn (Fn(&mut T) -> &mut dyn Prop) + Send + Sync>)
9999
}
100100

101101
/// Creates a method property with getters and setters.
@@ -139,7 +139,7 @@ impl<'a, T: 'a> Property<'a, T> {
139139
.set_zval(retval, false)
140140
.map_err(|e| format!("Failed to return property value to PHP: {:?}", e))?;
141141
Ok(())
142-
}) as Box<dyn Fn(&T, &mut Zval) -> PhpResult + 'a>
142+
}) as Box<dyn Fn(&T, &mut Zval) -> PhpResult + Send + Sync + 'a>
143143
});
144144

145145
let set = set.map(|set| {
@@ -148,7 +148,7 @@ impl<'a, T: 'a> Property<'a, T> {
148148
.ok_or("Unable to convert property value into required type.")?;
149149
set(self_, val);
150150
Ok(())
151-
}) as Box<dyn Fn(&mut T, &Zval) -> PhpResult + 'a>
151+
}) as Box<dyn Fn(&mut T, &Zval) -> PhpResult + Send + Sync + 'a>
152152
});
153153

154154
Self::Method { get, set }

src/zend/handlers.rs

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use std::{ffi::c_void, os::raw::c_int, ptr};
1+
use std::{ffi::c_void, mem::MaybeUninit, os::raw::c_int, ptr};
22

33
use crate::{
44
class::RegisteredClass,
@@ -16,6 +16,19 @@ use crate::{
1616
pub type ZendObjectHandlers = zend_object_handlers;
1717

1818
impl ZendObjectHandlers {
19+
/// Creates a new set of object handlers based on the standard object
20+
/// handlers.
21+
pub fn new<T: RegisteredClass>() -> ZendObjectHandlers {
22+
let mut this = MaybeUninit::uninit();
23+
24+
// SAFETY: `this` is allocated on the stack and is a valid memory location.
25+
unsafe { Self::init::<T>(&mut *this.as_mut_ptr()) };
26+
27+
// SAFETY: We just initialized the handlers in the previous statement, therefore
28+
// we are returning a valid object.
29+
unsafe { this.assume_init() }
30+
}
31+
1932
/// Initializes a given set of object handlers by copying the standard
2033
/// object handlers into the memory location, as well as setting up the
2134
/// `T` type destructor.
@@ -73,8 +86,12 @@ impl ZendObjectHandlers {
7386
.as_ref()
7487
.ok_or("Invalid property name pointer given")?;
7588
let self_ = &mut **obj;
76-
let mut props = T::get_properties();
77-
let prop = props.remove(prop_name.as_str().ok_or("Invalid property name given")?);
89+
let props = T::get_metadata().get_properties();
90+
let prop = props.get(
91+
prop_name
92+
.as_str()
93+
.ok_or("Invalid property name was given")?,
94+
);
7895

7996
// retval needs to be treated as initialized, so we set the type to null
8097
let rv_mut = rv.as_mut().ok_or("Invalid return zval given")?;
@@ -120,8 +137,8 @@ impl ZendObjectHandlers {
120137
.as_ref()
121138
.ok_or("Invalid property name pointer given")?;
122139
let self_ = &mut **obj;
123-
let mut props = T::get_properties();
124-
let prop = props.remove(prop_name.as_str().ok_or("Invalid property name given")?);
140+
let props = T::get_metadata().get_properties();
141+
let prop = props.get(prop_name.as_str().ok_or("Invalid property name given")?);
125142
let value_mut = value.as_mut().ok_or("Invalid return zval given")?;
126143

127144
Ok(match prop {
@@ -155,9 +172,9 @@ impl ZendObjectHandlers {
155172
.and_then(|obj| ZendClassObject::<T>::from_zend_obj_mut(obj))
156173
.ok_or("Invalid object pointer given")?;
157174
let self_ = &mut **obj;
158-
let struct_props = T::get_properties();
175+
let struct_props = T::get_metadata().get_properties();
159176

160-
for (name, val) in struct_props.into_iter() {
177+
for (name, val) in struct_props {
161178
let mut zv = Zval::new();
162179
if val.get(self_, &mut zv).is_err() {
163180
continue;
@@ -202,7 +219,7 @@ impl ZendObjectHandlers {
202219
let prop_name = member
203220
.as_ref()
204221
.ok_or("Invalid property name pointer given")?;
205-
let props = T::get_properties();
222+
let props = T::get_metadata().get_properties();
206223
let prop = props.get(prop_name.as_str().ok_or("Invalid property name given")?);
207224
let self_ = &mut **obj;
208225

0 commit comments

Comments
 (0)