Skip to content

Commit

Permalink
Improved shared interned strings handling. The previous implementatio…
Browse files Browse the repository at this point in the history 
…n worked incorrectly in ZTS build. It changed strings only in function/class tables of one thread. Now all threads gets the same shared interned strings. Also, on shutdown, we don't try to replace SHM interned strings back to process strings, but delay dettachment of SHM instead.
  • Loading branch information
@dstogov
dstogov committed Oct 25, 2018
1 parent f33da6f commit 33e777a
Show file tree
Hide file tree
Showing 6 changed files with 42 additions and 50 deletions.
23 changes: 14 additions & 9 deletions Zend/zend.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ void (*zend_printf_to_smart_str)(smart_str *buf, const char *format, va_list ap)
ZEND_API char *(*zend_getenv)(char *name, size_t name_len);
ZEND_API zend_string *(*zend_resolve_path)(const char *filename, size_t filename_len);
ZEND_API int (*zend_post_startup_cb)(void) = NULL;
ZEND_API void (*zend_post_shutdown_cb)(void) = NULL;

void (*zend_on_timeout)(int seconds);

Expand Down Expand Up @@ -954,7 +955,18 @@ int zend_post_startup(void) /* {{{ */

zend_compiler_globals *compiler_globals = ts_resource(compiler_globals_id);
zend_executor_globals *executor_globals = ts_resource(executor_globals_id);
#endif

if (zend_post_startup_cb) {
int (*cb)(void) = zend_post_startup_cb;

zend_post_startup_cb = NULL;
if (cb() != SUCCESS) {
return FAILURE;
}
}

#ifdef ZTS
*GLOBAL_FUNCTION_TABLE = *compiler_globals->function_table;
*GLOBAL_CLASS_TABLE = *compiler_globals->class_table;
*GLOBAL_CONSTANTS_TABLE = *executor_globals->zend_constants;
Expand All @@ -981,15 +993,6 @@ int zend_post_startup(void) /* {{{ */
global_map_ptr_last = CG(map_ptr_last);
#endif

if (zend_post_startup_cb) {
int (*cb)(void) = zend_post_startup_cb;

zend_post_startup_cb = NULL;
if (cb() != SUCCESS) {
return FAILURE;
}
}

