Skip to content

Commit a6598dc

Browse files
authored
perf: cache property lookups across repeated accesses (#726)
When PHP reads or writes an object property (e.g. `$obj->name`), ext-php-rs previously searched through all registered properties by name on every single access. In a loop hitting the same property 100k times, that same search ran 100k times. PHP provides a per-opcode "cache slot" mechanism to avoid this. On the first access, we now store the resolved property descriptor pointer in the cache slot. On all subsequent accesses to the same property on the same class, we skip the search entirely and read the cached pointer directly -- a single pointer comparison instead of string conversions and linear scans. Applied to all three property handlers: read, write, and isset/exists. Benchmarks (callgrind, 100k iterations): - Property reads: 235M -> 93.5M instructions (-60%) - Property writes: 220M -> 75.7M instructions (-66%)
1 parent a8b7b30 commit a6598dc

2 files changed

Lines changed: 91 additions & 20 deletions

File tree

src/zend/handlers.rs

Lines changed: 62 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ use crate::{
1010
zend_std_read_property, zend_std_write_property, zend_throw_error,
1111
},
1212
flags::{PropertyFlags, ZvalTypeFlags},
13+
internal::property::PropertyDescriptor,
1314
types::{ZendClassObject, ZendHashTable, ZendObject, ZendStr, Zval},
1415
};
1516

