Skip to content

Commit 292ea9b

Browse files
committed
Revert "ext/opcache: use C11 atomics for "restart_in" (php#10276)"
This reverts commit 061fcdb. While the commit does indeed improve performance, @dstogov complained that it disables the code path calling kill_all_lockers(), and thus hanging workers are never killed and restarted. php#10276 (comment) The fact that this feature was not implemented in the existing atomic code path (i.e. Windows) did not mean that the feature was considered not strictly necessary, but that the Windows implementation just did not need the feature because SAPIs that work on Windows do not manage child processes php#10276 (comment) php#10276 (comment)
1 parent f833a7e commit 292ea9b

File tree

3 files changed

+12
-32
lines changed

3 files changed

+12
-32
lines changed

ext/opcache/ZendAccelerator.c

Lines changed: 11 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -135,11 +135,7 @@ static void preload_shutdown(void);
135135
static void preload_activate(void);
136136
static void preload_restart(void);
137137

138-
#ifdef HAVE_STDATOMIC_H
139-
# define INCREMENT(v) (++ZCSG(v))
140-
# define DECREMENT(v) (--ZCSG(v))
141-
# define LOCKVAL(v) atomic_load(&ZCSG(v))
142-
#elif defined(ZEND_WIN32)
138+
#ifdef ZEND_WIN32
143139
# define INCREMENT(v) InterlockedIncrement64(&ZCSG(v))
144140
# define DECREMENT(v) InterlockedDecrement64(&ZCSG(v))
145141
# define LOCKVAL(v) (ZCSG(v))
@@ -277,7 +273,7 @@ static ZEND_INI_MH(accel_include_path_on_modify)
277273

278274
static inline void accel_restart_enter(void)
279275
{
280-
#ifdef INCREMENT
276+
#ifdef ZEND_WIN32
281277
INCREMENT(restart_in);
282278
#else
283279
struct flock restart_in_progress;
@@ -296,7 +292,7 @@ static inline void accel_restart_enter(void)
296292

297293
static inline void accel_restart_leave(void)
298294
{
299-
#ifdef DECREMENT
295+
#ifdef ZEND_WIN32
300296
ZCSG(restart_in_progress) = false;
301297
DECREMENT(restart_in);
302298
#else
@@ -317,9 +313,7 @@ static inline void accel_restart_leave(void)
317313
static inline int accel_restart_is_active(void)
318314
{
319315
if (ZCSG(restart_in_progress)) {
320-
#ifdef LOCKVAL
321-
return LOCKVAL(restart_in) != 0;
322-
#else
316+
#ifndef ZEND_WIN32
323317
struct flock restart_check;
324318

325319
restart_check.l_type = F_WRLCK;
@@ -337,6 +331,8 @@ static inline int accel_restart_is_active(void)
337331
} else {
338332
return 1;
339333
}
334+
#else
335+
return LOCKVAL(restart_in) != 0;
340336
#endif
341337
}
342338
return 0;
@@ -345,7 +341,7 @@ static inline int accel_restart_is_active(void)
345341
/* Creates a read lock for SHM access */
346342
static inline zend_result accel_activate_add(void)
347343
{
348-
#ifdef INCREMENT
344+
#ifdef ZEND_WIN32
349345
SHM_UNPROTECT();
350346
INCREMENT(mem_usage);
351347
SHM_PROTECT();
@@ -368,7 +364,7 @@ static inline zend_result accel_activate_add(void)
368364
/* Releases a lock for SHM access */
369365
static inline void accel_deactivate_sub(void)
370366
{
371-
#ifdef DECREMENT
367+
#ifdef ZEND_WIN32
372368
if (ZCG(counted)) {
373369
SHM_UNPROTECT();
374370
DECREMENT(mem_usage);
@@ -391,7 +387,7 @@ static inline void accel_deactivate_sub(void)
391387

392388
static inline void accel_unlock_all(void)
393389
{
394-
#ifdef DECREMENT
390+
#ifdef ZEND_WIN32
395391
accel_deactivate_sub();
396392
#else
397393
struct flock mem_usage_unlock_all;
@@ -832,7 +828,7 @@ static void accel_use_shm_interned_strings(void)
832828
HANDLE_UNBLOCK_INTERRUPTIONS();
833829
}
834830

835-
#ifndef LOCKVAL
831+
#ifndef ZEND_WIN32
836832
static inline void kill_all_lockers(struct flock *mem_usage_check)
837833
{
838834
int tries;
@@ -899,7 +895,7 @@ static inline void kill_all_lockers(struct flock *mem_usage_check)
899895

900896
static inline int accel_is_inactive(void)
901897
{
902-
#ifdef LOCKVAL
898+
#ifdef ZEND_WIN32
903899
if (LOCKVAL(mem_usage) == 0) {
904900
return SUCCESS;
905901
}

ext/opcache/ZendAccelerator.h

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -55,14 +55,6 @@
5555
#include "zend_accelerator_hash.h"
5656
#include "zend_accelerator_debug.h"
5757

58-
#ifdef HAVE_STDATOMIC_H
59-
# ifdef __cplusplus
60-
# include <atomic>
61-
# else
62-
# include <stdatomic.h>
63-
# endif
64-
#endif // HAVE_STDATOMIC_H
65-
6658
#ifndef PHPAPI
6759
# ifdef ZEND_WIN32
6860
# define PHPAPI __declspec(dllimport)
@@ -269,13 +261,7 @@ typedef struct _zend_accel_shared_globals {
269261
bool restart_pending;
270262
zend_accel_restart_reason restart_reason;
271263
bool cache_status_before_restart;
272-
#ifdef HAVE_STDATOMIC_H
273-
# ifdef __cplusplus
274-
std::atomic_llong mem_usage, restart_in;
275-
# else
276-
atomic_llong mem_usage, restart_in;
277-
# endif
278-
#elif defined(ZEND_WIN32)
264+
#ifdef ZEND_WIN32
279265
LONGLONG mem_usage;
280266
LONGLONG restart_in;
281267
#endif

ext/opcache/config.m4

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,6 @@ if test "$PHP_OPCACHE" != "no"; then
2323
dnl Always build as shared extension
2424
ext_shared=yes
2525

26-
AC_CHECK_HEADERS([stdatomic.h])
27-
2826
if test "$PHP_HUGE_CODE_PAGES" = "yes"; then
2927
AC_DEFINE(HAVE_HUGE_CODE_PAGES, 1, [Define to enable copying PHP CODE pages into HUGE PAGES (experimental)])
3028
fi

0 commit comments

Comments
 (0)