Skip to content

Commit

Permalink
store: track HashPosition for first and last elements (#134)
Browse files Browse the repository at this point in the history 
This produces a significant performance improvement for queues with large internal tables.

An internal table of large size may appear if the array had lots of elements inserted into it and later deleted.
This resulted in major performance losses for the reader of the elements, as `zend_hash_internal_pointer_reset_ex()` had to scan through many `IS_UNDEF` offsets to find the actual first element.

There are two ways to attack this problem:

1) reallocate the internal table as elements are deleted to reduce the internal table size - this proved to be relatively ineffective
2) track the start and end of the hashtable to avoid repeated scans during every shift() call - this is the approach taken in this commit, and provides major performance benefits

The test case written in #42 now runs to completion substantially faster, without any performance degradation.
  • Loading branch information
@dktapps
dktapps authored Nov 15, 2023
1 parent c58ba9b commit b2b6100
Show file tree
Hide file tree
Showing 2 changed files with 76 additions and 11 deletions.
85 changes: 74 additions & 11 deletions src/store.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,19 @@ static void pmmpthread_store_restore_zval_ex(zval* unstore, zval* zstorage, zend
static void pmmpthread_store_restore_zval(zval* unstore, zval* zstorage); /* }}} */
static void pmmpthread_store_storage_dtor(zval* element);

/* {{{ */
static inline void pmmpthread_store_invalidate_bounds(pmmpthread_store_t* store) {
store->first = HT_INVALID_IDX;
store->last = HT_INVALID_IDX;
} /* }}} */

/* {{{ */
void pmmpthread_store_init(pmmpthread_store_t* store) {
store->modcount = 0;
zend_hash_init(
&store->hash, 8, NULL,
(dtor_func_t)pmmpthread_store_storage_dtor, 1);
pmmpthread_store_invalidate_bounds(store);
} /* }}} */

/* {{{ */
Expand Down Expand Up @@ -286,6 +293,9 @@ int pmmpthread_store_delete(zend_object *object, zval *key) {
if (result == SUCCESS && was_pmmpthread_object) {
_pmmpthread_store_bump_modcount_nolock(threaded);
}
//TODO: it would be better if we can update this, if we deleted the first element
pmmpthread_store_invalidate_bounds(&ts_obj->props);

//TODO: sync local properties?
pmmpthread_monitor_unlock(&ts_obj->monitor);
} else result = FAILURE;
Expand Down Expand Up @@ -538,6 +548,24 @@ int pmmpthread_store_write(zend_object *object, zval *key, zval *write, zend_boo
if (result == SUCCESS && was_pmmpthread_object) {
_pmmpthread_store_bump_modcount_nolock(threaded);
}
if (ts_obj->props.first != HT_INVALID_IDX && ts_obj->props.first != 0) {
HashPosition start = 0;
if (zend_hash_get_current_data_ex(&ts_obj->props.hash, &start) != NULL) {
//a table rehash may have occurred, moving all elements to the start of the table
//this is usually because the table size was increased to accommodate the new element
pmmpthread_store_invalidate_bounds(&ts_obj->props);
}
}
if (key) {
//only invalidate position if an arbitrary key was used
//if the item was appended, the first element was either unchanged or the position was invalid anyway
pmmpthread_store_invalidate_bounds(&ts_obj->props);
} else if (ts_obj->props.last != HT_INVALID_IDX) {
//if we appended, the last element is now the new item
if (zend_hash_move_forward_ex(&ts_obj->props.hash, &ts_obj->props.last) == FAILURE) {
ZEND_ASSERT(0);
}
}
//this isn't necessary for any specific property write, but since we don't have any other way to clean up local
//cached ThreadSafe references that are dead, we have to take the opportunity
pmmpthread_store_sync_local_properties(object);
Expand Down Expand Up @@ -576,12 +604,22 @@ int pmmpthread_store_shift(zend_object *object, zval *member) {

if (pmmpthread_monitor_lock(&ts_obj->monitor)) {
zval key;
HashPosition position;
zval *zstorage;
HashPosition position = ts_obj->props.first;

zend_hash_internal_pointer_reset_ex(&ts_obj->props.hash, &position);
if (position == HT_INVALID_IDX) {
zend_hash_internal_pointer_reset_ex(&ts_obj->props.hash, &position);
}
if ((zstorage = zend_hash_get_current_data_ex(&ts_obj->props.hash, &position))) {
zend_hash_get_current_key_zval_ex(&ts_obj->props.hash, &key, &position);

if (zend_hash_num_elements(&ts_obj->props.hash) == 1) { //we're about to delete the last element
pmmpthread_store_invalidate_bounds(&ts_obj->props);
} else {
zend_hash_move_forward_ex(&ts_obj->props.hash, &position);
ts_obj->props.first = position;
}

zend_bool may_be_locally_cached;

pmmpthread_store_restore_zval_ex(member, zstorage, &may_be_locally_cached);
Expand Down Expand Up @@ -617,18 +655,28 @@ int pmmpthread_store_chunk(zend_object *object, zend_long size, zend_bool preser
pmmpthread_object_t *ts_obj = threaded->ts_obj;

if (pmmpthread_monitor_lock(&ts_obj->monitor)) {
HashPosition position;
zval *zstorage;

HashPosition position = ts_obj->props.first;
if (position == HT_INVALID_IDX) {
zend_hash_internal_pointer_reset_ex(&ts_obj->props.hash, &position);
}
array_init(chunk);
zend_hash_internal_pointer_reset_ex(&ts_obj->props.hash, &position);
zend_bool stale_local_cache = 0;
while((zend_hash_num_elements(Z_ARRVAL_P(chunk)) < size) &&
(zstorage = zend_hash_get_current_data_ex(&ts_obj->props.hash, &position))) {

zval key, zv;

zend_hash_get_current_key_zval_ex(&ts_obj->props.hash, &key, &position);

if (zend_hash_num_elements(&ts_obj->props.hash) == 1) { //we're about to delete the last element
pmmpthread_store_invalidate_bounds(&ts_obj->props);
} else {
zend_hash_move_forward_ex(&ts_obj->props.hash, &position);
ts_obj->props.first = position;
}

zend_bool may_be_locally_cached;
pmmpthread_store_restore_zval_ex(&zv, zstorage, &may_be_locally_cached);
if (!stale_local_cache) {
Expand All @@ -651,8 +699,6 @@ int pmmpthread_store_chunk(zend_object *object, zend_long size, zend_bool preser
}
zend_string_release(Z_STR(key));
}

zend_hash_internal_pointer_reset_ex(&ts_obj->props.hash, &position);
}
if (stale_local_cache) {
_pmmpthread_store_bump_modcount_nolock(threaded);
Expand All @@ -673,13 +719,22 @@ int pmmpthread_store_pop(zend_object *object, zval *member) {

if (pmmpthread_monitor_lock(&ts_obj->monitor)) {
zval key;
HashPosition position;
zval *zstorage;
HashPosition position = ts_obj->props.last;

zend_hash_internal_pointer_end_ex(&ts_obj->props.hash, &position);
if (position == HT_INVALID_IDX) {
zend_hash_internal_pointer_end_ex(&ts_obj->props.hash, &position);
}
if ((zstorage = zend_hash_get_current_data_ex(&ts_obj->props.hash, &position))) {
zend_hash_get_current_key_zval_ex(&ts_obj->props.hash, &key, &position);

if (zend_hash_num_elements(&ts_obj->props.hash) == 1) { //we're about to delete the last element
pmmpthread_store_invalidate_bounds(&ts_obj->props);
} else {
zend_hash_move_backwards_ex(&ts_obj->props.hash, &position);
ts_obj->props.last = position;
}

zend_bool may_be_locally_cached;
pmmpthread_store_restore_zval_ex(member, zstorage, &may_be_locally_cached);

Expand Down Expand Up @@ -1195,6 +1250,8 @@ int pmmpthread_store_merge(zend_object *destination, zval *from, zend_bool overw
if (overwrote_pmmpthread_object) {
_pmmpthread_store_bump_modcount_nolock(PMMPTHREAD_FETCH_FROM(destination));
}
pmmpthread_store_invalidate_bounds(&threaded[0]->props);

//TODO: sync local properties?

pmmpthread_monitor_unlock(&threaded[1]->monitor);
Expand Down Expand Up @@ -1313,9 +1370,15 @@ void pmmpthread_store_reset(zend_object *object, HashPosition *position) {
pmmpthread_object_t *ts_obj = PMMPTHREAD_FETCH_TS_FROM(object);

if (pmmpthread_monitor_lock(&ts_obj->monitor)) {
zend_hash_internal_pointer_reset_ex(&ts_obj->props.hash, position);
if (zend_hash_has_more_elements_ex(&ts_obj->props.hash, position) == FAILURE) { //empty
*position = HT_INVALID_IDX;
if (ts_obj->props.first == HT_INVALID_IDX) {
zend_hash_internal_pointer_reset_ex(&ts_obj->props.hash, position);
if (zend_hash_has_more_elements_ex(&ts_obj->props.hash, position) == FAILURE) { //empty
*position = HT_INVALID_IDX;
} else {
ts_obj->props.first = *position;
}
} else {
*position = ts_obj->props.first;
}
pmmpthread_monitor_unlock(&ts_obj->monitor);
}
Expand Down
2 changes: 2 additions & 0 deletions src/store.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@
typedef struct _pmmpthread_store_t {
HashTable hash;
zend_long modcount;
HashPosition first;
HashPosition last;
} pmmpthread_store_t;

void pmmpthread_store_init(pmmpthread_store_t* store);
Expand Down

0 comments on commit b2b6100

Please sign in to comment.