@@ -135,24 +136,23 @@ impl ZendObjectHandlers {
135136
cache_slot: *mut *mut c_void,
136137
rv: *mut Zval,
137138
) -> PhpResult<*mut Zval> {
138-
let prop_name = unsafe {
139-
member
140-
.as_ref()
141-
.ok_or("Invalid property name pointer given")?
142-
};
143139
let self_ = &*obj;
144-
let prop = T::get_metadata().find_property(prop_name.as_str()?);
140+
let prop = unsafe { resolve_property::<T>(member, cache_slot)? };
145141

146142
// retval needs to be treated as initialized, so we set the type to null
147143
let rv_mut = unsafe { rv.as_mut().ok_or("Invalid return zval given")? };
148144
rv_mut.u1.type_info = ZvalTypeFlags::Null.bits();
149145

150146
Ok(match prop {
151147
Some(prop_info) => {
152-
// Check visibility before allowing access
153148
let object_ce = unsafe { (*object).ce };
154149
if !unsafe { check_property_access(prop_info.flags, object_ce) } {
155150
let is_private = prop_info.flags.contains(PropertyFlags::Private);
151+
let prop_name = unsafe {
152+
member
153+
.as_ref()
154+
.ok_or("Invalid property name pointer given")?
155+
};
156156
unsafe {
157157
throw_property_access_error(
158158
T::CLASS_NAME,
@@ -208,21 +208,20 @@ impl ZendObjectHandlers {
208208
value: *mut Zval,
209209
cache_slot: *mut *mut c_void,
210210
) -> PhpResult<*mut Zval> {
211-
let prop_name = unsafe {
212-
member
213-
.as_ref()
214-
.ok_or("Invalid property name pointer given")?
215-
};
216211
let self_ = &mut *obj;
217-
let prop = T::get_metadata().find_property(prop_name.as_str()?);
212+
let prop = unsafe { resolve_property::<T>(member, cache_slot)? };
218213
let value_mut = unsafe { value.as_mut().ok_or("Invalid return zval given")? };
219214

220215
Ok(match prop {
221216
Some(prop_info) => {
222-
// Check visibility before allowing access
223217
let object_ce = unsafe { (*object).ce };
224218
if !unsafe { check_property_access(prop_info.flags, object_ce) } {
225219
let is_private = prop_info.flags.contains(PropertyFlags::Private);
220+
let prop_name = unsafe {
221+
member
222+
.as_ref()
223+
.ok_or("Invalid property name pointer given")?
224+
};
226225
unsafe {
227226
throw_property_access_error(
228227
T::CLASS_NAME,
@@ -341,12 +340,7 @@ impl ZendObjectHandlers {
341340
has_set_exists: c_int,
342341
cache_slot: *mut *mut c_void,
343342
) -> PhpResult<c_int> {
344-
let prop_name = unsafe {
345-
member
346-
.as_ref()
347-
.ok_or("Invalid property name pointer given")?
348-
};
349-
let prop = T::get_metadata().find_property(prop_name.as_str()?);
343+
let prop = unsafe { resolve_property::<T>(member, cache_slot)? };
350344
let self_ = &*obj;
351345

352346
match has_set_exists {
@@ -411,6 +405,54 @@ impl ZendObjectHandlers {
411405
}
412406
}
413407

408+
/// Resolves a property descriptor via `cache_slot` or linear scan fallback.
409+
///
410+
/// On cache hit (`cache_slot[1]` matches this class's metadata pointer), returns
411+
/// the cached `&PropertyDescriptor<T>` directly, skipping string conversion and
412+
/// `find_property`. On miss, performs the full lookup and populates the cache for
413+
/// subsequent calls.
414+
///
415+
/// # Safety
416+
///
417+
/// - `member` must be a valid `ZendStr` pointer.
418+
/// - `cache_slot` must be null or point to at least 2 writable `*mut c_void` slots
419+
/// (guaranteed by PHP's opcode compiler for all property access opcodes).
420+
#[allow(clippy::inline_always)]
421+
#[inline(always)]
422+
unsafe fn resolve_property<T: RegisteredClass>(
423+
member: *mut ZendStr,
424+
cache_slot: *mut *mut c_void,
425+
) -> PhpResult<Option<&'static PropertyDescriptor<T>>> {
426+
let meta = T::get_metadata();
427+
let meta_ptr = ptr::from_ref(meta).cast::<c_void>().cast_mut();
428+
429+
if !cache_slot.is_null() {
430+
let guard = unsafe { *cache_slot.add(1) };
431+
if guard == meta_ptr {
432+
let desc = unsafe { &*(*cache_slot).cast::<PropertyDescriptor<T>>() };
433+
return Ok(Some(desc));
434+
}
435+
}
436+
437+
let prop_name = unsafe {
438+
member
439+
.as_ref()
440+
.ok_or("Invalid property name pointer given")?
441+
};
442+
let Some(descriptor) = meta.find_property(prop_name.as_str()?) else {
443+
return Ok(None);
444+
};
445+
446+
if !cache_slot.is_null() {
447+
unsafe {
448+
*cache_slot = ptr::from_ref(descriptor).cast::<c_void>().cast_mut();
449+
*cache_slot.add(1) = meta_ptr;
450+
}
451+
}
452+
453+
Ok(Some(descriptor))
454+
}
455+
414456
/// Gets the current calling scope from the executor globals.
415457
///
416458
/// # Safety

tests/src/integration/class/class.php

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -350,3 +350,32 @@ public function __construct(string $data) {
350350

351351
$uncloneable = new TestUncloneableClass('test');
352352
assert_exception_thrown(fn() => clone $uncloneable, 'Cloning uncloneable class should throw');
353+
354+
// Test cache_slot: repeated property access in a tight loop.
355+
// After the first access resolves via find_property, subsequent accesses should
356+
// hit the cache_slot fast path and return the same correct value.
357+
$cacheObj = test_class('cached', 999);
358+
for ($i = 0; $i < 1000; $i++) {
359+
assert($cacheObj->string === 'cached', "Cached read failed at iteration $i");
360+
assert($cacheObj->number === 999, "Cached read (number) failed at iteration $i");
361+
}
362+
363+
// Test cache_slot: write then read in the same loop iteration.
364+
// Validates the cache returns the updated descriptor (same descriptor, new value).
365+
for ($i = 0; $i < 100; $i++) {
366+
$cacheObj->number = $i;
367+
assert($cacheObj->number === $i, "Write-then-read failed at iteration $i");
368+
}
369+
370+
// Test cache_slot: isset() followed by read on the same property.
371+
// Both has_property and read_property share cache_slot for the same member name.
372+
$issetObj = test_class('isset_test', 42);
373+
assert(isset($issetObj->string), 'isset should return true for defined property');
374+
assert($issetObj->string === 'isset_test', 'Read after isset should return correct value');
375+
376+
// Test cache_slot: visibility is still enforced after caching.
377+
// The cached descriptor must still go through the access check.
378+
$vis = new TestPropertyVisibility(1, 'secret', 'guarded');
379+
assert($vis->publicNum === 1, 'Public read should work before cache warms');
380+
assert($vis->publicNum === 1, 'Public read should work after cache warms');
381+
assert_exception_thrown(fn() => $vis->privateStr, 'Private access should throw even if cache_slot is warm');

0 commit comments

Comments
 (0)