Skip to content

Commit 3923531

Browse files
michael-grundermichael-grunder
committed
Merge branch 'feature/z_cmd_performance' into develop
2 parents eb0bbba + 5c4fe08 commit 3923531

File tree

3 files changed

+177
-156
lines changed

3 files changed

+177
-156
lines changed

‎library.c

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -467,6 +467,7 @@ int redis_cmd_init_sstr(smart_str *str, int num_args, char *keyword, int keyword
467467
smart_str_appendl(str, _NL, sizeof(_NL) - 1);
468468
smart_str_appendl(str, keyword, keyword_len);
469469
smart_str_appendl(str, _NL, sizeof(_NL) - 1);
470+
return str->len;
470471
}
471472

472473
/*
@@ -483,6 +484,44 @@ int redis_cmd_append_sstr(smart_str *str, char *append, int append_len) {
483484
return str->len;
484485
}
485486

487+
/*
488+
* Append an integer to a smart string command
489+
*/
490+
int redis_cmd_append_sstr_int(smart_str *str, int append) {
491+
char int_buf[32];
492+
int int_len = snprintf(int_buf, sizeof(int_buf), "%d", append);
493+
return redis_cmd_append_sstr(str, int_buf, int_len);
494+
}
495+
496+
/*
497+
* Append a long to a smart string command
498+
*/
499+
int redis_cmd_append_sstr_long(smart_str *str, long append) {
500+
char long_buf[32];
501+
int long_len = snprintf(long_buf, sizeof(long_buf), "%ld", append);
502+
return redis_cmd_append_sstr(str, long_buf, long_len);
503+
}
504+
505+
/*
506+
* Append a double to a smart string command
507+
*/
508+
int redis_cmd_append_sstr_dbl(smart_str *str, double value) {
509+
char *dbl_str;
510+
int dbl_len;
511+
512+
/// Convert to double
513+
REDIS_DOUBLE_TO_STRING(dbl_str, dbl_len, value);
514+
515+
// Append the string
516+
int retval = redis_cmd_append_sstr(str, dbl_str, dbl_len);
517+
518+
// Free our double string
519+
efree(dbl_str);
520+
521+
// Return new length
522+
return retval;
523+
}
524+
486525
/*
487526
* Append an integer command to a Redis command
488527
*/

‎library.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,10 @@ int redis_cmd_format_header(char **ret, char *keyword, int arg_count);
66
int redis_cmd_append_str(char **cmd, int cmd_len, char *append, int append_len);
77
int redis_cmd_init_sstr(smart_str *str, int num_args, char *keyword, int keyword_len);
88
int redis_cmd_append_sstr(smart_str *str, char *append, int append_len);
9+
int redis_cmd_append_sstr_int(smart_str *str, int append);
10+
int redis_cmd_append_sstr_long(smart_str *str, long append);
911
int redis_cmd_append_int(char **cmd, int cmd_len, int append);
10-
12+
int redis_cmd_append_sstr_dbl(smart_str *str, double value);
1113

1214
PHPAPI char * redis_sock_read(RedisSock *redis_sock, int *buf_len TSRMLS_DC);
1315

‎redis.c

