Skip to content

Commit 155ae36

Browse files
yatsukhnenkoyatsukhnenko
authored
Merge pull request phpredis#973 from yatsukhnenko/develop
RedisArray refactoring
2 parents 32dc527 + 2f2b3b3 commit 155ae36

File tree

3 files changed

+66
-75
lines changed

3 files changed

+66
-75
lines changed

‎redis_array.c‎

Lines changed: 34 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -89,8 +89,7 @@ static void redis_array_free(RedisArray *ra) {
8989

9090
/* Redis objects */
9191
for(i=0;i<ra->count;i++) {
92-
zval_dtor(ra->redis[i]);
93-
efree(ra->redis[i]);
92+
zval_dtor(&ra->redis[i]);
9493
efree(ra->hosts[i]);
9594
}
9695
efree(ra->redis);
@@ -488,7 +487,7 @@ PHP_METHOD(RedisArray, _target)
488487

489488
redis_inst = ra_find_node(ra, key, key_len, &i TSRMLS_CC);
490489
if(redis_inst) {
491-
ZVAL_STRING(return_value, ra->hosts[i], 1);
490+
RETURN_STRING(ra->hosts[i]);
492491
} else {
493492
RETURN_NULL();
494493
}
@@ -588,7 +587,7 @@ PHP_METHOD(RedisArray, _rehash)
588587

589588
static void multihost_distribute(INTERNAL_FUNCTION_PARAMETERS, const char *method_name)
590589
{
591-
zval *object, z_fun, *z_tmp;
590+
zval *object, z_fun, *z_tmp, *redis_inst;
592591
int i;
593592
RedisArray *ra;
594593

@@ -610,8 +609,9 @@ static void multihost_distribute(INTERNAL_FUNCTION_PARAMETERS, const char *metho
610609
MAKE_STD_ZVAL(z_tmp);
611610

612611
/* Call each node in turn */
613-
call_user_function(&redis_ce->function_table, &ra->redis[i],
614-
&z_fun, z_tmp, 0, NULL TSRMLS_CC);
612+
redis_inst = &ra->redis[i];
613+
call_user_function(&redis_ce->function_table, &redis_inst, &z_fun,
614+
z_tmp, 0, NULL TSRMLS_CC);
615615

616616
add_assoc_zval(return_value, ra->hosts[i], z_tmp);
617617
}
@@ -650,7 +650,7 @@ PHP_METHOD(RedisArray, bgsave)
650650

651651
PHP_METHOD(RedisArray, keys)
652652
{
653-
zval *object, *z_args[1], *z_tmp, z_fun;
653+
zval *object, *z_args[1], *z_tmp, z_fun, *redis_inst;
654654
RedisArray *ra;
655655
char *pattern;
656656
int pattern_len, i;
@@ -683,7 +683,9 @@ PHP_METHOD(RedisArray, keys)
683683
MAKE_STD_ZVAL(z_tmp);
684684

685685
/* Call KEYS on each node */
686-
call_user_function(&redis_ce->function_table, &ra->redis[i], &z_fun, z_tmp, 1, z_args TSRMLS_CC);
686+
redis_inst = &ra->redis[i];
687+
call_user_function(&redis_ce->function_table, &redis_inst, &z_fun,
688+
z_tmp, 1, z_args TSRMLS_CC);
687689

688690
/* Add the result for this host */
689691
add_assoc_zval(return_value, ra->hosts[i], z_tmp);
@@ -695,7 +697,7 @@ PHP_METHOD(RedisArray, keys)
695697

696698
PHP_METHOD(RedisArray, getOption)
697699
{
698-
zval *object, z_fun, *z_tmp, *z_args[1];
700+
zval *object, z_fun, *z_tmp, *z_args[1], *redis_inst;
699701
int i;
700702
RedisArray *ra;
701703
long opt;
@@ -722,8 +724,9 @@ PHP_METHOD(RedisArray, getOption)
722724
MAKE_STD_ZVAL(z_tmp);
723725

724726
/* Call each node in turn */
725-
call_user_function(&redis_ce->function_table, &ra->redis[i],
726-
&z_fun, z_tmp, 1, z_args TSRMLS_CC);
727+
redis_inst = &ra->redis[i];
728+
call_user_function(&redis_ce->function_table, &redis_inst, &z_fun,
729+
z_tmp, 1, z_args TSRMLS_CC);
727730

728731
add_assoc_zval(return_value, ra->hosts[i], z_tmp);
729732
}
@@ -734,7 +737,7 @@ PHP_METHOD(RedisArray, getOption)
734737

735738
PHP_METHOD(RedisArray, setOption)
736739
{
737-
zval *object, z_fun, *z_tmp, *z_args[2];
740+
zval *object, z_fun, *z_tmp, *z_args[2], *redis_inst;
738741
int i;
739742
RedisArray *ra;
740743
long opt;
@@ -765,8 +768,9 @@ PHP_METHOD(RedisArray, setOption)
765768
MAKE_STD_ZVAL(z_tmp);
766769

767770
/* Call each node in turn */
768-
call_user_function(&redis_ce->function_table, &ra->redis[i],
769-
&z_fun, z_tmp, 2, z_args TSRMLS_CC);
771+
redis_inst = &ra->redis[i];
772+
call_user_function(&redis_ce->function_table, &redis_inst, &z_fun,
773+
z_tmp, 2, z_args TSRMLS_CC);
770774

771775
add_assoc_zval(return_value, ra->hosts[i], z_tmp);
772776
}
@@ -778,7 +782,7 @@ PHP_METHOD(RedisArray, setOption)
778782

779783
PHP_METHOD(RedisArray, select)
780784
{
781-
zval *object, z_fun, *z_tmp, *z_args[2];
785+
zval *object, z_fun, *z_tmp, *z_args[2], *redis_inst;
782786
int i;
783787
RedisArray *ra;
784788
long opt;
@@ -801,12 +805,12 @@ PHP_METHOD(RedisArray, select)
801805

802806
array_init(return_value);
803807
for(i = 0; i < ra->count; ++i) {
804-
805808
MAKE_STD_ZVAL(z_tmp);
806809

807810
/* Call each node in turn */
808-
call_user_function(&redis_ce->function_table, &ra->redis[i],
809-
&z_fun, z_tmp, 1, z_args TSRMLS_CC);
811+
redis_inst = &ra->redis[i];
812+
call_user_function(&redis_ce->function_table, &redis_inst, &z_fun,
813+
z_tmp, 1, z_args TSRMLS_CC);
810814

811815
add_assoc_zval(return_value, ra->hosts[i], z_tmp);
812816
}
@@ -852,7 +856,7 @@ PHP_METHOD(RedisArray, mget)
852856
RedisArray *ra;
853857
int *pos, argc, *argc_each;
854858
HashTable *h_keys;
855-
zval **argv;
859+
zval **argv, *redis_inst;
856860

857861
/* Multi/exec support */
858862
HANDLE_MULTI_EXEC("MGET");
@@ -938,8 +942,9 @@ PHP_METHOD(RedisArray, mget)
938942

939943
/* call MGET on the node */
940944
MAKE_STD_ZVAL(z_ret);
941-
call_user_function(&redis_ce->function_table, &ra->redis[n],
942-
&z_fun, z_ret, 1, &z_argarray TSRMLS_CC);
945+
redis_inst = &ra->redis[n];
946+
call_user_function(&redis_ce->function_table, &redis_inst, &z_fun,
947+
z_ret, 1, &z_argarray TSRMLS_CC);
943948

944949
/* cleanup args array */
945950
zval_ptr_dtor(&z_argarray);
@@ -1055,10 +1060,6 @@ PHP_METHOD(RedisArray, mset)
10551060
for(n = 0; n < ra->count; ++n) { /* for each node */
10561061
int found = 0;
10571062

1058-
/* prepare call */
1059-
ZVAL_STRING(&z_fun, "MSET", 0);
1060-
redis_inst = ra->redis[n];
1061-
10621063
/* copy args */
10631064
MAKE_STD_ZVAL(z_argarray);
10641065
array_init(z_argarray);
@@ -1083,13 +1084,16 @@ PHP_METHOD(RedisArray, mset)
10831084
continue; /* don't run empty MSETs */
10841085
}
10851086

1087+
/* prepare call */
1088+
ZVAL_STRING(&z_fun, "MSET", 0);
1089+
redis_inst = &ra->redis[n];
10861090
if(ra->index) { /* add MULTI */
10871091
ra_index_multi(redis_inst, MULTI TSRMLS_CC);
10881092
}
10891093

10901094
/* call */
1091-
call_user_function(&redis_ce->function_table, &ra->redis[n],
1092-
&z_fun, &z_ret, 1, &z_argarray TSRMLS_CC);
1095+
call_user_function(&redis_ce->function_table, &redis_inst, &z_fun,
1096+
&z_ret, 1, &z_argarray TSRMLS_CC);
10931097

10941098
if(ra->index) {
10951099
ra_index_keys(z_argarray, redis_inst TSRMLS_CC); /* use SADD to add keys to node index */
@@ -1194,7 +1198,6 @@ PHP_METHOD(RedisArray, del)
11941198
for(n = 0; n < ra->count; ++n) { /* for each node */
11951199

11961200
int found = 0;
1197-
redis_inst = ra->redis[n];
11981201

11991202
/* copy args */
12001203
MAKE_STD_ZVAL(z_argarray);
@@ -1217,14 +1220,15 @@ PHP_METHOD(RedisArray, del)
12171220
continue;
12181221
}
12191222

1223+
redis_inst = &ra->redis[n];
12201224
if(ra->index) { /* add MULTI */
12211225
ra_index_multi(redis_inst, MULTI TSRMLS_CC);
12221226
}
12231227

12241228
/* call */
12251229
MAKE_STD_ZVAL(z_ret);
1226-
call_user_function(&redis_ce->function_table, &redis_inst,
1227-
&z_fun, z_ret, 1, &z_argarray TSRMLS_CC);
1230+
call_user_function(&redis_ce->function_table, &redis_inst, &z_fun,
1231+
z_ret, 1, &z_argarray TSRMLS_CC);
12281232

12291233
if(ra->index) {
12301234
ra_index_del(z_argarray, redis_inst TSRMLS_CC); /* use SREM to remove keys from node index */

‎redis_array.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ typedef struct RedisArray_ {
4343

4444
int count;
4545
char **hosts; /* array of host:port strings */
46-
zval **redis; /* array of Redis instances */
46+
zval *redis; /* array of Redis instances */
4747
zval *z_multi_exec; /* Redis instance to be used in multi-exec */
4848
zend_bool index; /* use per-node index */
4949
zend_bool auto_rehash; /* migrate keys on read operations */

‎redis_array_impl.c‎

Lines changed: 31 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ ra_load_hosts(RedisArray *ra, HashTable *hosts, long retry_interval, zend_bool b
3535
int i = 0, host_len, id;
3636
char *host, *p;
3737
short port;
38-
zval *zpData, z_cons, z_ret;
38+
zval *zpData, z_cons, z_ret, *redis_inst;
3939
RedisSock *redis_sock = NULL;
4040

4141
/* function calls on the Redis object */
@@ -47,8 +47,7 @@ ra_load_hosts(RedisArray *ra, HashTable *hosts, long retry_interval, zend_bool b
4747
if ((zpData = zend_hash_get_current_data(hosts)) == NULL || Z_TYPE_P(zpData) != IS_STRING)
4848
{
4949
for(i=0;i<ra->count;i++) {
50-
zval_dtor(ra->redis[i]);
51-
efree(ra->redis[i]);
50+
zval_dtor(&ra->redis[i]);
5251
efree(ra->hosts[i]);
5352
}
5453
efree(ra->redis);
@@ -74,10 +73,10 @@ ra_load_hosts(RedisArray *ra, HashTable *hosts, long retry_interval, zend_bool b
7473
}
7574

7675
/* create Redis object */
77-
MAKE_STD_ZVAL(ra->redis[i]);
78-
object_init_ex(ra->redis[i], redis_ce);
79-
INIT_PZVAL(ra->redis[i]);
80-
call_user_function(&redis_ce->function_table, &ra->redis[i], &z_cons, &z_ret, 0, NULL TSRMLS_CC);
76+
redis_inst = &ra->redis[i];
77+
object_init_ex(redis_inst, redis_ce);
78+
INIT_PZVAL(redis_inst);
79+
call_user_function(&redis_ce->function_table, &redis_inst, &z_cons, &z_ret, 0, NULL TSRMLS_CC);
8180

8281
/* create socket */
8382
redis_sock = redis_sock_create(host, host_len, port, ra->connect_timeout, ra->pconnect, NULL, retry_interval, b_lazy_connect);
@@ -95,9 +94,9 @@ ra_load_hosts(RedisArray *ra, HashTable *hosts, long retry_interval, zend_bool b
9594
id = zend_list_insert(redis_sock, le_redis_sock);
9695
#endif
9796
#if (PHP_MAJOR_VERSION < 7)
98-
add_property_resource(ra->redis[i], "socket", id);
97+
add_property_resource(&ra->redis[i], "socket", id);
9998
#else
100-
add_property_resource(ra->redis[i], "socket", Z_RES_P(id));
99+
add_property_resource(&ra->redis[i], "socket", Z_RES_P(id));
101100
#endif
102101

103102
ra->count = ++i;
@@ -363,8 +362,8 @@ ra_make_array(HashTable *hosts, zval *z_fun, zval *z_dist, HashTable *hosts_prev
363362

364363
/* create object */
365364
RedisArray *ra = emalloc(sizeof(RedisArray));
366-
ra->hosts = emalloc(count * sizeof(char*));
367-
ra->redis = emalloc(count * sizeof(zval*));
365+
ra->hosts = ecalloc(count, sizeof(char *));
366+
ra->redis = ecalloc(count, sizeof(zval));
368367
ra->count = 0;
369368
ra->z_fun = NULL;
370369
ra->z_dist = NULL;
@@ -500,7 +499,7 @@ ra_find_node(RedisArray *ra, const char *key, int key_len, int *out_pos TSRMLS_D
500499
}
501500
if(out_pos) *out_pos = pos;
502501

503-
return ra->redis[pos];
502+
return &ra->redis[pos];
504503
}
505504

506505
zval *
@@ -509,7 +508,7 @@ ra_find_node_by_name(RedisArray *ra, const char *host, int host_len TSRMLS_DC) {
509508
int i;
510509
for(i = 0; i < ra->count; ++i) {
511510
if(strncmp(ra->hosts[i], host, host_len) == 0) {
512-
return ra->redis[i];
511+
return &ra->redis[i];
513512
}
514513
}
515514
return NULL;
@@ -584,46 +583,34 @@ ra_index_del(zval *z_keys, zval *z_redis TSRMLS_DC) {
584583
void
585584
ra_index_keys(zval *z_pairs, zval *z_redis TSRMLS_DC) {
586585

586+
zval z_keys, *z_val;
587+
zend_string *zkey;
588+
ulong idx;
587589
/* Initialize key array */
588-
zval *z_keys;
589-
MAKE_STD_ZVAL(z_keys);
590-
HashPosition pos;
591590
#if PHP_VERSION_ID > 50300
592-
array_init_size(z_keys, zend_hash_num_elements(Z_ARRVAL_P(z_pairs)));
591+
array_init_size(&z_keys, zend_hash_num_elements(Z_ARRVAL_P(z_pairs)));
593592
#else
594-
array_init(z_keys);
593+
array_init(&z_keys);
595594
#endif
596595

597596
/* Go through input array and add values to the key array */
598-
zend_hash_internal_pointer_reset_ex(Z_ARRVAL_P(z_pairs), &pos);
599-
while (zend_hash_get_current_data_ex(Z_ARRVAL_P(z_pairs), &pos) != NULL) {
600-
char *key;
601-
unsigned int key_len;
602-
unsigned long num_key;
603-
zval *z_new;
604-
MAKE_STD_ZVAL(z_new);
605-
606-
switch (zend_hash_get_current_key_ex(Z_ARRVAL_P(z_pairs), &key, &key_len, &num_key, 1, &pos)) {
607-
case HASH_KEY_IS_STRING:
608-
ZVAL_STRINGL(z_new, key, (int)key_len - 1, 0);
609-
zend_hash_next_index_insert(Z_ARRVAL_P(z_keys), z_new);
610-
break;
611-
612-
case HASH_KEY_IS_LONG:
613-
Z_TYPE_P(z_new) = IS_LONG;
614-
Z_LVAL_P(z_new) = (long)num_key;
615-
zend_hash_next_index_insert(Z_ARRVAL_P(z_keys), z_new);
616-
break;
617-
}
618-
zend_hash_move_forward_ex(Z_ARRVAL_P(z_pairs), &pos);
619-
}
597+
ZEND_HASH_FOREACH_KEY_VAL(Z_ARRVAL_P(z_pairs), idx, zkey, z_val) {
598+
zval *z_new;
599+
MAKE_STD_ZVAL(z_new);
600+
601+
if (zkey) {
602+
ZVAL_STRINGL(z_new, zkey->val, zkey->len, 1);
603+
} else {
604+
ZVAL_LONG(z_new, idx);
605+
}
606+
zend_hash_next_index_insert(Z_ARRVAL(z_keys), z_new);
607+
} ZEND_HASH_FOREACH_END();
620608

621609
/* add keys to index */
622-
ra_index_change_keys("SADD", z_keys, z_redis TSRMLS_CC);
610+
ra_index_change_keys("SADD", &z_keys, z_redis TSRMLS_CC);
623611

624612
/* cleanup */
625-
zval_dtor(z_keys);
626-
efree(z_keys);
613+
zval_dtor(&z_keys);
627614
}
628615

629616
void
@@ -1256,7 +1243,7 @@ ra_rehash(RedisArray *ra, zend_fcall_info *z_cb, zend_fcall_info_cache *z_cb_cac
12561243
return; /* TODO: compare the two rings for equality */
12571244

12581245
for(i = 0; i < ra->prev->count; ++i) {
1259-
ra_rehash_server(ra, ra->prev->redis[i], ra->prev->hosts[i], ra->index, z_cb, z_cb_cache TSRMLS_CC);
1246+
ra_rehash_server(ra, &ra->prev->redis[i], ra->prev->hosts[i], ra->index, z_cb, z_cb_cache TSRMLS_CC);
12601247
}
12611248
}
12621249

0 commit comments

Comments
 (0)