From 02be0decbea105a9770dca83d24bab56eaf9ae4b Mon Sep 17 00:00:00 2001 From: jmjoy Date: Thu, 3 Apr 2025 12:37:51 +0800 Subject: [PATCH 1/5] refactor: optimizing `extends` and `implements` --- phper/src/classes.rs | 71 +++++++++++++++++++++++--------------------- 1 file changed, 37 insertions(+), 34 deletions(-) diff --git a/phper/src/classes.rs b/phper/src/classes.rs index ff8df16d..b7595bde 100644 --- a/phper/src/classes.rs +++ b/phper/src/classes.rs @@ -30,8 +30,7 @@ use std::{ marker::PhantomData, mem::{ManuallyDrop, replace, size_of, transmute, zeroed}, os::raw::c_int, - ptr, - ptr::null_mut, + ptr::{self, null_mut}, rc::Rc, slice, }; @@ -233,6 +232,12 @@ fn find_global_class_entry_ptr(name: impl AsRef) -> *mut zend_class_entry { } } +#[derive(Clone)] +enum InnerClassEntry { + Ptr(Rc>), + Name(String), +} + /// The [StateClass] holds [zend_class_entry] and inner state, created by /// [Module::add_class](crate::modules::Module::add_class) or /// [ClassEntity::bound_class]. @@ -274,8 +279,7 @@ fn find_global_class_entry_ptr(name: impl AsRef) -> *mut zend_class_entry { /// } /// ``` pub struct StateClass { - inner: Rc>, - name: Option, + inner: InnerClassEntry, _p: PhantomData, } @@ -283,8 +287,7 @@ impl StateClass<()> { /// Create from name, which will be looked up from globals. pub fn from_name(name: impl Into) -> Self { Self { - inner: Rc::new(RefCell::new(null_mut())), - name: Some(name.into()), + inner: InnerClassEntry::Name(name.into()), _p: PhantomData, } } @@ -293,22 +296,27 @@ impl StateClass<()> { impl StateClass { fn null() -> Self { Self { - inner: Rc::new(RefCell::new(null_mut())), - name: None, + inner: InnerClassEntry::Ptr(Rc::new(RefCell::new(null_mut()))), _p: PhantomData, } } fn bind(&self, ptr: *mut zend_class_entry) { - *self.inner.borrow_mut() = ptr; + match &self.inner { + InnerClassEntry::Ptr(p) => { + *p.borrow_mut() = ptr; + } + InnerClassEntry::Name(_) => { + unreachable!("Cannot bind() an StateClass created with from_name()"); + } + } } /// Converts to class entry. pub fn as_class_entry(&self) -> &ClassEntry { - if let Some(name) = &self.name { - ClassEntry::from_globals(name).unwrap() - } else { - unsafe { ClassEntry::from_mut_ptr(*self.inner.borrow()) } + match &self.inner { + InnerClassEntry::Ptr(ptr) => unsafe { ClassEntry::from_mut_ptr(*ptr.borrow()) }, + InnerClassEntry::Name(name) => ClassEntry::from_globals(name).unwrap(), } } @@ -338,7 +346,6 @@ impl Clone for StateClass { fn clone(&self) -> Self { Self { inner: self.inner.clone(), - name: self.name.clone(), _p: self._p, } } @@ -381,39 +388,39 @@ impl Clone for StateClass { /// ``` #[derive(Clone)] pub struct Interface { - inner: Rc>, - name: Option, + inner: InnerClassEntry, } impl Interface { fn null() -> Self { Self { - inner: Rc::new(RefCell::new(null_mut())), - name: None, + inner: InnerClassEntry::Ptr(Rc::new(RefCell::new(null_mut()))), } } /// Create a new interface from global name (eg "Stringable", "ArrayAccess") pub fn from_name(name: impl Into) -> Self { Self { - inner: Rc::new(RefCell::new(null_mut())), - name: Some(name.into()), + inner: InnerClassEntry::Name(name.into()), } } fn bind(&self, ptr: *mut zend_class_entry) { - if self.name.is_some() { - panic!("Cannot bind() an Interface created with from_name()"); + match &self.inner { + InnerClassEntry::Ptr(p) => { + *p.borrow_mut() = ptr; + } + InnerClassEntry::Name(_) => { + unreachable!("Cannot bind() an Interface created with from_name()"); + } } - *self.inner.borrow_mut() = ptr; } /// Converts to class entry. pub fn as_class_entry(&self) -> &ClassEntry { - if let Some(name) = &self.name { - ClassEntry::from_globals(name).unwrap() - } else { - unsafe { ClassEntry::from_mut_ptr(*self.inner.borrow()) } + match &self.inner { + InnerClassEntry::Ptr(ptr) => unsafe { ClassEntry::from_mut_ptr(*ptr.borrow()) }, + InnerClassEntry::Name(name) => ClassEntry::from_globals(name).unwrap(), } } } @@ -793,7 +800,7 @@ pub struct InterfaceEntity { interface_name: CString, method_entities: Vec, constants: Vec, - extends: Vec &'static ClassEntry>>, + extends: Vec, bound_interface: Interface, } @@ -840,11 +847,7 @@ impl InterfaceEntity { /// interface.extends(Interface::from_name("Stringable")); /// ``` pub fn extends(&mut self, interface: Interface) { - self.extends.push(Box::new(move || { - let entry: &'static ClassEntry = - unsafe { std::mem::transmute(interface.as_class_entry()) }; - entry - })); + self.extends.push(interface); } #[allow(clippy::useless_conversion)] @@ -861,7 +864,7 @@ impl InterfaceEntity { self.bound_interface.bind(class_ce); for interface in &self.extends { - let interface_ce = interface().as_ptr(); + let interface_ce = interface.as_class_entry().as_ptr(); zend_class_implements(class_ce, 1, interface_ce); } From d10da6082c976c9bbdc1374e95beb32bf17287b1 Mon Sep 17 00:00:00 2001 From: jmjoy Date: Thu, 3 Apr 2025 16:41:08 +0800 Subject: [PATCH 2/5] Optimize `as_class_entry` to avoid looking up every time --- phper/src/classes.rs | 44 ++++++++++++++++++++++++++------------------ 1 file changed, 26 insertions(+), 18 deletions(-) diff --git a/phper/src/classes.rs b/phper/src/classes.rs index b7595bde..928049e1 100644 --- a/phper/src/classes.rs +++ b/phper/src/classes.rs @@ -30,7 +30,7 @@ use std::{ marker::PhantomData, mem::{ManuallyDrop, replace, size_of, transmute, zeroed}, os::raw::c_int, - ptr::{self, null_mut}, + ptr::{self, null, null_mut}, rc::Rc, slice, }; @@ -234,7 +234,7 @@ fn find_global_class_entry_ptr(name: impl AsRef) -> *mut zend_class_entry { #[derive(Clone)] enum InnerClassEntry { - Ptr(Rc>), + Ptr(*const zend_class_entry), Name(String), } @@ -279,7 +279,7 @@ enum InnerClassEntry { /// } /// ``` pub struct StateClass { - inner: InnerClassEntry, + inner: Rc>, _p: PhantomData, } @@ -287,7 +287,7 @@ impl StateClass<()> { /// Create from name, which will be looked up from globals. pub fn from_name(name: impl Into) -> Self { Self { - inner: InnerClassEntry::Name(name.into()), + inner: Rc::new(RefCell::new(InnerClassEntry::Name(name.into()))), _p: PhantomData, } } @@ -296,15 +296,15 @@ impl StateClass<()> { impl StateClass { fn null() -> Self { Self { - inner: InnerClassEntry::Ptr(Rc::new(RefCell::new(null_mut()))), + inner: Rc::new(RefCell::new(InnerClassEntry::Ptr(null()))), _p: PhantomData, } } fn bind(&self, ptr: *mut zend_class_entry) { - match &self.inner { + match &mut *self.inner.borrow_mut() { InnerClassEntry::Ptr(p) => { - *p.borrow_mut() = ptr; + *p = ptr; } InnerClassEntry::Name(_) => { unreachable!("Cannot bind() an StateClass created with from_name()"); @@ -314,9 +314,13 @@ impl StateClass { /// Converts to class entry. pub fn as_class_entry(&self) -> &ClassEntry { - match &self.inner { - InnerClassEntry::Ptr(ptr) => unsafe { ClassEntry::from_mut_ptr(*ptr.borrow()) }, - InnerClassEntry::Name(name) => ClassEntry::from_globals(name).unwrap(), + match self.inner.borrow().clone() { + InnerClassEntry::Ptr(ptr) => unsafe { ClassEntry::from_ptr(ptr) }, + InnerClassEntry::Name(name) => { + let entry = ClassEntry::from_globals(name).unwrap(); + *self.inner.borrow_mut() = InnerClassEntry::Ptr(entry.as_ptr()); + entry + } } } @@ -388,27 +392,27 @@ impl Clone for StateClass { /// ``` #[derive(Clone)] pub struct Interface { - inner: InnerClassEntry, + inner: Rc>, } impl Interface { fn null() -> Self { Self { - inner: InnerClassEntry::Ptr(Rc::new(RefCell::new(null_mut()))), + inner: Rc::new(RefCell::new(InnerClassEntry::Ptr(null()))), } } /// Create a new interface from global name (eg "Stringable", "ArrayAccess") pub fn from_name(name: impl Into) -> Self { Self { - inner: InnerClassEntry::Name(name.into()), + inner: Rc::new(RefCell::new(InnerClassEntry::Name(name.into()))), } } fn bind(&self, ptr: *mut zend_class_entry) { - match &self.inner { + match &mut *self.inner.borrow_mut() { InnerClassEntry::Ptr(p) => { - *p.borrow_mut() = ptr; + *p = ptr; } InnerClassEntry::Name(_) => { unreachable!("Cannot bind() an Interface created with from_name()"); @@ -418,9 +422,13 @@ impl Interface { /// Converts to class entry. pub fn as_class_entry(&self) -> &ClassEntry { - match &self.inner { - InnerClassEntry::Ptr(ptr) => unsafe { ClassEntry::from_mut_ptr(*ptr.borrow()) }, - InnerClassEntry::Name(name) => ClassEntry::from_globals(name).unwrap(), + match self.inner.borrow().clone() { + InnerClassEntry::Ptr(ptr) => unsafe { ClassEntry::from_ptr(ptr) }, + InnerClassEntry::Name(name) => { + let entry = ClassEntry::from_globals(name).unwrap(); + *self.inner.borrow_mut() = InnerClassEntry::Ptr(entry.as_ptr()); + entry + } } } } From 1f8812cd670333d1d6624a7b017336038916258f Mon Sep 17 00:00:00 2001 From: jmjoy Date: Thu, 3 Apr 2025 16:47:03 +0800 Subject: [PATCH 3/5] Temporary variables are required --- phper/src/classes.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/phper/src/classes.rs b/phper/src/classes.rs index 928049e1..871e7c2c 100644 --- a/phper/src/classes.rs +++ b/phper/src/classes.rs @@ -314,7 +314,8 @@ impl StateClass { /// Converts to class entry. pub fn as_class_entry(&self) -> &ClassEntry { - match self.inner.borrow().clone() { + let inner = self.inner.borrow().clone(); + match inner { InnerClassEntry::Ptr(ptr) => unsafe { ClassEntry::from_ptr(ptr) }, InnerClassEntry::Name(name) => { let entry = ClassEntry::from_globals(name).unwrap(); @@ -422,7 +423,8 @@ impl Interface { /// Converts to class entry. pub fn as_class_entry(&self) -> &ClassEntry { - match self.inner.borrow().clone() { + let inner = self.inner.borrow().clone(); + match inner { InnerClassEntry::Ptr(ptr) => unsafe { ClassEntry::from_ptr(ptr) }, InnerClassEntry::Name(name) => { let entry = ClassEntry::from_globals(name).unwrap(); From b6142eb9ce2028a9756abfe1498e00272efdf20d Mon Sep 17 00:00:00 2001 From: jmjoy Date: Thu, 3 Apr 2025 18:20:44 +0800 Subject: [PATCH 4/5] Fix #195 --- phper/src/classes.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/phper/src/classes.rs b/phper/src/classes.rs index 871e7c2c..05780187 100644 --- a/phper/src/classes.rs +++ b/phper/src/classes.rs @@ -278,12 +278,12 @@ enum InnerClassEntry { /// module /// } /// ``` -pub struct StateClass { +pub struct StateClass { inner: Rc>, _p: PhantomData, } -impl StateClass<()> { +impl StateClass<[()]> { /// Create from name, which will be looked up from globals. pub fn from_name(name: impl Into) -> Self { Self { @@ -293,7 +293,7 @@ impl StateClass<()> { } } -impl StateClass { +impl StateClass { fn null() -> Self { Self { inner: Rc::new(RefCell::new(InnerClassEntry::Ptr(null()))), @@ -324,7 +324,9 @@ impl StateClass { } } } +} +impl StateClass { /// Create the object from class and call `__construct` with arguments. /// /// If the `__construct` is private, or protected and the called scope isn't @@ -605,7 +607,7 @@ impl ClassEntity { /// module /// } /// ``` - pub fn extends(&mut self, parent: StateClass) { + pub fn extends(&mut self, parent: StateClass) { self.parent = Some(unsafe { transmute::, StateClass<()>>(parent) }); } From d59c035f5767669980e9add12d1461e7f3938ec6 Mon Sep 17 00:00:00 2001 From: jmjoy Date: Thu, 3 Apr 2025 18:31:19 +0800 Subject: [PATCH 5/5] Avoid potential problems --- phper/src/classes.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/phper/src/classes.rs b/phper/src/classes.rs index 05780187..dc5a103a 100644 --- a/phper/src/classes.rs +++ b/phper/src/classes.rs @@ -452,7 +452,7 @@ pub struct ClassEntity { state_constructor: Rc, method_entities: Vec, property_entities: Vec, - parent: Option>, + parent: Option>, interfaces: Vec, constants: Vec, bound_class: StateClass, @@ -608,7 +608,7 @@ impl ClassEntity { /// } /// ``` pub fn extends(&mut self, parent: StateClass) { - self.parent = Some(unsafe { transmute::, StateClass<()>>(parent) }); + self.parent = Some(unsafe { transmute::, StateClass<[()]>>(parent) }); } /// Register class to `implements` the interface, due to the class can