Optimize dictTypeResizeAllowed to avoid mistaken OOM judgement. #12950
Conversation
|
interesting... the reason is that expand and shrink have different expected sizes for the new hash table, right? |
@soloestoy Yes, exactly. However the possibility of |
enjoy-binbin
left a comment
There was a problem hiding this comment.
I thought before that this size should be passed by the caller, otherwise it may be missed
oranagra
left a comment
There was a problem hiding this comment.
conceptually LGTM, but instead of passing a boolean, to the function i think we can pass the size.
the code just below the call to dictTypeResizeAllowed does use d->ht_used[0] with or without +1, so i think it should pass the same one to dictTypeResizeAllowed
…s#12950) When doing dict resizing, dictTypeResizeAllowed is used to judge whether the new allocated memory for rehashing would cause OOM. However when shrinking, we alloc `_dictNextExp(d->ht_used[0])` bytes of memory, while in `dictTypeResizeAllowed` we still use `_dictNextExp(d->ht_used[0]+1)` as the new allocated memory size. This will overestimate the memory used by shrinking at special conditions, causing a false OOM judgement.
…s#12950) When doing dict resizing, dictTypeResizeAllowed is used to judge whether the new allocated memory for rehashing would cause OOM. However when shrinking, we alloc `_dictNextExp(d->ht_used[0])` bytes of memory, while in `dictTypeResizeAllowed` we still use `_dictNextExp(d->ht_used[0]+1)` as the new allocated memory size. This will overestimate the memory used by shrinking at special conditions, causing a false OOM judgement.
When doing dict resizing, dictTypeResizeAllowed is used to judge whether the new allocated memory for rehashing would cause OOM.
However when shrinking, we alloc
_dictNextExp(d->ht_used[0])bytes of memory, while indictTypeResizeAllowedwe still use_dictNextExp(d->ht_used[0]+1)as the new allocated memory size. This will overestimate the memory used by shrinking at special conditions, causing a false OOM judgement.