Add support for multi-heaps #66
Conversation
|
Hey I'm just on vacation but I really appreciate this contribution! Could I ask that we do not rename the main file so that the changes are easier to review using the GitHub diff tools? A rename can be done later in a separate commit when we are happy. I haven't reviewed the changes in detail yet - next week is more likely. I'm not ignoring the request, just not working this week :-) |
|
Sure thing! |
|
Based on the other repos you have and your profile, I am guessing you are working on synths and music/sound controllers. This is something I have a passing interest in and would be interested in sparring with you for your project - if you are open to that. I'm not at all working on any audio projects, my history is with fire alarm systems and of course LEGO related electronics and firmware. I support hacking and enjoy sparring/mentoring/sharing experiences. Feel free to reach out to me on LinkedIn or email - I'm not hard to find. |
|
Overall a really welcome contribution and super clean implementation - thanks so much for this. Could I also ask you to add some notes to the README that include credit for your work right at the top, as well as notes on the new API additions and typical use cases. I am guessing that for most users they would use the standard umm_malloc() and umm_free() functions on the default stack and then use the multi-functions on a secondary stack that they have allocated separately, or they would always use the multi stack (that's probably a better pattern). One nice note to add is that this is a way to make multiple fixed size pools (for which there are better algorithms like the uPython memory allocator) but pools are great for fixed size items because they avoid fragmentation. What's really cool about this change is that we don't need to bump the major version number because the change is backward compatible. Sorry for the picky comments - your work is really awesome but I'm wired to keep the codebase as clean as possible - I really appreciate the updated test cases too. |
Of course, I'd welcome feedback/mentoring on any of the embedded work I do. Most of the code I'm currently working with is not mine, and is instead a legacy project inherited from a company deciding to open-source their main product, so I'm doing modernization and refactoring more often than any actually interesting novel audio programming these days. When I do get back to primarily my own work, feedback would be very appreciated :)
No need to apologize! I totally understand. I'll update the README and add some usage notes. Fixed-sized pools are actually the initial use we're using this for, however we're not using it for fixed sized items. The current custom allocator in the project seems to have trouble with tracking too many allocations and is unfortunately very tightly coupled to a cache[/invalidation] system at the moment, while the scripting engine we're adding (Wren) does a lot of little allocations. Fixing the current allocator is not an easy task, so instead we're simply getting a very large allocation and then relying on a secondary allocator, originally dlmalloc, now umm_malloc to do the fine-grained management. An arena allocator would probably best considering Wren is a dynamic language, but we don't have information regarding potential object lifetime when doing the allocation. If fragmentation becomes a concern I'll probably investigate fixed-sized pooling, but for now this works and is not a huge concern. Ultimately I'm hoping to transition away from the current custom allocation scheme to a substantially more modular one built on umm_malloc, as it fulfills some of the more obscure needs of the current system for in-place expansion and contraction with realloc(). |
|
This is an absolutely beautiful contribution and merge request - thanks so much for making the initial changes and following up with feedback and comments. I hope that making the changes in what I hope is a fairly clean codebase was an energizing experience - I have lacked the motivation to do it for my own use cases and really appreciate the effort, and hope we can continue to collaborate Thanks again! |
This adds support for multi-heaps, similar to dlmalloc's mspace functionality. Rather than relying on umm_malloc to do the coordination on whether or not a pointer belongs in a specific heap, the coordination (and storage of the
umm_heap[_config]) is left to the caller. This allows for easy wrapping to an OO style interface instead of a C one if the user desires.Why?
I was investigating memory allocators specifically for an ARMv7 (Cortex-A9) project that has two separate RAM regions (internal and SDRAM), and this seemed perfect except for the lack of multi-heap support.
Changes:
umm_heap_configdeclaration toumm_malloc.hto allow for known storage size by caller.pheapfield to bevoid*instead ofumm_blocksince I thought it was unnecessary to move more structure definitions to the header, and it's assigned from avoid*anywaysheapthat is now passed to all internal functions rather than relying on theumm_heap_currentumm_heap_config umm_heap_currentumm_malloc.cnew proxy functions that simply callumm_multi_*functions with the movedumm_heap_config umm_heap_currentmulti_*functions and single-heap proxies to info, integrity, and poisonI ran the tests on my machine, but I don't know if there's a CI action set up here or not.
As of 563d7f7 all tests pass for me.