Modules: add RM_GetCommandKeys(). #7884
Conversation
src/module.c
Outdated
| if (res->numkeys == res->size) { | ||
| /* If we're using the preallocated array, reallocate on the heap | ||
| * and move the data. Otherwise we can just realloc. */ | ||
| int newsize = res->size * 2; |
There was a problem hiding this comment.
above a certain size, *2 may be too much.
we can consider just adding 1000 instead.
There was a problem hiding this comment.
Capping the doubling is reasonable but I wouldn't want to use intervals of 1000 on extreme cases.
src/module.c
Outdated
| @@ -672,12 +671,12 @@ int moduleGetCommandKeysViaAPI(struct redisCommand *cmd, robj **argv, int argc, | |||
| ctx.module = cp->module; | |||
| ctx.client = NULL; | |||
| ctx.flags |= REDISMODULE_CTX_KEYS_POS_REQUEST; | |||
| if (!result->keys) result->keys = result->keysbuf; /* Initialize to use preallocated array */ | |||
There was a problem hiding this comment.
isn't the init with the preallocated array already handled by the caller?
There was a problem hiding this comment.
No, it can't without a macro that takes the name of the variable.
There was a problem hiding this comment.
ohh right, the caller does GETKEYS_RESULT_INIT which doesn't do that.
it's prepareGetKeysResult that does it (static and only used in db.c).
and it is designed to be used once (not repeatedly).
i guess what i was thinking is to make it non-static, and change it in a way that it can be called repeatedly, then it can be used in both moduleGetCommandKeysViaAPI (first init), and also in RM_KeyAtPos (doing the reallocs).
just a thought, your way is good too.
Avoid using a static buffer for short key index responses, and make it caller's responsibility to stack-allocate a result type. Responses that don't fit are still allocated on the heap.
This is essentially the same as calling COMMAND GETKEYS but provides a more efficient interface that can be used in every context (i.e. not a Redis command).
This is essentially the same as calling COMMAND GETKEYS but provides a more efficient interface that can be used in every context (i.e. not a Redis command).
This pull request also includes refactoring of the
getKeysFromCommand()interface to eliminate the previously used static buffer.