Skip to content
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.

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

structure redisReply alignment #30

Closed
mezzatto opened this issue Mar 29, 2011 · 2 comments
Closed

structure redisReply alignment #30

mezzatto opened this issue Mar 29, 2011 · 2 comments

Comments

@mezzatto
Copy link

If you reorder the members of the structure redisReply, it is possible to reduce its size. Like this:

typedef struct redisReply {
int type; /* REDIS_REPLY_* /
long long integer; /
The integer when type is REDIS_REPLY_INTEGER /
int len; /
Length of string /
char *str; /
Used for both REDIS_REPLY_ERROR and REDIS_REPLY_STRING /
size_t elements; /
number of elements, for REDIS_REPLY_ARRAY _/
struct redisReply *_element; /* elements vector for REDIS_REPLY_ARRAY */
} redisReply;

typedef struct redisReply_new {
int type; /* REDIS_REPLY_* /
int len; /
Length of string /
size_t elements; /
number of elements, for REDIS_REPLY_ARRAY /
long long integer; /
The integer when type is REDIS_REPLY_INTEGER /
char *str; /
Used for both REDIS_REPLY_ERROR and REDIS_REPLY_STRING _/
struct redisReply *_element; /* elements vector for REDIS_REPLY_ARRAY */
} redisReplyNew;

Testing:

printf("sizeof(redisReply): %ld\n", sizeof(redisReply));
printf("sizeof(redisReplyNew): %ld\n", sizeof(redisReplyNew));

The output:

sizeof(redisReply): 48
sizeof(redisReplyNew): 40

Obs.: I have a 64 bit system.

@mezzatto
Copy link
Author

mezzatto commented Nov 8, 2011

Any comments about that change?

It can also be done like this:

typedef struct redisReply {
struct redisReply *element; / elements vector for REDIS_REPLY_ARRAY /
char *str; /
Used for both REDIS_REPLY_ERROR and REDIS_REPLY_STRING /
long long integer; /
The integer when type is REDIS_REPLY_INTEGER /
int type; /
REDIS_REPLY_* /
int len; /
Length of string /
size_t elements; /
number of elements, for REDIS_REPLY_ARRAY */
} redisReply;

The above structure also has 40 bytes.

I would really appreciate if this change could be done. My app has many redisReply allocated at the same time. Many processes with many threads, requesting many thing from Redis, doing some stuff and finally calling freeReplyObject().

Do I need to change it myself and submit a pull request?

@pietern
Copy link
Contributor

pietern commented Jul 11, 2013

I think making it a union would be even more interesting in terms of memory footprint (also see #150).

In the next major release we'll also have to add a reference counter field, so things need to be overhauled anyway.

Closing this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants