Re: merging r/w locks

On Saturday 13 September 2025 00:37:08 (+02:00), Rob Landers wrote: > > > On Fri, Sep 12, 2025, at 23:39, Hans Krentel wrote: > > > > > > > > On Tuesday 09 September 2025 10:14:31 (+02:00), Rob Landers wrote: > > > > > Hi internals, > > > > > > Is there any hope to have > > > https://github.com/php/php-src/pull/16565 merged before 8.5? > > > It's too late to address all the locks, but having it as part of the ABI would be nice so we > > > can start fixing them for 8.6/9.0. In FrankenPHP, if something takes a lock, it can significantly > > > affect performance. By using r/w locks instead of exclusive locks, we can use shared locks when > > > reading instead of exclusive locks. > > > > > > — Rob > > > > My educated guess would be that environ is per process. > > > > -- hakre > > > > Hi Hakre, > > We already solved the environment locks in FrankenPHP (which uses threads instead of processes > in ZTS mode) by basically “deleting” the built-in environment functions and replacing them. > Environment access is not thread safe, and Go (the language of FrankenPHP) uses its own locks around > environment access. Thus we needed to prevent concurrent access to the environment in the two > runtimes (TSRM vs. Go). That part of the PR will just help with other SAPI performance when using > PHP compiled in ZTS mode. Well if you go go-routine level read-only this is probably safe, but it _probably_ depends on what you expect to find on the top of _SERVER per each SAPI. IIRC for FrankenPHP this is EMBED via CGO. > > The main feature of the PR is not the environment locks, but the adding of non-exclusive locks > to TSRM. For example, there are TSRM locks in OPcache, but they’re only used *during writes* and > *don’t protect reads*. This can lead to very subtle bugs and crashes that we see sporadically in > FrankenPHP. By enabling r/w locks (the new tsrm_rwlock_rlock in this PR), we can protect reads *and* > writes, without degrading existing performance. If you see a benefit and it's across SAPIs -- why not consider it? If it's that stable and works under any process model that folllows the C semantics of GO (and PHP), this should be portable/transparent, shouldn't it? > > TSRM itself could use this as well, so that every time FrankenPHP spawns a new PHP thread, it > doesn’t need to contend for reading/writing globals during startup (and for dealing with cases > where TSRM cache can’t be used, which affects ALL global reads/writes if I understand correctly > — which is possible I don’t). > > I believe there are several other instances that would benefit from r/w locks as well, such as > mysqlnd, but it has been awhile since I looked at this, and I’m just going off notes. I don't know those API/ABI good enough, but perhaps Johannes could know on spot for your question. > > — Rob -- hakre