From 4c7b0b5185f3c2c76ed0676ceacb4baa8dd34b29 Mon Sep 17 00:00:00 2001 From: Tyson Andre Date: Sat, 5 Nov 2022 15:20:32 -0400 Subject: [PATCH] Reduce memory per shm string by 8 bytes 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 --- apc_cache.c | 38 +++++++++++++++++++++++--------------- apc_cache.h | 25 ++++++++++++++++++++++++- apc_iterator.c | 10 ++++++---- apc_persist.c | 46 +++++++++++++++++++++++----------------------- apc_sma.c | 2 +- tests/apc_020.phpt | 2 +- 6 files changed, 78 insertions(+), 45 deletions(-) diff --git a/apc_cache.c b/apc_cache.c index 1a9075e7..b6cfae20 100644 --- a/apc_cache.c +++ b/apc_cache.c @@ -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. @@ -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 ); } @@ -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); @@ -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 */ @@ -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 @@ -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; @@ -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; @@ -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); @@ -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; @@ -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); @@ -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); diff --git a/apc_cache.h b/apc_cache.h index e987a546..9f44db2b 100644 --- a/apc_cache.h +++ b/apc_cache.h @@ -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 { @@ -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 */ diff --git a/apc_iterator.c b/apc_iterator.c index f0b51397..581b6c07 100644 --- a/apc_iterator.c +++ b/apc_iterator.c @@ -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); @@ -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; diff --git a/apc_persist.c b/apc_persist.c index 21d3f1d1..54dd016d 100644 --- a/apc_persist.c +++ b/apc_persist.c @@ -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) @@ -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); } @@ -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; } @@ -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; @@ -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); @@ -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)); @@ -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; } @@ -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; @@ -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); @@ -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; } @@ -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)); diff --git a/apc_sma.c b/apc_sma.c index 51a3198d..8c6eb344 100644 --- a/apc_sma.c +++ b/apc_sma.c @@ -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; diff --git a/tests/apc_020.phpt b/tests/apc_020.phpt index ea388d31..865358a2 100644 --- a/tests/apc_020.phpt +++ b/tests/apc_020.phpt @@ -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++; }