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 ;)

Author: davidcole1340

Cargo.toml

@@ -15,6 +15,7 @@ exclude = ["/.github", "/.crates", "/guide"]
 bitflags = "1.2.1"
 parking_lot = "0.11.2"
 cfg-if = "1.0"
+once_cell = "1.8.0"
 anyhow = { version = "1", optional = true }
 ext-php-rs-derive = { version = "=0.7.2", path = "./crates/macros" }
 

src/class.rs

@@ -3,10 +3,11 @@
 use std::{
     collections::HashMap,
     marker::PhantomData,
-    mem::MaybeUninit,
-    sync::atomic::{AtomicBool, AtomicPtr, Ordering},
+    sync::atomic::{AtomicPtr, Ordering},
 };
 
+use once_cell::sync::OnceCell;
+
 use crate::{
     builders::FunctionBuilder,
     exception::PhpException,
@@ -37,6 +38,10 @@ pub trait RegisteredClass: Sized + 'static {
     /// The key should be the name of the property and the value should be a
     /// reference to the property with reference to `self`. The value is a
     /// [`Property`].
+    ///
+    /// Instead of using this method directly, you should access the properties
+    /// through the [`ClassMetadata::get_properties`] function, which builds the
+    /// hashmap one and stores it in memory.
     fn get_properties<'a>() -> HashMap<&'static str, Property<'a, Self>>;
 }
 
@@ -81,8 +86,8 @@ impl<T> From<T> for ConstructorResult<T> {
 /// Stores the class entry and handlers for a Rust type which has been exported
 /// to PHP. Usually allocated statically.
 pub struct ClassMetadata<T> {
-    handlers_init: AtomicBool,
-    handlers: MaybeUninit<ZendObjectHandlers>,
+    handlers: OnceCell<ZendObjectHandlers>,
+    properties: OnceCell<HashMap<&'static str, Property<'static, T>>>,
     ce: AtomicPtr<ClassEntry>,
 
     // `AtomicPtr` is used here because it is `Send + Sync`.
@@ -95,8 +100,8 @@ impl<T> ClassMetadata<T> {
     /// Creates a new class metadata instance.
     pub const fn new() -> Self {
         Self {
-            handlers_init: AtomicBool::new(false),
-            handlers: MaybeUninit::uninit(),
+            handlers: OnceCell::new(),
+            properties: OnceCell::new(),
             ce: AtomicPtr::new(std::ptr::null_mut()),
             phantom: PhantomData,
         }
@@ -107,10 +112,7 @@ impl<T: RegisteredClass> ClassMetadata<T> {
     /// Returns an immutable reference to the object handlers contained inside
     /// the class metadata.
     pub fn handlers(&self) -> &ZendObjectHandlers {
-        self.check_handlers();
-
-        // SAFETY: `check_handlers` guarantees that `handlers` has been initialized.
-        unsafe { &*self.handlers.as_ptr() }
+        self.handlers.get_or_init(ZendObjectHandlers::new::<T>)
     }
 
     /// Checks if the class entry has been stored, returning a boolean.
@@ -142,20 +144,23 @@ impl<T: RegisteredClass> ClassMetadata<T> {
     /// Panics if the class entry has already been set in the class metadata.
     /// This function should only be called once.
     pub fn set_ce(&self, ce: &'static mut ClassEntry) {
-        if !self.ce.load(Ordering::SeqCst).is_null() {
-            panic!("Class entry has already been set.");
-        }
-
-        self.ce.store(ce, Ordering::SeqCst);
+        self.ce
+            .compare_exchange(
+                std::ptr::null_mut(),
+                ce,
+                Ordering::SeqCst,
+                Ordering::Relaxed,
+            )
+            .expect("Class entry has already been set");
     }
 
-    /// Checks if the handlers have been initialized, and initializes them if
-    /// they are not.
-    fn check_handlers(&self) {
-        if !self.handlers_init.load(Ordering::SeqCst) {
-            // SAFETY: `MaybeUninit` has the same size as the handlers.
-            unsafe { ZendObjectHandlers::init::<T>(self.handlers.as_ptr() as *mut _) };
-            self.handlers_init.store(true, Ordering::SeqCst);
-        }
+    /// Retrieves a reference to the hashmap storing the classes property
+    /// accessors.
+    ///
+    /// # Returns
+    ///
+    /// Immutable reference to the properties hashmap.
+    pub fn get_properties(&self) -> &HashMap<&'static str, Property<'static, T>> {
+        self.properties.get_or_init(T::get_properties)
     }
 }

src/props.rs

@@ -67,10 +67,10 @@ impl<'a, T: Clone + IntoZval + FromZval<'a>> Prop<'a> for T {
 /// * Method properties, where getter and/or setter functions are provided,
 ///   which are used to get and set the value of the property.
 pub enum Property<'a, T> {
-    Field(Box<dyn Fn(&mut T) -> &mut dyn Prop>),
+    Field(Box<dyn (Fn(&mut T) -> &mut dyn Prop) + Send + Sync>),
     Method {
-        get: Option<Box<dyn Fn(&T, &mut Zval) -> PhpResult + 'a>>,
-        set: Option<Box<dyn Fn(&mut T, &Zval) -> PhpResult + 'a>>,
+        get: Option<Box<dyn Fn(&T, &mut Zval) -> PhpResult + Send + Sync + 'a>>,
+        set: Option<Box<dyn Fn(&mut T, &Zval) -> PhpResult + Send + Sync + 'a>>,
     },
 }
 
@@ -93,9 +93,9 @@ impl<'a, T: 'a> Property<'a, T> {
     /// ```
     pub fn field<F>(f: F) -> Self
     where
-        F: (Fn(&mut T) -> &mut dyn Prop) + 'static,
+        F: (Fn(&mut T) -> &mut dyn Prop) + Send + Sync + 'static,
     {
-        Self::Field(Box::new(f) as Box<dyn Fn(&mut T) -> &mut dyn Prop>)
+        Self::Field(Box::new(f) as Box<dyn (Fn(&mut T) -> &mut dyn Prop) + Send + Sync>)
     }
 
     /// Creates a method property with getters and setters.
@@ -139,7 +139,7 @@ impl<'a, T: 'a> Property<'a, T> {
                     .set_zval(retval, false)
                     .map_err(|e| format!("Failed to return property value to PHP: {:?}", e))?;
                 Ok(())
-            }) as Box<dyn Fn(&T, &mut Zval) -> PhpResult + 'a>
+            }) as Box<dyn Fn(&T, &mut Zval) -> PhpResult + Send + Sync + 'a>
         });
 
         let set = set.map(|set| {
@@ -148,7 +148,7 @@ impl<'a, T: 'a> Property<'a, T> {
                     .ok_or("Unable to convert property value into required type.")?;
                 set(self_, val);
                 Ok(())
-            }) as Box<dyn Fn(&mut T, &Zval) -> PhpResult + 'a>
+            }) as Box<dyn Fn(&mut T, &Zval) -> PhpResult + Send + Sync + 'a>
         });
 
         Self::Method { get, set }

