Skip to content

Add specialized pair construction API #3990

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Add specialized pair construction API #3990

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions Zend/zend_hash.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,26 @@ ZEND_API HashTable* ZEND_FASTCALL _zend_new_array(uint32_t nSize)
return ht;
}

ZEND_API HashTable* ZEND_FASTCALL zend_new_pair(zval *val1, zval *val2)
{
Bucket *p;
HashTable *ht = emalloc(sizeof(HashTable));
Copy link
Member

@krakjoe krakjoe Mar 26, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How aggressive do you want to be here ?

Might it be possible to make alloc for ht+buckets one allocation, might it also be possible to omit hash_init_int, and write a hash_init_pair (in place of real_init_packed) ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would love to combine those allocations, but I don't think it's possible. The rest of the hash API assumes that these are separate allocations, and in the end the pair is still a normal array and can be used as such.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a shame ... what about a specialized init ? among other things, init_int sets flags and then immediately real_init_packed sets them otherwise ? it looks like you could avoid the persistent branch in real_init too ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would hope that all of this will get optimized because these functions are always_inline, but I'll have to check assembly to be sure.

_zend_hash_init_int(ht, HT_MIN_SIZE, ZVAL_PTR_DTOR, 0);
ht->nNumUsed = ht->nNumOfElements = ht->nNextFreeElement = 2;
zend_hash_real_init_packed_ex(ht);

p = ht->arData;
ZVAL_COPY_VALUE(&p->val, val1);
p->h = 0;
p->key = NULL;

p++;
ZVAL_COPY_VALUE(&p->val, val2);
p->h = 1;
p->key = NULL;
return ht;
}

static void ZEND_FASTCALL zend_hash_packed_grow(HashTable *ht)
{
HT_ASSERT_RC1(ht);
Expand Down
1 change: 1 addition & 0 deletions Zend/zend_hash.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,7 @@ ZEND_API int ZEND_FASTCALL zend_hash_rehash(HashTable *ht);

ZEND_API HashTable* ZEND_FASTCALL _zend_new_array_0(void);
ZEND_API HashTable* ZEND_FASTCALL _zend_new_array(uint32_t size);
ZEND_API HashTable* ZEND_FASTCALL zend_new_pair(zval *val1, zval *val2);
ZEND_API uint32_t zend_array_count(HashTable *ht);
ZEND_API HashTable* ZEND_FASTCALL zend_array_dup(HashTable *source);
ZEND_API void ZEND_FASTCALL zend_array_destroy(HashTable *ht);
Expand Down
33 changes: 13 additions & 20 deletions ext/pcre/php_pcre.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -964,23 +964,17 @@ PHPAPI void php_pcre_free_match_data(pcre2_match_data *match_data)
}/*}}}*/

static void init_unmatched_null_pair() {
zval tmp;
zval *pair = &PCRE_G(unmatched_null_pair);
array_init_size(pair, 2);
ZVAL_NULL(&tmp);
zend_hash_next_index_insert_new(Z_ARRVAL_P(pair), &tmp);
ZVAL_LONG(&tmp, -1);
zend_hash_next_index_insert_new(Z_ARRVAL_P(pair), &tmp);
zval val1, val2;
ZVAL_NULL(&val1);
ZVAL_LONG(&val2, -1);
ZVAL_ARR(&PCRE_G(unmatched_null_pair), zend_new_pair(&val1, &val2));
}

static void init_unmatched_empty_pair() {
zval tmp;
zval *pair = &PCRE_G(unmatched_empty_pair);
array_init_size(pair, 2);
ZVAL_EMPTY_STRING(&tmp);
zend_hash_next_index_insert_new(Z_ARRVAL_P(pair), &tmp);
ZVAL_LONG(&tmp, -1);
zend_hash_next_index_insert_new(Z_ARRVAL_P(pair), &tmp);
zval val1, val2;
ZVAL_EMPTY_STRING(&val1);
ZVAL_LONG(&val2, -1);
ZVAL_ARR(&PCRE_G(unmatched_empty_pair), zend_new_pair(&val1, &val2));
}

static zend_always_inline void populate_match_value_str(
Expand Down Expand Up @@ -1013,7 +1007,7 @@ static inline void add_offset_pair(
zval *result, const char *subject, PCRE2_SIZE start_offset, PCRE2_SIZE end_offset,
zend_string *name, uint32_t unmatched_as_null)
{
zval match_pair, tmp;
zval match_pair;

/* Add (match, offset) to the return value */
if (PCRE2_UNSET == start_offset) {
Expand All @@ -1029,11 +1023,10 @@ static inline void add_offset_pair(
ZVAL_COPY(&match_pair, &PCRE_G(unmatched_empty_pair));
}
} else {
array_init_size(&match_pair, 2);
populate_match_value_str(&tmp, subject, start_offset, end_offset);
zend_hash_next_index_insert_new(Z_ARRVAL(match_pair), &tmp);
ZVAL_LONG(&tmp, start_offset);
zend_hash_next_index_insert_new(Z_ARRVAL(match_pair), &tmp);
zval val1, val2;
populate_match_value_str(&val1, subject, start_offset, end_offset);
ZVAL_LONG(&val2, start_offset);
ZVAL_ARR(&match_pair, zend_new_pair(&val1, &val2));
}

if (name) {
Expand Down