Skip to content

Commit 6ca64a1

Browse files
yatsukhnenkoyatsukhnenko
committed
Refactor RedisArray
1 parent 2634350 commit 6ca64a1

File tree

1 file changed

+60
-75
lines changed

1 file changed

+60
-75
lines changed

‎redis_array.c

Lines changed: 60 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -495,7 +495,7 @@ PHP_METHOD(RedisArray, _instance)
495495

496496
PHP_METHOD(RedisArray, _function)
497497
{
498-
zval *object, *z_fun;
498+
zval *object;
499499
RedisArray *ra;
500500

501501
if (zend_parse_method_parameters(ZEND_NUM_ARGS(), getThis(), "O",
@@ -507,13 +507,12 @@ PHP_METHOD(RedisArray, _function)
507507
RETURN_FALSE;
508508
}
509509

510-
z_fun = &ra->z_fun;
511-
RETURN_ZVAL(z_fun, 1, 0);
510+
RETURN_ZVAL(&ra->z_fun, 1, 0);
512511
}
513512

514513
PHP_METHOD(RedisArray, _distributor)
515514
{
516-
zval *object, *z_dist;
515+
zval *object;
517516
RedisArray *ra;
518517

519518
if (zend_parse_method_parameters(ZEND_NUM_ARGS(), getThis(), "O",
@@ -525,8 +524,7 @@ PHP_METHOD(RedisArray, _distributor)
525524
RETURN_FALSE;
526525
}
527526

528-
z_dist = &ra->z_dist;
529-
RETURN_ZVAL(z_dist, 1, 0);
527+
RETURN_ZVAL(&ra->z_dist, 1, 0);
530528
}
531529

532530
PHP_METHOD(RedisArray, _rehash)
@@ -815,12 +813,10 @@ PHP_METHOD(RedisArray, select)
815813
/* MGET will distribute the call to several nodes and regroup the values. */
816814
PHP_METHOD(RedisArray, mget)
817815
{
818-
zval *object, *z_keys, z_argarray, *data, z_ret, *z_cur, z_tmp_array;
819-
int i, j, n;
820-
RedisArray *ra;
821-
int *pos, argc, *argc_each;
816+
zval *object, *z_keys, *data, z_ret, *z_cur, z_tmp_array, z_fun, z_arg, **argv;
817+
int i, j, n, *pos, argc, *argc_each;
822818
HashTable *h_keys;
823-
zval **argv;
819+
RedisArray *ra;
824820

825821
if ((ra = redis_array_get(getThis())) == NULL) {
826822
RETURN_FALSE;
@@ -840,11 +836,10 @@ PHP_METHOD(RedisArray, mget)
840836
if ((argc = zend_hash_num_elements(h_keys)) == 0) {
841837
RETURN_FALSE;
842838
}
843-
argv = emalloc(argc * sizeof(zval*));
844-
pos = emalloc(argc * sizeof(int));
839+
argv = ecalloc(argc, sizeof(*argv));
840+
pos = ecalloc(argc, sizeof(*pos));
845841

846-
argc_each = emalloc(ra->count * sizeof(int));
847-
memset(argc_each, 0, ra->count * sizeof(int));
842+
argc_each = ecalloc(ra->count, sizeof(*argc_each));
848843

849844
/* associate each key to a redis node */
850845
i = 0;
@@ -856,95 +851,86 @@ PHP_METHOD(RedisArray, mget)
856851
/* Handle the possibility that we're a reference */
857852
ZVAL_DEREF(data);
858853

859-
/* phpredis proper can only use string or long keys, so restrict to that here */
860-
if (Z_TYPE_P(data) != IS_STRING && Z_TYPE_P(data) != IS_LONG) {
861-
php_error_docref(NULL, E_ERROR, "MGET: all keys must be strings or longs");
862-
efree(argv);
863-
efree(pos);
864-
efree(argc_each);
865-
RETURN_FALSE;
866-
}
867-
868854
/* Convert to a string for hash lookup if it isn't one */
869855
if (Z_TYPE_P(data) == IS_STRING) {
870856
key_len = Z_STRLEN_P(data);
871857
key_lookup = Z_STRVAL_P(data);
872-
} else {
858+
} else if (Z_TYPE_P(data) == IS_LONG) {
873859
key_len = snprintf(kbuf, sizeof(kbuf), ZEND_LONG_FMT, Z_LVAL_P(data));
874860
key_lookup = (char*)kbuf;
861+
} else {
862+
/* phpredis proper can only use string or long keys, so restrict to that here */
863+
php_error_docref(NULL, E_ERROR, "MGET: all keys must be strings or longs");
864+
RETVAL_FALSE;
865+
goto cleanup;
875866
}
876867

877868
/* Find our node */
878869
if (ra_find_node(ra, key_lookup, key_len, &pos[i]) == NULL) {
879-
/* TODO: handle */
870+
RETVAL_FALSE;
871+
goto cleanup;
880872
}
881873

882874
argc_each[pos[i]]++; /* count number of keys per node */
883875
argv[i++] = data;
884876
} ZEND_HASH_FOREACH_END();
885877

878+
/* prepare call */
886879
array_init(&z_tmp_array);
880+
ZVAL_STRINGL(&z_fun, "MGET", sizeof("MGET") - 1);
881+
887882
/* calls */
888883
for(n = 0; n < ra->count; ++n) { /* for each node */
889884
/* We don't even need to make a call to this node if no keys go there */
890885
if(!argc_each[n]) continue;
891886

892887
/* copy args for MGET call on node. */
893-
array_init(&z_argarray);
888+
array_init(&z_arg);
894889

895890
for(i = 0; i < argc; ++i) {
896-
if(pos[i] != n) continue;
897-
898-
zval z_ret;
899-
ZVAL_ZVAL(&z_ret, argv[i], 1, 0);
900-
add_next_index_zval(&z_argarray, &z_ret);
891+
if (pos[i] == n) {
892+
add_next_index_zval(&z_arg, argv[i]);
893+
}
901894
}
902895

903-
zval z_fun;
904-
/* prepare call */
905-
ZVAL_STRINGL(&z_fun, "MGET", 4);
906896
/* call MGET on the node */
907-
call_user_function(&redis_ce->function_table, &ra->redis[n], &z_fun, &z_ret, 1, &z_argarray);
908-
zval_dtor(&z_fun);
897+
call_user_function(&redis_ce->function_table, &ra->redis[n], &z_fun, &z_ret, 1, &z_arg);
909898

910899
/* cleanup args array */
911-
zval_dtor(&z_argarray);
900+
zval_dtor(&z_arg);
912901

913902
/* Error out if we didn't get a proper response */
914903
if (Z_TYPE(z_ret) != IS_ARRAY) {
915904
/* cleanup */
916905
zval_dtor(&z_ret);
917906
zval_dtor(&z_tmp_array);
918-
efree(argv);
919-
efree(pos);
920-
efree(argc_each);
921-
922-
/* failure */
923-
RETURN_FALSE;
907+
RETVAL_FALSE;
908+
goto cleanup;
924909
}
925910

926911
for(i = 0, j = 0; i < argc; ++i) {
927912
if (pos[i] != n || (z_cur = zend_hash_index_find(Z_ARRVAL(z_ret), j++)) == NULL) continue;
928913

929-
zval z_ret;
930-
ZVAL_ZVAL(&z_ret, z_cur, 1, 0);
931-
add_index_zval(&z_tmp_array, i, &z_ret);
914+
ZVAL_ZVAL(&z_arg, z_cur, 1, 0);
915+
add_index_zval(&z_tmp_array, i, &z_arg);
932916
}
933917
zval_dtor(&z_ret);
934918
}
935919

920+
zval_dtor(&z_fun);
921+
936922
array_init(return_value);
937923
/* copy temp array in the right order to return_value */
938924
for(i = 0; i < argc; ++i) {
939925
if ((z_cur = zend_hash_index_find(Z_ARRVAL(z_tmp_array), i)) == NULL) continue;
940926

941-
zval z_ret;
942-
ZVAL_ZVAL(&z_ret, z_cur, 1, 0);
943-
add_next_index_zval(return_value, &z_ret);
927+
ZVAL_ZVAL(&z_arg, z_cur, 1, 0);
928+
add_next_index_zval(return_value, &z_arg);
944929
}
945930

946931
/* cleanup */
947932
zval_dtor(&z_tmp_array);
933+
cleanup:
948934
efree(argv);
949935
efree(pos);
950936
efree(argc_each);
@@ -954,13 +940,11 @@ PHP_METHOD(RedisArray, mget)
954940
/* MSET will distribute the call to several nodes and regroup the values. */
955941
PHP_METHOD(RedisArray, mset)
956942
{
957-
zval *object, *z_keys, z_argarray, *data, z_ret, **argv;
958-
int i = 0, n;
943+
zval *object, *z_keys, z_argarray, *data, z_fun, z_ret, **argv;
944+
int i = 0, n, *pos, argc, *argc_each, key_len;
959945
RedisArray *ra;
960-
int *pos, argc, *argc_each;
961946
HashTable *h_keys;
962947
char *key, kbuf[40];
963-
int key_len;
964948
zend_string **keys, *zkey;
965949
zend_ulong idx;
966950

@@ -982,12 +966,11 @@ PHP_METHOD(RedisArray, mset)
982966
if ((argc = zend_hash_num_elements(h_keys)) == 0) {
983967
RETURN_FALSE;
984968
}
985-
argv = emalloc(argc * sizeof(zval*));
986-
pos = emalloc(argc * sizeof(int));
987-
keys = ecalloc(argc, sizeof(zend_string *));
969+
argv = ecalloc(argc, sizeof(*argv));
970+
pos = ecalloc(argc, sizeof(*pos));
971+
keys = ecalloc(argc, sizeof(*keys));
988972

989-
argc_each = emalloc(ra->count * sizeof(int));
990-
memset(argc_each, 0, ra->count * sizeof(int));
973+
argc_each = ecalloc(ra->count, sizeof(*argc_each));
991974

992975
/* associate each key to a redis node */
993976
ZEND_HASH_FOREACH_KEY_VAL(h_keys, idx, zkey, data) {
@@ -1001,16 +984,26 @@ PHP_METHOD(RedisArray, mset)
1001984
}
1002985

1003986
if (ra_find_node(ra, key, (int)key_len, &pos[i]) == NULL) {
1004-
// TODO: handle
987+
for (n = 0; n < i; ++n) {
988+
zend_string_release(keys[n]);
989+
}
990+
efree(keys);
991+
efree(argv);
992+
efree(pos);
993+
efree(argc_each);
994+
RETURN_FALSE;
1005995
}
1006996

1007997
argc_each[pos[i]]++; /* count number of keys per node */
1008-
keys[i] = zend_string_init(key, key_len, 0);
998+
keys[i] = zkey ? zend_string_copy(zkey) : zend_string_init(key, key_len, 0);
1009999
argv[i] = data;
10101000
i++;
10111001
} ZEND_HASH_FOREACH_END();
10121002

10131003

1004+
/* prepare call */
1005+
ZVAL_STRINGL(&z_fun, "MSET", sizeof("MSET") - 1);
1006+
10141007
/* calls */
10151008
for (n = 0; n < ra->count; ++n) { /* for each node */
10161009
/* We don't even need to make a call to this node if no keys go there */
@@ -1023,7 +1016,6 @@ PHP_METHOD(RedisArray, mset)
10231016
for(i = 0; i < argc; ++i) {
10241017
if(pos[i] != n) continue;
10251018

1026-
zval z_ret;
10271019
if (argv[i] == NULL) {
10281020
ZVAL_NULL(&z_ret);
10291021
} else {
@@ -1040,26 +1032,19 @@ PHP_METHOD(RedisArray, mset)
10401032

10411033
if(ra->index) { /* add MULTI */
10421034
ra_index_multi(&ra->redis[n], MULTI);
1043-
}
1044-
1045-
zval z_fun;
1046-
1047-
/* prepare call */
1048-
ZVAL_STRINGL(&z_fun, "MSET", 4);
1049-
1050-
/* call */
1051-
call_user_function(&redis_ce->function_table, &ra->redis[n], &z_fun, &z_ret, 1, &z_argarray);
1052-
zval_dtor(&z_fun);
1053-
zval_dtor(&z_ret);
1054-
1055-
if(ra->index) {
1035+
call_user_function(&redis_ce->function_table, &ra->redis[n], &z_fun, &z_ret, 1, &z_argarray);
10561036
ra_index_keys(&z_argarray, &ra->redis[n]); /* use SADD to add keys to node index */
10571037
ra_index_exec(&ra->redis[n], NULL, 0); /* run EXEC */
1038+
} else {
1039+
call_user_function(&redis_ce->function_table, &ra->redis[n], &z_fun, &z_ret, 1, &z_argarray);
10581040
}
10591041

10601042
zval_dtor(&z_argarray);
1043+
zval_dtor(&z_ret);
10611044
}
10621045

1046+
zval_dtor(&z_fun);
1047+
10631048
/* Free any keys that we needed to allocate memory for, because they weren't strings */
10641049
for(i = 0; i < argc; i++) {
10651050
zend_string_release(keys[i]);

0 commit comments

Comments
 (0)