Lines changed: 135 additions & 155 deletions
Original file line numberDiff line numberDiff line change
@@ -4355,187 +4355,167 @@ PHP_METHOD(Redis, zIncrBy)
43554355
generic_incrby_method(INTERNAL_FUNCTION_PARAM_PASSTHRU, "ZINCRBY", sizeof("ZINCRBY")-1);
43564356
}
43574357
/* }}} */
4358-
PHPAPI void generic_z_command(INTERNAL_FUNCTION_PARAMETERS, char *command, int command_len) {
4359-
4360-
zval *object, *keys_array, *weights_array = NULL, **data;
4361-
HashTable *arr_weights_hash = NULL, *arr_keys_hash;
4362-
int key_output_len, array_weights_count, array_keys_count, operation_len = 0;
4363-
char *key_output, *operation;
4364-
RedisSock *redis_sock;
43654358

4366-
HashPosition pointer;
4367-
char *cmd = "";
4368-
char *old_cmd;
4369-
int cmd_len, cmd_elements;
4370-
int free_key_output;
4359+
PHPAPI void generic_z_command(INTERNAL_FUNCTION_PARAMETERS, char *command, int command_len) {
4360+
zval *object, *z_keys, *z_weights = NULL, **z_data;
4361+
HashTable *ht_keys, *ht_weights = NULL;
4362+
RedisSock *redis_sock;
4363+
smart_str cmd = {0};
4364+
HashPosition ptr;
4365+
char *store_key, *agg_op = NULL;
4366+
int cmd_arg_count = 2, store_key_len, agg_op_len, keys_count;
43714367

4372-
if(zend_parse_method_parameters(ZEND_NUM_ARGS() TSRMLS_CC, getThis(), "Osa|a!s",
4373-
&object, redis_ce,
4374-
&key_output, &key_output_len, &keys_array, &weights_array, &operation, &operation_len) == FAILURE) {
4375-
RETURN_FALSE;
4376-
}
4368+
// Grab our parameters
4369+
if(zend_parse_method_parameters(ZEND_NUM_ARGS() TSRMLS_CC, getThis(), "Osa|a!s",
4370+
&object, redis_ce, &store_key, &store_key_len,
4371+
&z_keys, &z_weights, &agg_op, &agg_op_len) == FAILURE)
4372+
{
4373+
RETURN_FALSE;
4374+
}
43774375

4376+
// We'll need our socket
43784377
if(redis_sock_get(object, &redis_sock TSRMLS_CC, 0) < 0) {
4379-
RETURN_FALSE;
4378+
RETURN_FALSE;
43804379
}
43814380

4382-
arr_keys_hash = Z_ARRVAL_P(keys_array);
4383-
array_keys_count = zend_hash_num_elements(arr_keys_hash);
4381+
// Grab our keys argument as an array
4382+
ht_keys = Z_ARRVAL_P(z_keys);
43844383

4385-
if (array_keys_count == 0) {
4384+
// Nothing to do if there aren't any keys
4385+
if((keys_count = zend_hash_num_elements(ht_keys)) == 0) {
43864386
RETURN_FALSE;
4387+
} else {
4388+
// Increment our overall argument count
4389+
cmd_arg_count += keys_count;
43874390
}
43884391

4389-
if(weights_array != NULL) {
4390-
arr_weights_hash = Z_ARRVAL_P(weights_array);
4391-
array_weights_count = zend_hash_num_elements(arr_weights_hash);
4392-
if (array_weights_count == 0) {
4393-
RETURN_FALSE;
4394-
}
4395-
if((array_weights_count != 0) && (array_weights_count != array_keys_count)) {
4396-
RETURN_FALSE;
4397-
}
4392+
// Grab and validate our weights array
4393+
if(z_weights != NULL) {
4394+
ht_weights = Z_ARRVAL_P(z_weights);
43984395

4399-
}
4396+
// This command is invalid if the weights array isn't the same size
4397+
// as our keys array.
4398+
if(zend_hash_num_elements(ht_weights) != keys_count) {
4399+
RETURN_FALSE;
4400+
}
44004401

4401-
free_key_output = redis_key_prefix(redis_sock, &key_output, &key_output_len TSRMLS_CC);
4402-
cmd_elements = 3;
4403-
cmd_len = redis_cmd_format(&cmd,
4404-
"$%d" _NL /* command_len */
4405-
"%s" _NL /* command */
4402+
// Increment our overall argument count by the number of keys
4403+
// plus one, for the "WEIGHTS" argument itself
4404+
cmd_arg_count += keys_count + 1;
4405+
}
44064406

4407-
"$%d" _NL /* key_output_len */
4408-
"%s" _NL /* key_output */
4407+
// AGGREGATE option
4408+
if(agg_op_len != 0) {
4409+
// Verify our aggregation option
4410+
if(strncasecmp(agg_op, "SUM", sizeof("SUM")) &&
4411+
strncasecmp(agg_op, "MIN", sizeof("MIN")) &&
4412+
strncasecmp(agg_op, "MAX", sizeof("MAX")))
4413+
{
4414+
RETURN_FALSE;
4415+
}
44094416

4410-
"$%d" _NL
4411-
"%d" _NL /* array_keys_count */
4417+
// Two more arguments: "AGGREGATE" and agg_op
4418+
cmd_arg_count += 2;
4419+
}
44124420

4413-
, command_len, command, command_len
4414-
, key_output_len, key_output, key_output_len
4415-
, integer_length(array_keys_count), array_keys_count);
4416-
if(free_key_output) efree(key_output);
4421+
// Command header
4422+
redis_cmd_init_sstr(&cmd, cmd_arg_count, command, command_len);
44174423

4418-
/* keys */
4419-
for (zend_hash_internal_pointer_reset_ex(arr_keys_hash, &pointer);
4420-
zend_hash_get_current_data_ex(arr_keys_hash, (void**) &data,
4421-
&pointer) == SUCCESS;
4422-
zend_hash_move_forward_ex(arr_keys_hash, &pointer)) {
4424+
// Prefix our key if necessary and add the output key
4425+
int key_free = redis_key_prefix(redis_sock, &store_key, &store_key_len TSRMLS_CC);
4426+
redis_cmd_append_sstr(&cmd, store_key, store_key_len);
4427+
if(key_free) efree(store_key);
44234428

4424-
if (Z_TYPE_PP(data) == IS_STRING) {
4425-
char *old_cmd = NULL;
4426-
char *data_str;
4427-
int data_len;
4428-
int free_data;
4429-
4430-
if(*cmd) {
4431-
old_cmd = cmd;
4432-
}
4433-
data_str = Z_STRVAL_PP(data);
4434-
data_len = Z_STRLEN_PP(data);
4435-
4436-
free_data = redis_key_prefix(redis_sock, &data_str, &data_len TSRMLS_CC);
4437-
cmd_len = redis_cmd_format(&cmd,
4438-
"%s" /* cmd */
4439-
"$%d" _NL
4440-
"%s" _NL
4441-
, cmd, cmd_len
4442-
, data_len, data_str, data_len);
4443-
cmd_elements++;
4444-
if(free_data) efree(data_str);
4445-
if(old_cmd) {
4446-
efree(old_cmd);
4447-
}
4429+
// Number of input keys argument
4430+
redis_cmd_append_sstr_int(&cmd, keys_count);
4431+
4432+
// Process input keys
4433+
for(zend_hash_internal_pointer_reset_ex(ht_keys, &ptr);
4434+
zend_hash_get_current_data_ex(ht_keys, (void**)&z_data, &ptr)==SUCCESS;
4435+
zend_hash_move_forward_ex(ht_keys, &ptr))
4436+
{
4437+
char *key;
4438+
int key_free, key_len;
4439+
zval *z_tmp = NULL;
4440+
4441+
if(Z_TYPE_PP(z_data) == IS_STRING) {
4442+
key = Z_STRVAL_PP(z_data);
4443+
key_len = Z_STRLEN_PP(z_data);
4444+
} else {
4445+
MAKE_STD_ZVAL(z_tmp);
4446+
*z_tmp = **z_data;
4447+
convert_to_string(z_tmp);
4448+
4449+
key = Z_STRVAL_P(z_tmp);
4450+
key_len = Z_STRLEN_P(z_tmp);
44484451
}
4449-
}
44504452

4451-
/* weight */
4452-
if(weights_array != NULL) {
4453-
cmd_len = redis_cmd_format(&cmd,
4454-
"%s" /* cmd */
4455-
"$7" _NL
4456-
"WEIGHTS" _NL
4457-
, cmd, cmd_len);
4458-
cmd_elements++;
4459-
4460-
for (zend_hash_internal_pointer_reset_ex(arr_weights_hash, &pointer);
4461-
zend_hash_get_current_data_ex(arr_weights_hash, (void**) &data, &pointer) == SUCCESS;
4462-
zend_hash_move_forward_ex(arr_weights_hash, &pointer)) {
4463-
4464-
// Ignore non numeric arguments, unless they're the special Redis numbers
4465-
// "inf" ,"-inf", and "+inf" which can be passed as weights
4466-
if (Z_TYPE_PP(data) != IS_LONG && Z_TYPE_PP(data) != IS_DOUBLE &&
4467-
strncasecmp(Z_STRVAL_PP(data), "inf", sizeof("inf")) != 0 &&
4468-
strncasecmp(Z_STRVAL_PP(data), "-inf", sizeof("-inf")) != 0 &&
4469-
strncasecmp(Z_STRVAL_PP(data), "+inf", sizeof("+inf")) != 0)
4470-
{
4471-
continue;
4472-
}
4453+
// Apply key prefix if necessary
4454+
key_free = redis_key_prefix(redis_sock, &key, &key_len TSRMLS_CC);
44734455

4474-
old_cmd = NULL;
4475-
if(*cmd) {
4476-
old_cmd = cmd;
4477-
}
4456+
// Append this input set
4457+
redis_cmd_append_sstr(&cmd, key, key_len);
44784458

4479-
if(Z_TYPE_PP(data) == IS_LONG) {
4480-
cmd_len = redis_cmd_format(&cmd,
4481-
"%s" /* cmd */
4482-
"$%d" _NL /* data_len */
4483-
"%d" _NL /* data */
4484-
, cmd, cmd_len
4485-
, integer_length(Z_LVAL_PP(data)), Z_LVAL_PP(data));
4486-
4487-
} else if(Z_TYPE_PP(data) == IS_DOUBLE) {
4488-
cmd_len = redis_cmd_format(&cmd,
4489-
"%s" /* cmd */
4490-
"$%f" _NL /* data, including size */
4491-
, cmd, cmd_len
4492-
, Z_DVAL_PP(data));
4493-
} else if(Z_TYPE_PP(data) == IS_STRING) {
4494-
cmd_len = redis_cmd_format(&cmd,
4495-
"%s" /* cmd */
4496-
"$%d" _NL /* data len */
4497-
"%s" _NL /* data */
4498-
, cmd, cmd_len, Z_STRLEN_PP(data),
4499-
Z_STRVAL_PP(data), Z_STRLEN_PP(data));
4500-
}
4459+
// Free our key if it was prefixed
4460+
if(key_free) efree(key);
45014461

4502-
// keep track of elements added
4503-
cmd_elements++;
4504-
if(old_cmd) {
4505-
efree(old_cmd);
4506-
}
4507-
}
4508-
}
4462+
// Free our temporary z_val if it was converted
4463+
if(z_tmp) {
4464+
zval_dtor(z_tmp);
4465+
efree(z_tmp);
4466+
z_tmp = NULL;
4467+
}
4468+
}
45094469

4510-
if(operation_len != 0) {
4511-
char *old_cmd = NULL;
4512-
old_cmd = cmd;
4513-
cmd_len = redis_cmd_format(&cmd,
4514-
"%s" /* cmd */
4515-
"$9" _NL
4516-
"AGGREGATE" _NL
4517-
"$%d" _NL
4518-
"%s" _NL
4519-
, cmd, cmd_len
4520-
, operation_len, operation, operation_len);
4521-
cmd_elements += 2;
4522-
efree(old_cmd);
4523-
}
4470+
// Weights
4471+
if(ht_weights != NULL) {
4472+
// Append "WEIGHTS" argument
4473+
redis_cmd_append_sstr(&cmd, "WEIGHTS", sizeof("WEIGHTS") - 1);
45244474

4525-
old_cmd = cmd;
4526-
cmd_len = redis_cmd_format(&cmd,
4527-
"*%d" _NL
4528-
"%s"
4529-
, cmd_elements
4530-
, cmd, cmd_len);
4531-
efree(old_cmd);
4475+
// Process weights
4476+
for(zend_hash_internal_pointer_reset_ex(ht_weights, &ptr);
4477+
zend_hash_get_current_data_ex(ht_weights, (void**)&z_data, &ptr)==SUCCESS;
4478+
zend_hash_move_forward_ex(ht_weights, &ptr))
4479+
{
4480+
// Ignore non numeric arguments, unless they're special Redis numbers
4481+
if (Z_TYPE_PP(z_data) != IS_LONG && Z_TYPE_PP(z_data) != IS_DOUBLE &&
4482+
strncasecmp(Z_STRVAL_PP(z_data), "inf", sizeof("inf")) != 0 &&
4483+
strncasecmp(Z_STRVAL_PP(z_data), "-inf", sizeof("-inf")) != 0 &&
4484+
strncasecmp(Z_STRVAL_PP(z_data), "+inf", sizeof("+inf")) != 0)
4485+
{
4486+
// We should abort if we have an invalid weight, rather than pass
4487+
// a different number of weights than the user is expecting
4488+
efree(cmd.c);
4489+
RETURN_FALSE;
4490+
}
45324491

4533-
REDIS_PROCESS_REQUEST(redis_sock, cmd, cmd_len);
4534-
IF_ATOMIC() {
4535-
redis_long_response(INTERNAL_FUNCTION_PARAM_PASSTHRU, redis_sock, NULL, NULL);
4536-
}
4537-
REDIS_PROCESS_RESPONSE(redis_long_response);
4492+
// Append the weight based on the input type
4493+
switch(Z_TYPE_PP(z_data)) {
4494+
case IS_LONG:
4495+
redis_cmd_append_sstr_long(&cmd, Z_LVAL_PP(z_data));
4496+
break;
4497+
case IS_DOUBLE:
4498+
redis_cmd_append_sstr_dbl(&cmd, Z_DVAL_PP(z_data));
4499+
break;
4500+
case IS_STRING:
4501+
redis_cmd_append_sstr(&cmd, Z_STRVAL_PP(z_data), Z_STRLEN_PP(z_data));
4502+
break;
4503+
}
4504+
}
4505+
}
4506+
4507+
// Aggregation options, if we have them
4508+
if(agg_op_len != 0) {
4509+
redis_cmd_append_sstr(&cmd, "AGGREGATE", sizeof("AGGREGATE") - 1);
4510+
redis_cmd_append_sstr(&cmd, agg_op, agg_op_len);
4511+
}
45384512

4513+
// Kick off our request
4514+
REDIS_PROCESS_REQUEST(redis_sock, cmd.c, cmd.len);
4515+
IF_ATOMIC() {
4516+
redis_long_response(INTERNAL_FUNCTION_PARAM_PASSTHRU, redis_sock, NULL, NULL);
4517+
}
4518+
REDIS_PROCESS_RESPONSE(redis_long_response);
45394519
}
45404520

45414521
/* zInter */

0 commit comments

Comments
 (0)