Skip to content

ext/mysqlnd: Use relaxed atomics for statistics in ZTS builds #24448

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ArtUkrainskiy wants to merge 1 commit into
base: master
Choose a base branch
from
+34 −21
Open

ext/mysqlnd: Use relaxed atomics for statistics in ZTS builds #24448

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions UPGRADING.INTERNALS
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,13 @@ PHP 8.7 INTERNALS UPGRADE NOTES
mysqlnd_reverse_api_register_api() and mysqlnd_reverse_api_get_api_list()
no longer exist.

- Mysqlnd:
. In ZTS builds the values of MYSQLND_STATS are atomic_uint_least64_t and
the LOCK_access member was removed. The MYSQLND_INC_STATISTIC*() and
MYSQLND_DEC_STATISTIC() macros update them with relaxed atomic additions.
. The MYSQLND_STATS_LOCK() and MYSQLND_STATS_UNLOCK() macros were removed.
MYSQLND_STATS_UPDATE_VALUE() is now an expression instead of a block.

- Standard:
. php_explode() now takes a HashTable* instead of a zval* to store the
resulting parts. It already expected the zval* to be IS_ARRAY.
Expand Down
1 change: 0 additions & 1 deletion ext/mysqlnd/mysqlnd_block_alloc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ PHPAPI void
mysqlnd_mempool_destroy(MYSQLND_MEMORY_POOL * pool)
{
DBG_ENTER("mysqlnd_mempool_destroy");
/* mnd_free will reference LOCK_access and might crash, depending on the caller...*/
zend_arena_destroy(pool->arena);
DBG_VOID_RETURN;
}
Expand Down
1 change: 0 additions & 1 deletion ext/mysqlnd/mysqlnd_driver.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ PHPAPI void mysqlnd_library_init(void)
mysqlnd_conn_set_methods(&MYSQLND_CLASS_METHOD_TABLE_NAME(mysqlnd_conn));
mysqlnd_conn_data_set_methods(&MYSQLND_CLASS_METHOD_TABLE_NAME(mysqlnd_conn_data));
_mysqlnd_init_ps_subsystem();
/* Should be calloc, as mnd_calloc will reference LOCK_access*/
mysqlnd_stats_init(&mysqlnd_global_stats, STAT_LAST, 1);
mysqlnd_plugin_subsystem_init();
{
Expand Down
16 changes: 10 additions & 6 deletions ext/mysqlnd/mysqlnd_statistics.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,12 @@ mysqlnd_fill_stats_hash(const MYSQLND_STATS * const stats, const MYSQLND_STRING
for (i = 0; i < stats->count; i++) {
char tmp[25];

snprintf(tmp, sizeof(tmp), "%" PRIu64, stats->values[i]);
#ifdef ZTS
uint64_t value = atomic_load_explicit(&stats->values[i], memory_order_relaxed);
#else
uint64_t value = stats->values[i];
#endif
snprintf(tmp, sizeof(tmp), "%" PRIu64, value);
add_assoc_string_ex(return_value, names[i].s, names[i].l, tmp);
}
}
Expand All @@ -216,7 +221,9 @@ mysqlnd_stats_init(MYSQLND_STATS ** stats, const size_t statistic_count, const b
*stats = pecalloc(1, size, persistent);
(*stats)->count = statistic_count;
#ifdef ZTS
(*stats)->LOCK_access = tsrm_mutex_alloc();
for (size_t i = 0; i < statistic_count; i++) {
atomic_init(&(*stats)->values[i], 0);
}
#endif
}
/* }}} */
Expand All @@ -226,10 +233,7 @@ mysqlnd_stats_init(MYSQLND_STATS ** stats, const size_t statistic_count, const b
PHPAPI void
mysqlnd_stats_end(MYSQLND_STATS * stats, const bool persistent)
{
#ifdef ZTS
tsrm_mutex_free(stats->LOCK_access);
#endif
/* mnd_free will reference LOCK_access and crash...*/
/* mnd_free would update the statistics being freed */
pefree(stats, persistent);
}
/* }}} */
Expand Down
15 changes: 4 additions & 11 deletions ext/mysqlnd/mysqlnd_statistics.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,19 +17,12 @@
#define MYSQLND_STATISTICS_H

#ifdef ZTS
#define MYSQLND_STATS_LOCK(stats) tsrm_mutex_lock((stats)->LOCK_access)
#define MYSQLND_STATS_UNLOCK(stats) tsrm_mutex_unlock((stats)->LOCK_access)
#define MYSQLND_STATS_UPDATE_VALUE(stats, statistic, value) \
atomic_fetch_add_explicit(&(stats)->values[(statistic)], (value), memory_order_relaxed)
#else
#define MYSQLND_STATS_LOCK(stats)
#define MYSQLND_STATS_UNLOCK(stats)
#endif

#define MYSQLND_STATS_UPDATE_VALUE(stats, statistic, value) \
{ \
MYSQLND_STATS_LOCK(stats); \
(stats)->values[(statistic)] += (value); \
MYSQLND_STATS_UNLOCK(_p_s); \
}
((stats)->values[(statistic)] += (value))
#endif

#define MYSQLND_DEC_STATISTIC(enabler, stats, statistic) \
{ \
Expand Down
15 changes: 13 additions & 2 deletions ext/mysqlnd/mysqlnd_structs.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -292,13 +292,24 @@ typedef enum_func_status (*mysqlnd_fetch_row_func)(MYSQLND_RES *result,

typedef struct st_mysqlnd_stats MYSQLND_STATS;

#if defined(ZTS) && defined(__cplusplus)
/* The C11 atomics used by struct st_mysqlnd_stats and the MYSQLND_*_STATISTIC*() macros */
extern "C++" {
using std::atomic_uint_least64_t;
using std::atomic_fetch_add_explicit;
using std::atomic_load_explicit;
using std::memory_order_relaxed;
}
#endif

struct st_mysqlnd_stats
{
size_t count;
#ifdef ZTS
MUTEX_T LOCK_access;
#endif
atomic_uint_least64_t values[] ZEND_ELEMENT_COUNT(count);
#else
uint64_t values[] ZEND_ELEMENT_COUNT(count);
#endif
};


Expand Down
Loading