ext/session: start to implement GH-14019 #17575
Conversation
|
Stupid question: why glib? Could we get away with using the internals HashTable with shared memory? |
|
the glib hashtable implementation is multiplatform, solid and fast so it lessen the burden of implementation. |
devnexen
force-pushed
the
session_mem_rewrite
branch
from
f8bd3cf to
d4c9c26
Compare
Girgias
left a comment
Girgias
left a comment
There was a problem hiding this comment.
This seems reasonable, but I'm not a ZTS specialist, I would like @arnaud-lb opinion on the ZTS aspect. :)
ext/session/mod_mm.c
Outdated
| zend_result ret; | ||
|
|
||
| ps_mm_instance = calloc(sizeof(*ps_mm_instance), 1); | ||
| ps_mm_instance = calloc(1, sizeof(*ps_mm_instance)); |
There was a problem hiding this comment.
Could this be ecalloc() or must it be the system malloc?
There was a problem hiding this comment.
hmmm not sure we can use zendmm that early but I ll try later
There was a problem hiding this comment.
It must be a persistent allocator (system one or pecalloc(..., true))
| #include "php.h" | ||
|
|
||
| #ifdef HAVE_LIBMM | ||
| #ifdef HAVE_LIBGLIB |
There was a problem hiding this comment.
You need similar changes in ext/session/session.c, otherwise mod_mm is not enabled.
| { | ||
| data->owner = getpid(); | ||
| data->mm = mm_create(0, path); | ||
| data->mm = g_mapped_file_new(path, TRUE, NULL); |
There was a problem hiding this comment.
This fails on my machine when path doesn't exist
| data->hash_cnt = 0; | ||
| data->hash_max = 511; | ||
| data->hash = mm_calloc(data->mm, data->hash_max + 1, sizeof(ps_sd *)); | ||
| data->hash = g_hash_table_new(ps_mm_hash, ps_mm_key_equals); |
There was a problem hiding this comment.
I'm not familiar with glib, does the call to g_mapped_file_new() above imply that every g_ function will use that mapped file for allocation?
|
The approach for ZTS supports seems fine, but I don't understand how this is using shared memory. Could you clarify @devnexen? |
|
I think I may have been "extra zealous" regarding ZTS but I ll look after your remarks sometime this week end |
4 participants
No description provided.