-
Notifications
You must be signed in to change notification settings - Fork 7
Make heap segment limits and sizes configurable #49
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
Conversation
|
Open question: I was considering moving the |
|
out of curiosity - what is the motivation for this? Did you find that some default allocation sizes were inappropriate for project X? (icon I assume) |
Yes, that's exactly it. The default "large limit" (i.e. the threshold after which allocations go into the "huge" category where allocations are one-off) is 64 KiB. I'm going off memory here, but some halo exchanges are of the order ~100s of KiB, meaning they go into the huge category. This also means they get reallocated every halo exchange (except for one; one is kept around by default), and in turn at least one new IPC handle is created every halo exchange, and creating IPC handles is very expensive. This PR makes it 1. customizable and 2. increases the defaults (not yet, I need to test what are good enough defaults). Most small-ish (~1 Mib at least?) allocations should almost always go into the regular buckets of allocations and the backing allocations should almost always stay around. |
|
The major todos are cleaned up now, but the question of defaults (I'm testing at the moment) and whether to move the num reserved segments and never free parameters to the config (API break) are still open. |
|
I've done some tests on a few icon experiments and to make all halo exchanges always come from the large segments the large segment/large limit would have to be at least:
I see three (somewhat overlapping) options:
I'm leaning toward:
Any objections? @tehrengruber any comments from the PMAP perspective? |
I'll make these changes today. That should remove the need to almost ever change the defaults. |
…located more frequently
| // Note: sizes below are defaults and can be changed through heap_config and | ||
| // environment variables. | ||
| // |
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.
This is the final TODO unless there are other comments: the example below is based on the old defaults and should be updated before merging.
|
I've updated the defaults and on mch-ch2 the defaults are enough to avoid reallocations. If I'm not completely off with my calculations, using the new defaults, if all the size classes are fully used and allocated up to |
boeschf
left a comment
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.
Looks good to me - thank you! A few minor things about redundant implementations that could be factored out.
boeschf
left a comment
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.
lgtm, thanks
|
Apparently I have some tests to fix... They seem to pass locally for me so not quite sure what's going wrong there. I'll have a look. Thanks for all the comments so far @boeschf! |
|
I think I've fixed the test as well now. I messed up the return type of |
Allows configuring the
heapobject through aheap_configobject.Additionally uses a
heap_configobject initialized through environment variables as the default value for theheap_configgiven to theheapconstructor.This allows a simple way to change the limits and sizes globally, but allows locally overriding if one needs more control when constructing a
heap.The defaults have been increased.