[ArtUkrainskiy]

Fixes GH-24447.

In ZTS builds (FrankenPHP, threaded Apache), every mysqlnd statistics update locks a mutex, and the global statistics are one object shared by all threads. Statistics are on by default, so with MySQL they cost 12–91% more CPU per request, depending on how many cores the threads run on. This PR replaces the mutex with relaxed atomic additions; with it, statistics cost 0.6–1.4%. NTS builds are unchanged.

Change

• Under ZTS, values[] is atomic_uint_least64_t, updated with atomic_fetch_add_explicit(..., memory_order_relaxed), read in mysqlnd_fill_stats_hash() with atomic_load_explicit(), and initialized with atomic_init(). LOCK_access is removed.
• NTS keeps uint64_t and +=: the machine code of the 455 mysqlnd functions is the same as on master.
• For C++ extensions, mysqlnd_structs.h brings the std:: atomics into scope under ZTS, as zend_portability.h does for atomic_bool.

Why it is correct

• The mutex only protected the increments: mysqlnd_fill_stats_hash() already read without it and nothing resets the counters, so readers never had a consistent snapshot. The unlocked read was a data race; it is now an atomic load.
• The counters are independent, so relaxed ordering is enough.
• 50,000 requests from 50 clients to FrankenPHP with 16 threads count exactly 50,000 com_query and 1,000,000 rows on master and with this PR; a deliberately racy variant counts 49,460 and 997,554.

Results

FrankenPHP 1.13.1 (worker mode, 16 threads) built against master b7256ee and this PR, one prepared SELECT of 20 rows per request, MySQL 8.4.11, wrk -t4 -c50, AMD Ryzen 9 5950X (two CCDs), GCC 15.2 with the flags of the official Docker image; the PR build was measured before the atomic_init() loop was added. Cost of statistics on over off, CPU time per request of the FrankenPHP process, median of 5–15 alternating passes:

FrankenPHP cores	master	this PR
4, one CCD	+12.1%	+0.6%
4, two on each CCD	+22.7%	+1.4%
8, one CCD	+40.8%	+0.8%
8, four on each CCD	+90.9%	+1.0%
4, one CCD, binary protocol	+13.9%	+1.4%

With statistics off, both builds run the same code, and the text-protocol series are within 0.3% of each other; passes fall into two states for a reason I have not found, so the per-pass ranges in the report are wide while the medians are stable. On a full JSON API with four queries per request on PHP 8.5.11, a prototype of this change brings the cost from 7–32% (depending on load) to 0.8–2.8%. Absolute numbers, every pass and the scripts: report, JSON API data.

Verification

• ext/mysqlnd, ext/mysqli and ext/pdo_mysql tests: same results as master (763 passed; the one failure, pdo_mysql___construct.phpt, is from the local DSN setup and fails on master too).
• CI green, including Windows x64 ZTS, Linux x32 ZTS and ASan; --enable-werror with GCC 13.3 and clang 18.1.
• C++: the headers compile as C++11/17/20 with g++ and clang++; C and C++ see the same struct layout, and counters updated from C++ threads are exact when read in C.

Compatibility

struct st_mysqlnd_stats changes layout under ZTS, so this is for master only, and extensions using the statistics macros need a rebuild. MYSQLND_STATS_LOCK()/MYSQLND_STATS_UNLOCK() are removed. In C++ under ZTS the struct cannot be copied by value. See UPGRADING.INTERNALS.

Update (5d34b0b): some 32-bit targets (ARMv5, MIPS, PowerPC, RISC-V) implement 64-bit atomics in libatomic, so in ZTS builds configure now links mysqlnd with -latomic when needed. Checked on Debian trixie armel (GCC 14.2, under qemu): configure picks -latomic, PHP builds and the statistics count correctly; on x86-64 nothing is added.

[kamil-tekiela]

This is a pretty inconsequential change because I cannot imagine a reason that anyone would use mysqlnd statistics outside of PHP source. But if it is not a risky change, then we can merge it. However, I know nothing about ZTS or FrankenPHP so I cannot review this properly. I also don't know if this will compile cleanly under every supported setup. Will it also compile with C++? What about mysqlnd extensions, are they impacted?

If mysqlnd.collect_statistics is on by default in release builds, then I would be in favor of changing this. In a traditional setup, statistics collection is unobservable performance-wise. I am pretty sure nobody is going to notice when they are off.

