Repository navigation
ext/mysqlnd: Use relaxed atomics for statistics in ZTS builds - #24448
ArtUkrainskiy wants to merge 1 commit into
Conversation
In ZTS builds every statistics update locked a mutex, and the global statistics are one object shared by all threads of the process. The values are now atomic_uint_least64_t updated with relaxed atomic additions, and LOCK_access is removed. NTS builds are unchanged. FrankenPHP 1.13.1 in worker mode with 16 threads, one prepared SELECT of 20 rows per request, AMD Ryzen 9 5950X, GCC 15.2. CPU time per request with statistics on (the default), by the cores FrankenPHP runs on: - 4 cores of one CCD: 0.3029 -> 0.2722 ms (-10%) - 4 cores, two on each CCD: 0.3553 -> 0.2934 ms (-17%) - 8 cores of one CCD: 0.3810 -> 0.2739 ms (-28%) - 8 cores, four on each CCD: 0.5274 -> 0.2781 ms (-47%) Statistics on now cost 0.6-1.4% over statistics off, against 12%, 23%, 41% and 91% before.
757907b to
a43cc05
Compare
|
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 See also: https://planet.mysql.com/entry/?id=21675:
|
|
Thanks! Short answers:
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. |
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. |
|
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. |
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. |
|
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 |
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
values[]isatomic_uint_least64_t, updated withatomic_fetch_add_explicit(..., memory_order_relaxed), read inmysqlnd_fill_stats_hash()withatomic_load_explicit(), and initialized withatomic_init().LOCK_accessis removed.uint64_tand+=: the machine code of the 455 mysqlnd functions is the same as on master.mysqlnd_structs.hbrings thestd::atomics into scope under ZTS, aszend_portability.hdoes foratomic_bool.Why it is correct
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.com_queryand 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
SELECTof 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 theatomic_init()loop was added. Cost of statistics on over off, CPU time per request of the FrankenPHP process, median of 5–15 alternating passes: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
pdo_mysql___construct.phpt, is from the local DSN setup and fails on master too).--enable-werrorwith GCC 13.3 and clang 18.1.Not tested: Windows x86 ZTS, ARM64, targets without lock-free 64-bit atomics (they may need libatomic), Apache with a threaded MPM, ext/parallel.
Compatibility
struct st_mysqlnd_statschanges 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. SeeUPGRADING.INTERNALS.