Skip to content
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

Add a heap allocator #133

Merged
merged 1 commit into from
May 6, 2020
Merged

Add a heap allocator #133

merged 1 commit into from
May 6, 2020

Conversation

SamTebbs33
Copy link
Collaborator

@SamTebbs33 SamTebbs33 commented Apr 22, 2020

This patch adds a heap allocator that in contrast to the fixed buffer allocator, allows freeing of memory. One is created by default to keep track of kernel memory and more heaps could be created to keep track of memory occupied by user programs etc.

The heap uses a buddy allocation system as it provides efficient allocation, freeing and little external fragmentation.

The current implementation doesn't support complex shrinking or re-allocation so I will create an issue for these that can be addressed if they actually end up being used.

Closes #8

Copy link
Member

@DrDeano DrDeano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs some more tests, and maybe a runtime tests, but can't wait to start using

src/kernel/kmain.zig Outdated Show resolved Hide resolved
src/kernel/heap.zig Outdated Show resolved Hide resolved
src/kernel/kmain.zig Outdated Show resolved Hide resolved
src/kernel/heap.zig Outdated Show resolved Hide resolved
src/kernel/heap.zig Outdated Show resolved Hide resolved
src/kernel/heap.zig Show resolved Hide resolved
src/kernel/heap.zig Show resolved Hide resolved
src/kernel/heap.zig Outdated Show resolved Hide resolved
src/kernel/heap.zig Show resolved Hide resolved
src/kernel/heap.zig Show resolved Hide resolved
Copy link
Member

@DrDeano DrDeano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Almost there :P

src/kernel/heap.zig Outdated Show resolved Hide resolved
// Freeing will error if the pointer was never allocated in the first place, but the Allocator API doesn't allow errors from shrink so use unreachable
// It's not nice but is the only thing that can be done. Besides, if the unreachable is ever triggered then a double-free bug has been found
heap.free(@ptrToInt(&old_mem[0]), old_mem.len) catch unreachable;
if (new_size != 0) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probs don't need to add this now, but maybe add a issue, but to make this more efficient, could just free the memory that is to be shrinked and not all the memory then allocate again. Like heap.free(@ptrToInt(&old_mem[new_size]), old_mem.len - new_size) catch unreachable; If this easy to do, then could do this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is a nice idea, but I don't think it would work all the time because of how the buddy system is structured. If the start of the memory that is no longer needed is in the same block as the memory that is still needed all of it will be freed. For example if old_mem starts at address 0, has length 32 and the min block size is 16 then a shrink to length 10 would try to free 11..32, but because of how the structure works this would end up freeing the whole allocation from 0 to 32, which we wouldn't want. If we find a way to do it more efficiently than freeing and re-allocating that would work in all cases I'm up for it :)

Copy link
Member

@DrDeano DrDeano May 6, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes, so could have new_size be aligned up to the nearest block size; std.mem.alignForward(new_size, block_size).

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that would still have the same issue

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, can create an issue maybe to say that there could be some optimisation here.

Copy link
Collaborator Author

@SamTebbs33 SamTebbs33 May 6, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea :) There definitely should be something we can do.

src/kernel/heap.zig Show resolved Hide resolved
src/kernel/heap.zig Outdated Show resolved Hide resolved
src/kernel/heap.zig Outdated Show resolved Hide resolved
src/kernel/heap.zig Outdated Show resolved Hide resolved
src/kernel/heap.zig Show resolved Hide resolved
@SamTebbs33 SamTebbs33 merged commit 9cf4858 into develop May 6, 2020
@SamTebbs33 SamTebbs33 deleted the feature/heap-allocator branch May 6, 2020 22:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Heap memory manager
2 participants