Skip to content

When converting a set to dict, presize for one more element to be added#11559

Merged
oranagra merged 6 commits intoredis:unstablefrom
zuiderkwast:set-dict-presize-for-plus-one
Dec 6, 2022
Merged

When converting a set to dict, presize for one more element to be added#11559
oranagra merged 6 commits intoredis:unstablefrom
zuiderkwast:set-dict-presize-for-plus-one

Conversation

@zuiderkwast
Copy link
Contributor

In most cases when a listpack or intset is converted to a dict, the conversion is trigged when adding an element. The extra element is added after conversion to dict (in all cases except when the conversion is triggered by set-max-intset-entries being reached).

If set-max-listpack-entries is set to a power of two, let's say 128, when adding the 129th element, the 128 element listpack is first converted to a dict with a hashtable presized for 128 elements. After converting to dict, the 129th element is added to the dict which immediately triggers incremental rehashing to size 256.

This commit instead presizes the dict to one more element, with the assumtion that conversion to dict is followed by adding another element, so the dict doesn't immediately need rehashing.

In most cases when a listpack or intset is converted to a dict, the
conversion is trigged when adding an element. The extra element is added
after conversion to dict (in all cases except when the conversion is
triggered by set-max-intset-entries being reached).

If set-max-listpack-entries is set to a power of two, let's say 128, when
adding the 129th element, the 128 element listpack is first converted
to a dict with a hashtable presized for 128 elements. After converting
to dict, the 129th element is added to the dict which immediately triggers
incremental rehashing to size 256.

This commit instead presizes the dict to one more element, with the
assumtion that conversion to dict is followed by adding another element,
so the dict doesn't immediately need rehashing.
@zuiderkwast zuiderkwast marked this pull request as ready for review November 30, 2022 19:46
Copy link
Contributor

@enjoy-binbin enjoy-binbin left a comment

Choose a reason for hiding this comment

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

make sense to me

@sundb
Copy link
Collaborator

sundb commented Dec 1, 2022

Does dictExpand in hashTypeConvertListpack need to do the same modification?

@zuiderkwast
Copy link
Contributor Author

zuiderkwast commented Dec 1, 2022

Does dictExpand in hashTypeConvertListpack need to do the same modification?

@sundb In hashTypeSet, we check hash_max_listpack_entries and convert to dict after setting (adding) the value.

Co-authored-by: Oran Agra <[email protected]>
@oranagra oranagra merged commit 8a315fc into redis:unstable Dec 6, 2022
madolson pushed a commit to madolson/redis that referenced this pull request Apr 19, 2023
…ed (redis#11559)

In most cases when a listpack or intset is converted to a dict, the conversion
is trigged when adding an element. The extra element is added after conversion
to dict (in all cases except when the conversion is triggered by
set-max-intset-entries being reached).

If set-max-listpack-entries is set to a power of two, let's say 128, when
adding the 129th element, the 128 element listpack is first converted to a dict
with a hashtable presized for 128 elements. After converting to dict, the 129th
element is added to the dict which immediately triggers incremental rehashing
to size 256.

This commit instead presizes the dict to one more element, with the assumption
that conversion to dict is followed by adding another element, so the dict
doesn't immediately need rehashing.

Co-authored-by: sundb <[email protected]>
Co-authored-by: Oran Agra <[email protected]>
enjoy-binbin pushed a commit to enjoy-binbin/redis that referenced this pull request Jul 31, 2023
…ed (redis#11559)

In most cases when a listpack or intset is converted to a dict, the conversion
is trigged when adding an element. The extra element is added after conversion
to dict (in all cases except when the conversion is triggered by
set-max-intset-entries being reached).

If set-max-listpack-entries is set to a power of two, let's say 128, when
adding the 129th element, the 128 element listpack is first converted to a dict
with a hashtable presized for 128 elements. After converting to dict, the 129th
element is added to the dict which immediately triggers incremental rehashing
to size 256.

This commit instead presizes the dict to one more element, with the assumption
that conversion to dict is followed by adding another element, so the dict
doesn't immediately need rehashing.

Co-authored-by: sundb <[email protected]>
Co-authored-by: Oran Agra <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants