Add alignment block splitting to malloc_freelist#91
Add alignment block splitting to malloc_freelist#91larkmjc wants to merge 1 commit intoembeddedartistry:masterfrom
Conversation
The main allocator is modified to split blocks to align them. The fundamental algorithm is mostly unchanged although there is some additional logic to calculate alignment_slack in the case that the alignment is greater than the alignment of the found block. The alignment_slack is used to split the block with the surplus returned to the freelist. Some indent has been removed from the code for readability. aligned_free is now redundant but it is kept for compatibility. It is no longer necessary to unwrap an offset field in wrapper headers as the main allocation header block is aligned and free can be called directly on aligned allocations, so aligned_free now simply delegates to free(). The documentation is updated to reflect these changes.
|
Thanks for the PR! I am traveling this week, and I will give it a detailed look when I returned.
|
|
Thanks! It's mostly just a heads up. Take your time... It hadn't occurred to me but perhaps you indented code on the bodies of if conditions as a secure coding practice because if-not-clause-or-not-clause with an early return could be more prone to logic errors than an if-and-clause-and-clause with an indented block. In any case it should not be too hard to change the indentation if you prefer it the other way. |
|
curious if you had a chance to look at this PR more closely. |
|
Hey @michaeljclark, really sorry about the delay here. I went on that trip and came back facing complete burn out and did not do any work for quite a while. Just getting back into the swing of things and still getting caught up on the backlog. Anyway, I do like the change. It does have some implications re: structure of the library, now that aligned_malloc.c doesn't need to be its own thing. I need to think about how that will change as a result. I'll likely tweak this, preserving the commit credit for you of course. You did guess right re: the |
Description
The main allocator is modified to split blocks to align them, returning surplus to the free list.
I am sharing this because I think this approach is a little cleaner than the existing approach. It has several advantages and adds minimal complexity. It has been tested locally but it needs more testing and review. The primary code path should be unchanged for regular allocations besides dropping the minimum allocation size and calculating alignment_slack which should always be zero for regular allocations.
The fundamental algorithm is mostly unchanged although there is additional logic to calculate alignment_slack in the case that the alignment is greater than the alignment of the found block. If alignment_slack is non-zero it is used to split the block so that block is sufficiently aligned with the surplus returned to the free list. Some indent has been removed for readability.
aligned_free is now redundant but it is kept for compatibility. It is no longer necessary to unwrap an offset field in wrapper headers as the main allocation header block is aligned and free can be called directly on aligned allocations, so aligned_free now simply delegates to free(). Also, the surplus memory to align blocks is not wasted and can be allocated.
This changes needs rigorous testing and review. This is a heads up!
Type of change
How Has This Been Tested?
libmemory_freelist_testtest passes.Test Configuration:
Checklist: