Skip to content

optimize memory usage of deferred replies#7146

Merged
antirez merged 1 commit intoredis:unstablefrom
oranagra:optimize_deferred_reply
Apr 27, 2020
Merged

optimize memory usage of deferred replies#7146
antirez merged 1 commit intoredis:unstablefrom
oranagra:optimize_deferred_reply

Conversation

@oranagra
Copy link
Member

When deffered reply is added the previous reply node cannot be used so
all the extra space we allocated in it is wasted. in case someone uses
deffered replies in a loop, each time adding a small reply, each of
these reply nodes (the small string reply) would have consumed a 16k
block.
now when we add anther diferred reply node, we trim the unused portion
of the previous reply block.

see #7123

When deffered reply is added the previous reply node cannot be used so
all the extra space we allocated in it is wasted. in case someone uses
deffered replies in a loop, each time adding a small reply, each of
these reply nodes (the small string reply) would have consumed a 16k
block.
now when we add anther diferred reply node, we trim the unused portion
of the previous reply block.

see #7123
@oranagra
Copy link
Member Author

@fayadexinqing here's the optimization to deferred replies.
i tried reverting your fix in cluster.c and the new test you added still passes.

@antirez
Copy link
Contributor

antirez commented Apr 24, 2020

Thanks @oranagra, this looks like a good idea to me. Especially because of modules.

Copy link
Contributor

@trevor211 trevor211 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@antirez
Copy link
Contributor

antirez commented Apr 27, 2020

@oranagra reviewed, looks good to me, merging. Thanks!

@antirez antirez merged commit 828736e into redis:unstable Apr 27, 2020
JackieXie168 pushed a commit to JackieXie168/redis that referenced this pull request Apr 28, 2020
optimize memory usage of deferred replies
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

Successfully merging this pull request may close these issues.

3 participants