src/zend/handlers.rs

@@ -1,4 +1,4 @@
-use std::{ffi::c_void, os::raw::c_int, ptr};
+use std::{ffi::c_void, mem::MaybeUninit, os::raw::c_int, ptr};
 
 use crate::{
     class::RegisteredClass,
@@ -16,6 +16,19 @@ use crate::{
 pub type ZendObjectHandlers = zend_object_handlers;
 
 impl ZendObjectHandlers {
+    /// Creates a new set of object handlers based on the standard object
+    /// handlers.
+    pub fn new<T: RegisteredClass>() -> ZendObjectHandlers {
+        let mut this = MaybeUninit::uninit();
+
+        // SAFETY: `this` is allocated on the stack and is a valid memory location.
+        unsafe { Self::init::<T>(&mut *this.as_mut_ptr()) };
+
+        // SAFETY: We just initialized the handlers in the previous statement, therefore
+        // we are returning a valid object.
+        unsafe { this.assume_init() }
+    }
+
     /// Initializes a given set of object handlers by copying the standard
     /// object handlers into the memory location, as well as setting up the
     /// `T` type destructor.
@@ -73,8 +86,12 @@ impl ZendObjectHandlers {
                 .as_ref()
                 .ok_or("Invalid property name pointer given")?;
             let self_ = &mut **obj;
-            let mut props = T::get_properties();
-            let prop = props.remove(prop_name.as_str().ok_or("Invalid property name given")?);
+            let props = T::get_metadata().get_properties();
+            let prop = props.get(
+                prop_name
+                    .as_str()
+                    .ok_or("Invalid property name was given")?,
+            );
 
             // retval needs to be treated as initialized, so we set the type to null
             let rv_mut = rv.as_mut().ok_or("Invalid return zval given")?;
@@ -120,8 +137,8 @@ impl ZendObjectHandlers {
                 .as_ref()
                 .ok_or("Invalid property name pointer given")?;
             let self_ = &mut **obj;
-            let mut props = T::get_properties();
-            let prop = props.remove(prop_name.as_str().ok_or("Invalid property name given")?);
+            let props = T::get_metadata().get_properties();
+            let prop = props.get(prop_name.as_str().ok_or("Invalid property name given")?);
             let value_mut = value.as_mut().ok_or("Invalid return zval given")?;
 
             Ok(match prop {
@@ -155,9 +172,9 @@ impl ZendObjectHandlers {
                 .and_then(|obj| ZendClassObject::<T>::from_zend_obj_mut(obj))
                 .ok_or("Invalid object pointer given")?;
             let self_ = &mut **obj;
-            let struct_props = T::get_properties();
+            let struct_props = T::get_metadata().get_properties();
 
-            for (name, val) in struct_props.into_iter() {
+            for (name, val) in struct_props {
                 let mut zv = Zval::new();
                 if val.get(self_, &mut zv).is_err() {
                     continue;
@@ -202,7 +219,7 @@ impl ZendObjectHandlers {
             let prop_name = member
                 .as_ref()
                 .ok_or("Invalid property name pointer given")?;
-            let props = T::get_properties();
+            let props = T::get_metadata().get_properties();
             let prop = props.get(prop_name.as_str().ok_or("Invalid property name given")?);
             let self_ = &mut **obj;