-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
sizehint! now supports shrinking arrays #2879 #26201
Conversation
sizehint! reduces the allocated space in some cases
Top shows we are using 3.144g of memory.
After using
|
src/array.c
Outdated
|
||
if (sz <= a->maxsize) { | ||
size_t dec = a->maxsize - sz; | ||
//if we dont save >10% of maxsize then its not worth it to shrink |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would make it a factor of 2, since that's the increment that we grow by.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I disagree, going from 2 GB to 1.2 GB is a large gain. sizehint!
-ing to a value shorter than the array is likely rare, and if you do, you likely want to regain the memory.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A possible rationale for the different behavior when shrinking vs. when growing is that you strictly need to increase the storage size to push!
a new element, so it makes sense to directly double the size to avoid doing it repeatedly. OTC, to call pop!
, you don't strictly need to shrink the storage size: you can perfectly call resize!
after a bunch of pop!
calls, and it will be more efficient. So it makes sense to optimize for this case and assume the user really wants the exact requested size.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree; if somebody specifically sizehint!
s to a smaller size, the assumption that the array will keep growing doesn't really hold any more. We could compromise on 25% maybe?
if (sz <= a->maxsize) { | ||
size_t dec = a->maxsize - sz; | ||
//if we dont save at least an eighth of maxsize then its not worth it to shrink | ||
if (dec < a->maxsize / 8) return; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a->maxsize >> 3
? Though the compiler may do this optimization for us.
Putting a reference to #33974 here. cc @adrianisuru |
sizehint! reduces the allocated space in some cases