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

Fix keytable size calculation. #1300

Closed
wants to merge 3 commits into from
Closed

Fix keytable size calculation. #1300

wants to merge 3 commits into from

Conversation

hsb0818
Copy link
Contributor

@hsb0818 hsb0818 commented Sep 27, 2013

The total length should be divided by 'keystep'.

mattsta pushed a commit to mattsta/redis that referenced this pull request Aug 2, 2014
We only need to allocate memory for key positions, not
for every argument position.  The most keys we can have
is total arguments / keystep.

Closes redis#1300
mattsta pushed a commit to mattsta/redis that referenced this pull request Aug 2, 2014
We only need to allocate memory for key positions, not
for every argument position.  The most keys we can have
is total arguments / keystep.

Closes redis#1300
@mattsta mattsta mentioned this pull request Aug 2, 2014
mattsta pushed a commit to mattsta/redis that referenced this pull request Aug 6, 2014
We only need to allocate memory for key positions, not
for every argument position.  The most keys we can have
is total arguments / keystep.

Closes redis#1300
@mattsta
Copy link
Contributor

mattsta commented Aug 25, 2014

Denied because "command getkeys mset a 1 b will allocate 6 bytes for two 4 bytes integers. Moreover the commit is pointless because when there is a first/last/step key discovery pattern, you can only allocate like 2x, for example with MSET, you never have pathological cases like SADD key ... a very large number of elements ... where you allocate a lot of memory with just a key. Not worth to make the code more complex IMHO."

@mattsta mattsta closed this Aug 25, 2014
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.

None yet

2 participants