Skip to content

Commit

Permalink
Reduce memory per shm string by 8 bytes
Browse files Browse the repository at this point in the history 
Related to #323

This slightly helps performance of `apc.serializer=default`
when there are lots of small strings to unpersist.
(and reduces size of serialized data by 8 bytes with other serializers)

With APCu's current design and eviction always being a possibility,
strings must always be copied.
(zval flags could be used to check the type if that stopped being the case)

For example, fetching an array mapping small strings to small strings:

	Before: bench_apcu_fetch_large_config: numValues=     65536
	repetitions=       256 elapsed=3.010 shared memory=   9381144
	After: bench_apcu_fetch_large_config: numValues=     65536
	repetitions=       256 elapsed=2.980 shared memory=   8332560
  • Loading branch information
@TysonAndre
TysonAndre committed Nov 5, 2022
1 parent a25dbce commit 4c7b0b5
Show file tree
Hide file tree
Showing 6 changed files with 78 additions and 45 deletions.
38 changes: 23 additions & 15 deletions apc_cache.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,11 +120,19 @@ static inline void apc_cache_hash_slot(
*hash = ZSTR_HASH(key);
*slot = *hash % cache->nslots;
} /* }}} */
/* {{{ apc_cache_hash_slot_persisted
Note: These calculations can and should be done outside of a lock */
static inline void apc_cache_hash_slot_persisted(
apc_cache_t* cache, apc_persisted_zend_string *key, zend_ulong* hash, size_t* slot) {
ZEND_ASSERT(ZSTR_H(key) != 0);
*hash = ZSTR_H(key);
*slot = *hash % cache->nslots;
} /* }}} */

