Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:128696
Content-Type: text/plain; charset=utf-8
Message-ID: <1757718325216.2367189491.2395335932@yahoo.de>
To: rob@bottled.codes
Cc: internals@lists.php.net
Subject: Re: [PHP-DEV] merging r/w locks
Date: Fri, 12 Sep 2025 23:15:26 +0000
In-Reply-To: <2a34b4a4-a1f3-40d1-a005-49a91a31b07c@app.fastmail.com>
References: <2a34b4a4-a1f3-40d1-a005-49a91a31b07c@app.fastmail.com>
User-Agent: Vivaldi Mail/7.5.3735.66
Content-Transfer-Encoding: quoted-printable
Precedence: list
MIME-Version: 1.0
From: hanskrentel@yahoo.de (Hans Krentel)




On Saturday 13 September 2025 00:37:08 (+02:00), Rob Landers wrote:

>=20
>=20
> On Fri, Sep 12, 2025, at 23:39, Hans Krentel wrote:
> >=20
> >=20
> >=20
> > On Tuesday 09 September 2025 10:14:31 (+02:00), Rob Landers wrote:
> >=20
> > > Hi internals,
> > >=20
> > > 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.
> > >=20
> > > =E2=80=94 Rob
> >=20
> > My educated guess would be that environ is per process.
> >=20
> > -- hakre
> >=20
>=20
> Hi Hakre,
>=20
> We already solved the environment locks in FrankenPHP (which uses threads=
 instead of processes in ZTS mode) by basically =E2=80=9Cdeleting=E2=80=9D =
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=E2=80=99re only used *during writes* and *don=E2=80=99t =
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?

>=20
> TSRM itself could use this as well, so that every time FrankenPHP spawns =
a new PHP thread, it doesn=E2=80=99t need to contend for reading/writing =
globals during startup (and for dealing with cases where TSRM cache =
can=E2=80=99t be used, which affects ALL global reads/writes if I =
understand correctly =E2=80=94 which is possible I don=E2=80=99t).
>=20
> 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=E2=80=99m just going off notes.

I don't know those API/ABI good enough, but perhaps Johannes could know on =
spot for your question.

>=20
> =E2=80=94 Rob


-- hakre