return SUCCESS;
}
/* }}} */
Expand Down Expand Up @@ -1025,6 +1028,8 @@ void zend_shutdown(void) /* {{{ */
GLOBAL_CLASS_TABLE = NULL;
GLOBAL_AUTO_GLOBALS_TABLE = NULL;
GLOBAL_CONSTANTS_TABLE = NULL;
ts_free_id(executor_globals_id);
ts_free_id(compiler_globals_id);
#else
if (CG(map_ptr_base)) {
free(CG(map_ptr_base));
Expand Down
3 changes: 3 additions & 0 deletions Zend/zend.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,10 @@ extern void (*zend_printf_to_smart_string)(smart_string *buf, const char *format
extern void (*zend_printf_to_smart_str)(smart_str *buf, const char *format, va_list ap);
extern ZEND_API char *(*zend_getenv)(char *name, size_t name_len);
extern ZEND_API zend_string *(*zend_resolve_path)(const char *filename, size_t filename_len);

/* These two callbacks are especially for opcache */
extern ZEND_API int (*zend_post_startup_cb)(void);
extern ZEND_API void (*zend_post_shutdown_cb)(void);

ZEND_API ZEND_COLD void zend_error(int type, const char *format, ...) ZEND_ATTRIBUTE_FORMAT(printf, 2, 3);
ZEND_API ZEND_COLD void zend_throw_error(zend_class_entry *exception_ce, const char *format, ...) ZEND_ATTRIBUTE_FORMAT(printf, 2, 3);
Expand Down
16 changes: 0 additions & 16 deletions Zend/zend_string.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,6 @@ static HashTable interned_strings_permanent;

static zend_new_interned_string_func_t interned_string_request_handler = zend_new_interned_string_request;
static zend_string_init_interned_func_t interned_string_init_request_handler = zend_string_init_interned_request;
static zend_string_copy_storage_func_t interned_string_copy_storage = NULL;
static zend_string_copy_storage_func_t interned_string_restore_storage = NULL;

ZEND_API zend_string *zend_empty_string = NULL;
ZEND_API zend_string *zend_one_char_string[256];
Expand Down Expand Up @@ -83,8 +81,6 @@ ZEND_API void zend_interned_strings_init(void)

interned_string_request_handler = zend_new_interned_string_request;
interned_string_init_request_handler = zend_string_init_interned_request;
interned_string_copy_storage = NULL;
interned_string_restore_storage = NULL;

zend_empty_string = NULL;
zend_known_strings = NULL;
Expand Down Expand Up @@ -301,26 +297,14 @@ ZEND_API void zend_interned_strings_set_request_storage_handlers(zend_new_intern
interned_string_init_request_handler = init_handler;
}

ZEND_API void zend_interned_strings_set_permanent_storage_copy_handlers(zend_string_copy_storage_func_t copy_handler, zend_string_copy_storage_func_t restore_handler)
{
interned_string_copy_storage = copy_handler;
interned_string_restore_storage = restore_handler;
}

ZEND_API void zend_interned_strings_switch_storage(zend_bool request)
{
if (request) {
if (interned_string_copy_storage) {
interned_string_copy_storage();
}
zend_new_interned_string = interned_string_request_handler;
zend_string_init_interned = interned_string_init_request_handler;
} else {
zend_new_interned_string = zend_new_interned_string_permanent;
zend_string_init_interned = zend_string_init_interned_permanent;
if (interned_string_restore_storage) {
interned_string_restore_storage();
}
}
}

Expand Down
1 change: 0 additions & 1 deletion Zend/zend_string.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ ZEND_API void zend_interned_strings_dtor(void);
ZEND_API void zend_interned_strings_activate(void);
ZEND_API void zend_interned_strings_deactivate(void);
ZEND_API void zend_interned_strings_set_request_storage_handlers(zend_new_interned_string_func_t handler, zend_string_init_interned_func_t init_handler);
ZEND_API void zend_interned_strings_set_permanent_storage_copy_handlers(zend_string_copy_storage_func_t copy_handler, zend_string_copy_storage_func_t restore_handler);
ZEND_API void zend_interned_strings_switch_storage(zend_bool request);

ZEND_API extern zend_string *zend_empty_string;
Expand Down
42 changes: 18 additions & 24 deletions ext/opcache/ZendAccelerator.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -726,19 +726,6 @@ static zend_string* ZEND_FASTCALL accel_replace_string_by_shm_permanent(zend_str
return str;
}

static zend_string* ZEND_FASTCALL accel_replace_string_by_process_permanent(zend_string *str)
{
zend_string *ret = zend_interned_string_find_permanent(str);

if (ret) {
zend_string_release(str);
return ret;
}
ZEND_ASSERT(0);
return str;
}


static void accel_use_shm_interned_strings(void)
{
HANDLE_BLOCK_INTERRUPTIONS();
Expand All @@ -760,11 +747,6 @@ static void accel_use_shm_interned_strings(void)
HANDLE_UNBLOCK_INTERRUPTIONS();
}

static void accel_use_permanent_interned_strings(void)
{
accel_copy_permanent_strings(accel_replace_string_by_process_permanent);
}

#ifndef ZEND_WIN32
static inline void kill_all_lockers(struct flock *mem_usage_check)
{
Expand Down Expand Up @@ -2553,8 +2535,6 @@ static int zend_accel_init_shm(void)
STRTAB_INVALID_POS,
(char*)ZCSG(interned_strings).start -
((char*)&ZCSG(interned_strings) + sizeof(zend_string_table)));

zend_interned_strings_set_permanent_storage_copy_handlers(accel_use_shm_interned_strings, accel_use_permanent_interned_strings);
}

zend_interned_strings_set_request_storage_handlers(accel_new_interned_string_for_php, accel_init_interned_string_for_php);
Expand Down Expand Up @@ -2777,6 +2757,9 @@ static int accel_startup(zend_extension *extension)
orig_post_startup_cb = zend_post_startup_cb;
zend_post_startup_cb = accel_post_startup;

/* Prevent unloadig */
extension->handle = 0;

return SUCCESS;
}

Expand Down Expand Up @@ -2817,9 +2800,6 @@ static int accel_post_startup(void)
case SUCCESSFULLY_REATTACHED:
zend_shared_alloc_lock();
accel_shared_globals = (zend_accel_shared_globals *) ZSMMG(app_shared_globals);
if (ZCG(accel_directives).interned_strings_buffer) {
zend_interned_strings_set_permanent_storage_copy_handlers(accel_use_shm_interned_strings, accel_use_permanent_interned_strings);
}
zend_interned_strings_set_request_storage_handlers(accel_new_interned_string_for_php, accel_init_interned_string_for_php);
zend_shared_alloc_unlock();
break;
Expand Down Expand Up @@ -2915,9 +2895,20 @@ static int accel_post_startup(void)

zend_optimizer_startup();

if (!file_cache_only && ZCG(accel_directives).interned_strings_buffer) {
accel_use_shm_interned_strings();
}

return SUCCESS;
}

static void (*orig_post_shutdown_cb)(void);

static void accel_post_shutdown(void)
{
zend_shared_alloc_shutdown();
}

void accel_shutdown(void)
{
zend_ini_entry *ini_entry;
Expand Down Expand Up @@ -2945,8 +2936,11 @@ void accel_shutdown(void)
#endif

if (!_file_cache_only) {
zend_shared_alloc_shutdown();
/* Delay SHM dettach */
orig_post_shutdown_cb = zend_post_shutdown_cb;
zend_post_shutdown_cb = accel_post_shutdown;
}

zend_compile_file = accelerator_orig_compile_file;

if ((ini_entry = zend_hash_str_find_ptr(EG(ini_directives), "include_path", sizeof("include_path")-1)) != NULL) {
Expand Down
7 changes: 7 additions & 0 deletions main/main.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2520,6 +2520,13 @@ void php_module_shutdown(void)
zend_interned_strings_dtor();
#endif

if (zend_post_shutdown_cb) {
void (*cb)(void) = zend_post_shutdown_cb;

zend_post_shutdown_cb = NULL;
cb();
}

module_initialized = 0;

#ifndef ZTS
Expand Down

0 comments on commit 33e777a

Please sign in to comment.