See also: https://planet.mysql.com/entry/?id=21675:

For thread safety statistics are protected by a mutex. There is one mutex for all statistics. Due to the high number of fine grained statistics many mutex locks will happen when collecting statistics. For performance reasons you should turn off statistics in production. Of course, no mutex locks will happen if you use CGI/Fast-CGI.

[ArtUkrainskiy]

Thanks! Short answers:

• Default: yes, collect_statistics is 1 by default and On in both php.ini files.
• Builds: CI is green on all jobs, including Windows x64 ZTS, 32-bit ZTS and ASan. NTS machine code is unchanged.
• C++: yes. The headers compile as C++11/17/20 with g++ and clang++, and C and C++ see the same struct layout.
• Extensions: the only public user is mysql_xdevapi (C++), unmaintained since 2022; it does not build against master even without this PR. With this PR it gets no new errors, only a harmless -Wclass-memaccess warning on a memset().

On the default: I would support turning it off. NTS pays nothing, ZTS pays on every packet and row, and ZTS setups like FrankenPHP are growing. In my experience apps that use the statistics enable them on purpose (we sent them to Datadog). I would keep that change separate; this PR makes the statistics cheap for those who keep them on (0.6–1.4% CPU per request instead of up to 91%).

Details: https://artukrainskiy.github.io/posts/mysqlnd-zts-statistics/

cc @bukka as a code owner of ext/mysqlnd, for the ZTS side.

[kamil-tekiela]

In my experience apps that use the statistics enable them on purpose (we sent them to Datadog)

Are you saying that you use them actively? Can you share some more details so that I understand better? What purpose do they serve you? How do you utilize them?

FWIW, they weren't meant for PHP users, but for the developers of mysqlnd and its plugins.

[ArtUkrainskiy]

Not anymore. It was about four years ago at a previous job, so I don't remember the details: we exported them from the application to Datadog as client-side database metrics, next to the APM data. I mentioned it only as an example that some applications do read them and enable them on purpose.

[edorian]

Are you saying that you use them actively? Can you share some more details so that I understand better? What purpose do they serve you? How do you utilize them?

For what it's worth: We do utilize them at Tideways (PHP APM) for production instrumentation to give people insights into their queries at a minimal performance overhead. It's a very useful feature for us.

[ArtUkrainskiy]

Thanks, that's very useful to know. It changes my view on the default: with this PR the statistics cost about 1% CPU per request even on ZTS, so there is much less reason to turn them off by default, and Tideways users keep the data without enabling anything.

I guess the mysql.index_usage and network annotations on MySQL spans come from no_index_used/bad_index_used and bytes_sent/bytes_received. Does the extension read the connection's MYSQLND_STATS::values directly in C? With this PR they are atomic_uint_least64_t under ZTS, so it would need a rebuild but no source change. Since Tideways supports FrankenPHP, your users there pay for the mutex today.

[henderkes]

I had no idea that mysqlnd was using locks in ZTS, probably because I didn't know it collected statistics in the first place.

The changes look right, but I recall there are some platforms that (still) need to link in -latomic for atomic support. Have you checked that mysqlnd can still build (not aarch64 or x86_64, I think something like riscv iirc)?

[ArtUkrainskiy]

The changes look right, but I recall there are some platforms that (still) need to link in -latomic for atomic support. Have you checked that mysqlnd can still build (not aarch64 or x86_64, I think something like riscv iirc)?

Good catch, thanks. 64-bit atomics compile to libatomic calls on 32-bit ARMv5, MIPS, PowerPC and RISC-V, and php-src does not link libatomic, so this would have broken e.g. the arm32v5 ZTS Docker image. I pushed a fallback: the atomics are used only where ATOMIC_LLONG_LOCK_FREE == 2, other platforms keep the mutex. Checked on Debian armel (GCC 14.2, under qemu): it builds and links without libatomic, and the statistics count correctly.

[henderkes]

I don't want a fallback, I just want to see -latomic linked in if necessary. Should be a simple build system fix.

[ArtUkrainskiy]

Done: the fallback is gone and the C code is as before; in ZTS builds config9.m4 now links mysqlnd with -latomic when a 64-bit atomic_fetch_add_explicit() does not link without it. On Debian armel (GCC 14.2, under qemu) configure picks -latomic, PHP builds and the statistics count correctly; on x86-64 it reports "none required" and nothing is added.