static inline zend_bool apc_entry_key_equals(const apc_cache_entry_t *entry, zend_string *key, zend_ulong hash) {
return ZSTR_H(entry->key) == hash
&& ZSTR_LEN(entry->key) == ZSTR_LEN(key)
&& memcmp(ZSTR_VAL(entry->key), ZSTR_VAL(key), ZSTR_LEN(key)) == 0;
static inline zend_bool apc_entry_key_equals(const apc_cache_entry_t *entry, const char *key, const size_t len, zend_ulong hash) {
return ZSTR_H(entry->persisted_key) == hash
&& ZSTR_LEN(entry->persisted_key) == len
&& memcmp(ZSTR_VAL(entry->persisted_key), key, len) == 0;
}

/* An entry is hard expired if the creation time if older than the per-entry TTL.
Expand Down Expand Up @@ -200,7 +208,7 @@ static void apc_cache_wlocked_gc(apc_cache_t* cache)
if (dead->ref_count > 0) {
apc_debug(
"GC cache entry '%s' was on gc-list for %ld seconds",
ZSTR_VAL(dead->key), gc_sec
ZSTR_VAL(dead->persisted_key), gc_sec
);
}

Expand Down Expand Up @@ -260,7 +268,7 @@ PHP_APCU_API int APC_UNSERIALIZER_NAME(php) (APC_UNSERIALIZER_ARGS)
result = php_var_unserialize(value, &tmp, buf + buf_len, &var_hash);
PHP_VAR_UNSERIALIZE_DESTROY(var_hash);
BG(serialize_lock)--;

if (!result) {
php_error_docref(NULL, E_NOTICE, "Error at offset %ld of %ld bytes", (zend_long)(tmp - buf), (zend_long)buf_len);
ZVAL_NULL(value);
Expand Down Expand Up @@ -324,7 +332,7 @@ PHP_APCU_API apc_cache_t* apc_cache_create(apc_sma_t* sma, apc_serializer_t* ser

static inline zend_bool apc_cache_wlocked_insert(
apc_cache_t *cache, apc_cache_entry_t *new_entry, zend_bool exclusive) {
zend_string *key = new_entry->key;
apc_persisted_zend_string *key = new_entry->persisted_key;
time_t t = new_entry->ctime;

/* process deleted list */
Expand All @@ -337,12 +345,12 @@ static inline zend_bool apc_cache_wlocked_insert(
size_t s;

/* calculate hash and entry */
apc_cache_hash_slot(cache, key, &h, &s);
apc_cache_hash_slot_persisted(cache, key, &h, &s);

entry = &cache->slots[s];
while (*entry) {
/* check for a match by hash and string */
if (apc_entry_key_equals(*entry, key, h)) {
if (apc_entry_key_equals(*entry, ZSTR_VAL(key), ZSTR_LEN(key), h)) {
/*
* At this point we have found the user cache entry. If we are doing
* an exclusive insert (apc_add) we are going to bail right away if
Expand Down Expand Up @@ -424,7 +432,7 @@ static inline apc_cache_entry_t *apc_cache_rlocked_find_nostat(
entry = cache->slots[s];
while (entry) {
/* check for a matching key by has and identifier */
if (apc_entry_key_equals(entry, key, h)) {
if (apc_entry_key_equals(entry, ZSTR_VAL(key), ZSTR_LEN(key), h)) {
/* Check to make sure this entry isn't expired by a hard TTL */
if (apc_cache_entry_hard_expired(entry, t)) {
break;
Expand Down Expand Up @@ -452,7 +460,7 @@ static inline apc_cache_entry_t *apc_cache_rlocked_find(
entry = cache->slots[s];
while (entry) {
/* check for a matching key by has and identifier */
if (apc_entry_key_equals(entry, key, h)) {
if (apc_entry_key_equals(entry, ZSTR_VAL(key), ZSTR_LEN(key), h)) {
/* Check to make sure this entry isn't expired by a hard TTL */
if (apc_cache_entry_hard_expired(entry, t)) {
break;
Expand Down Expand Up @@ -980,7 +988,7 @@ PHP_APCU_API zend_bool apc_cache_delete(apc_cache_t *cache, zend_string *key)

while (*entry) {
/* check for a match by hash and identifier */
if (apc_entry_key_equals(*entry, key, h)) {
if (apc_entry_key_equals(*entry, ZSTR_VAL(key), ZSTR_LEN(key), h)) {
/* executing removal */
apc_cache_wlocked_remove_entry(cache, entry);

Expand Down Expand Up @@ -1009,7 +1017,7 @@ static void apc_cache_init_entry(
apc_cache_entry_t *entry, zend_string *key, const zval *val, const int32_t ttl, time_t t)
{
entry->ttl = ttl;
entry->key = key;
entry->tmp_key = key;
ZVAL_COPY_VALUE(&entry->val, val);

entry->next = NULL;
Expand Down Expand Up @@ -1041,7 +1049,7 @@ static zval apc_cache_link_info(apc_cache_t *cache, apc_cache_entry_t *p)
zval link, zv;
array_init(&link);

ZVAL_STR(&zv, zend_string_dup(p->key, 0));
ZVAL_STR(&zv, apc_create_zend_string_from_persisted(p->persisted_key));
zend_hash_add_new(Z_ARRVAL(link), apc_str_info, &zv);

array_add_long(&link, apc_str_ttl, p->ttl);
Expand Down Expand Up @@ -1158,7 +1166,7 @@ PHP_APCU_API void apc_cache_stat(apc_cache_t *cache, zend_string *key, zval *sta

while (entry) {
/* check for a matching key by has and identifier */
if (apc_entry_key_equals(entry, key, h)) {
if (apc_entry_key_equals(entry, ZSTR_VAL(key), ZSTR_LEN(key), h)) {
array_init(stat);
array_add_long(stat, apc_str_hits, entry->nhits);
array_add_long(stat, apc_str_access_time, entry->atime);
Expand Down
25 changes: 24 additions & 1 deletion apc_cache.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
#include "apc_lock.h"
#include "apc_globals.h"
#include "TSRM.h"
#include "Zend/zend_string.h"

typedef struct apc_cache_slam_key_t apc_cache_slam_key_t;
struct apc_cache_slam_key_t {
Expand All @@ -46,10 +47,32 @@ struct apc_cache_slam_key_t {
#endif
};

typedef struct _apc_persisted_zend_string {
zend_ulong h; /* hash value */
size_t len;
char val[1];
} apc_persisted_zend_string;

/* {{{ apc_create_zend_string_from_persisted */
static zend_always_inline zend_string *apc_create_zend_string_from_persisted(const apc_persisted_zend_string *str) {
const size_t len = str->len;
ZEND_ASSERT(str->h != 0);
zend_string *ret = zend_string_alloc(len, 0);
ZSTR_H(ret) = str->h;

memcpy(ZSTR_VAL(ret), str->val, len + 1);
ZEND_ASSERT(ZSTR_VAL(ret)[len] == '\0');
return ret;
}
/* }}} */

/* {{{ struct definition: apc_cache_entry_t */
typedef struct apc_cache_entry_t apc_cache_entry_t;
struct apc_cache_entry_t {
zend_string *key; /* entry key */
union {
zend_string *tmp_key; /* entry key */
apc_persisted_zend_string *persisted_key; /* entry key */
};
zval val; /* the zval copied at store time */
apc_cache_entry_t *next; /* next entry in linked list */
zend_long ttl; /* the ttl on this specific entry */
Expand Down
10 changes: 6 additions & 4 deletions apc_iterator.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,9 @@ static apc_iterator_item_t* apc_iterator_item_ctor(
array_init(&item->value);
ht = Z_ARRVAL(item->value);

item->key = zend_string_dup(entry->key, 0);
//fprintf(stderr, "Creating from %p %s\n", entry->persisted_key, ZSTR_VAL(entry->persisted_key));
item->key = apc_create_zend_string_from_persisted(entry->persisted_key);
//fprintf(stderr, "Created %s\n", ZSTR_VAL(item->key));

if (APC_ITER_TYPE & iterator->format) {
ZVAL_STR_COPY(&zv, apc_str_user);
Expand Down Expand Up @@ -178,18 +180,18 @@ static int apc_iterator_search_match(apc_iterator_t *iterator, apc_cache_entry_t
#if PHP_VERSION_ID >= 70300
rval = pcre2_match(
php_pcre_pce_re(iterator->pce),
(PCRE2_SPTR) ZSTR_VAL(entry->key), ZSTR_LEN(entry->key),
(PCRE2_SPTR) ZSTR_VAL(entry->persisted_key), ZSTR_LEN(entry->persisted_key),
0, 0, iterator->re_match_data, php_pcre_mctx()) >= 0;
#else
rval = pcre_exec(
iterator->pce->re, iterator->pce->extra,
ZSTR_VAL(entry->key), ZSTR_LEN(entry->key),
ZSTR_VAL(entry->persisted_key), ZSTR_LEN(entry->persisted_key),
0, 0, NULL, 0) >= 0;
#endif
}

if (iterator->search_hash) {
rval = zend_hash_exists(iterator->search_hash, entry->key);
rval = zend_hash_str_exists(iterator->search_hash, ZSTR_VAL(entry->persisted_key), ZSTR_LEN(entry->persisted_key));
}

return rval;
Expand Down
46 changes: 23 additions & 23 deletions apc_persist.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,9 @@ typedef struct _apc_persist_context_t {
HashTable already_allocated;
} apc_persist_context_t;

#define _APC_PERSISTED_ZSTR_STRUCT_SIZE(len) (XtOffsetOf(apc_persisted_zend_string, val) + len + 1)
#define ADD_SIZE(sz) ctxt->size += ZEND_MM_ALIGNED_SIZE(sz)
#define ADD_SIZE_STR(len) ADD_SIZE(_ZSTR_STRUCT_SIZE(len))
#define ADD_SIZE_STR(len) ADD_SIZE(_APC_PERSISTED_ZSTR_STRUCT_SIZE(len))

#define ALLOC(sz) apc_persist_alloc(ctxt, sz)
#define COPY(val, sz) apc_persist_alloc_copy(ctxt, val, sz)
Expand Down Expand Up @@ -233,7 +234,7 @@ static zend_bool apc_persist_calc_zval(apc_persist_context_t *ctxt, const zval *

static zend_bool apc_persist_calc(apc_persist_context_t *ctxt, const apc_cache_entry_t *entry) {
ADD_SIZE(sizeof(apc_cache_entry_t));
ADD_SIZE_STR(ZSTR_LEN(entry->key));
ADD_SIZE_STR(ZSTR_LEN(entry->tmp_key));
return apc_persist_calc_zval(ctxt, &entry->val);
}

Expand Down Expand Up @@ -265,31 +266,27 @@ static inline void *apc_persist_alloc_copy(
return ptr;
}

static zend_string *apc_persist_copy_cstr(
static apc_persisted_zend_string *apc_persist_copy_cstr(
apc_persist_context_t *ctxt, const char *orig_buf, size_t buf_len, zend_ulong hash) {
zend_string *str = ALLOC(_ZSTR_STRUCT_SIZE(buf_len));
apc_persisted_zend_string *str = ALLOC(_APC_PERSISTED_ZSTR_STRUCT_SIZE(buf_len));

GC_SET_REFCOUNT(str, 1);
GC_SET_PERSISTENT_TYPE(str, IS_STRING);

ZSTR_H(str) = hash;
ZSTR_LEN(str) = buf_len;
memcpy(ZSTR_VAL(str), orig_buf, buf_len);
ZSTR_VAL(str)[buf_len] = '\0';
zend_string_hash_val(str);
str->h = hash ? hash : zend_inline_hash_func(orig_buf, buf_len);
str->len = buf_len;
memcpy(str->val, orig_buf, buf_len);
str->val[buf_len] = '\0';

return str;
}

static zend_string *apc_persist_copy_zstr_no_add(
static apc_persisted_zend_string *apc_persist_copy_zstr_no_add(
apc_persist_context_t *ctxt, const zend_string *orig_str) {
return apc_persist_copy_cstr(
ctxt, ZSTR_VAL(orig_str), ZSTR_LEN(orig_str), ZSTR_H(orig_str));
}

static inline zend_string *apc_persist_copy_zstr(
static inline apc_persisted_zend_string *apc_persist_copy_zstr(
apc_persist_context_t *ctxt, const zend_string *orig_str) {
zend_string *str = apc_persist_copy_zstr_no_add(ctxt, orig_str);
apc_persisted_zend_string *str = apc_persist_copy_zstr_no_add(ctxt, orig_str);
apc_persist_add_already_allocated(ctxt, orig_str, str);
return str;
}
Expand Down Expand Up @@ -372,7 +369,7 @@ static zend_array *apc_persist_copy_ht(apc_persist_context_t *ctxt, const HashTa
}

if (p->key) {
p->key = apc_persist_copy_zstr_no_add(ctxt, p->key);
p->key = (zend_string*)apc_persist_copy_zstr_no_add(ctxt, p->key);
ht->u.flags &= ~HASH_FLAG_STATIC_KEYS;
} else if ((zend_long) p->h >= (zend_long) ht->nNextFreeElement) {
ht->nNextFreeElement = p->h + 1;
Expand All @@ -387,7 +384,7 @@ static zend_array *apc_persist_copy_ht(apc_persist_context_t *ctxt, const HashTa

static void apc_persist_copy_serialize(
apc_persist_context_t *ctxt, zval *zv) {
zend_string *str;
apc_persisted_zend_string *str;
zend_uchar orig_type = Z_TYPE_P(zv);
ZEND_ASSERT(orig_type == IS_ARRAY || orig_type == IS_OBJECT);

Expand All @@ -412,7 +409,8 @@ static void apc_persist_copy_zval_impl(apc_persist_context_t *ctxt, zval *zv) {
switch (Z_TYPE_P(zv)) {
case IS_STRING:
if (!ptr) ptr = apc_persist_copy_zstr(ctxt, Z_STR_P(zv));
ZVAL_STR(zv, ptr);
Z_STR_P(zv) = ptr;
Z_TYPE_INFO_P(zv) = IS_STRING_EX; /* Never an interned string */
return;
case IS_ARRAY:
if (!ptr) ptr = apc_persist_copy_ht(ctxt, Z_ARRVAL_P(zv));
Expand All @@ -429,7 +427,7 @@ static void apc_persist_copy_zval_impl(apc_persist_context_t *ctxt, zval *zv) {
static apc_cache_entry_t *apc_persist_copy(
apc_persist_context_t *ctxt, const apc_cache_entry_t *orig_entry) {
apc_cache_entry_t *entry = COPY(orig_entry, sizeof(apc_cache_entry_t));
entry->key = apc_persist_copy_zstr_no_add(ctxt, entry->key);
entry->persisted_key = apc_persist_copy_zstr_no_add(ctxt, entry->tmp_key);
apc_persist_copy_zval(ctxt, &entry->val);
return entry;
}
Expand Down Expand Up @@ -513,7 +511,7 @@ static inline void apc_unpersist_zval(apc_unpersist_context_t *ctxt, zval *zv) {
}

static zend_bool apc_unpersist_serialized(
zval *dst, zend_string *str, apc_serializer_t *serializer) {
zval *dst, apc_persisted_zend_string *str, apc_serializer_t *serializer) {
apc_unserialize_t unserialize = APC_UNSERIALIZER_NAME(php);
void *config = NULL;

Expand Down Expand Up @@ -544,7 +542,8 @@ static inline void apc_unpersist_add_already_copied(
}
}

static zend_string *apc_unpersist_zstr(apc_unpersist_context_t *ctxt, const zend_string *orig_str) {
static zend_string *apc_unpersist_zstr(apc_unpersist_context_t *ctxt, const apc_persisted_zend_string *orig_str) {
ZEND_ASSERT(ZSTR_LEN(orig_str) <= 1000000);
zend_string *str = zend_string_init(ZSTR_VAL(orig_str), ZSTR_LEN(orig_str), 0);
ZSTR_H(str) = ZSTR_H(orig_str);
apc_unpersist_add_already_copied(ctxt, orig_str, str);
Expand Down Expand Up @@ -624,7 +623,7 @@ static zend_array *apc_unpersist_ht(
p->val = q->val;
p->h = q->h;
if (q->key) {
p->key = zend_string_dup(q->key, 0);
p->key = apc_create_zend_string_from_persisted((apc_persisted_zend_string *)q->key);
} else {
p->key = NULL;
}
Expand All @@ -645,7 +644,8 @@ static void apc_unpersist_zval_impl(apc_unpersist_context_t *ctxt, zval *zv) {

switch (Z_TYPE_P(zv)) {
case IS_STRING:
Z_STR_P(zv) = apc_unpersist_zstr(ctxt, Z_STR_P(zv));
/* apc_persist_copy_zval will have properly set the flags already */
Z_STR_P(zv) = apc_unpersist_zstr(ctxt, (apc_persisted_zend_string *)Z_STR_P(zv));
return;
case IS_REFERENCE:
Z_REF_P(zv) = apc_unpersist_ref(ctxt, Z_REF_P(zv));
Expand Down
2 changes: 1 addition & 1 deletion apc_sma.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ static APC_HOTSPOT size_t sma_allocate(sma_header_t *header, size_t size, size_t
realsize = ALIGNWORD(size + block_size);

/*
* First, insure that the segment contains at least realsize free bytes,
* First, ensure that the segment contains at least realsize free bytes,
* even if they are not contiguous.
*/
shmaddr = header;
Expand Down
2 changes: 1 addition & 1 deletion tests/apc_020.phpt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ apcu_inc_request_time(1);
// Fill the cache
$i = 0;
while (apcu_exists("dummy")) {
apcu_store("key" . $i, str_repeat('x', 500));
apcu_store("key" . $i, str_repeat('x', 700)); // whether this test passes depends on the size of items, because that affects apc_cache_default_expunge needing a full eviction
$i++;
}

Expand Down

0 comments on commit 4c7b0b5

Please sign in to comment.