rhempel / umm_malloc

Memory Manager For Small(ish) Microprocessors
MIT License
395 stars 105 forks source link

Add support for multi-heaps #66

Closed stellar-aria closed 1 year ago

stellar-aria commented 1 year ago

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:

I 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.

rhempel commented 1 year ago

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 :-)

stellar-aria commented 1 year ago

Sure thing!

rhempel commented 1 year ago

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.

rhempel commented 1 year ago

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.

stellar-aria commented 1 year ago

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.

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 :)

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.

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().

rhempel commented 1 year ago

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!