Skip to content

Conversation

dktapps
Copy link
Member

@dktapps dktapps commented Apr 14, 2025

This wasn't entirely successful and it does mean that ThreadSafe objects pay a performance penalty for reading & writing properties. I haven't attempted to optimise this yet. However this change means that we get PHP 8.4 property hooks without having to copy-paste a bunch of core logic from php-src.

This works by pushing values from shared storage to local cache on read,
and then letting std handlers operate on the local property table.
Vice versa for write.

This allows us to get support for stuff like 8.4 property hooks without
copy pasting lots of code and having inconsistencies.

this isn't fully stable yet, but should be working.
the std handlers will already generate these warnings anyway, so we're
just duplicating them.
@dktapps
Copy link
Member Author

dktapps commented May 3, 2025

Main performance considerations:

  • Double property info hashtable access - we could probably use fake caches for the std property handlers to avoid this, but it'll make the code more fragile
  • Not able to cache property info because Zend would skip our custom handler
  • Defined properties can't be optimized to array accesses within thread-safe objects because we can't guarantee that the class used for the object will be the same on all threads (because of autoloading). However, this should be a very rare edge case, so it's possible we might be able to optimise this if we verify the destination thread's class properties match and bail if they don't (or possibly have some kind of translation layer, but this seems like a lot of hassle for a rare edge case).

@dktapps
Copy link
Member Author

dktapps commented Aug 20, 2025

Probably worth restricting this to hooked properties to avoid paying a performance penalty on regular properties

@dktapps
Copy link
Member Author

dktapps commented Aug 28, 2025

After looking into how hooked properties actually work, I'm no longer convinced this change makes sense. This is almost certainly much more complex than bringing in the code needed to invoke the hooks in pmmpthread, as well as being worse for performance.

@dktapps dktapps closed this Aug 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant