Skip to content

Improve coarse_alloc() #1249

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ldorau
Copy link
Contributor

@ldorau ldorau commented Apr 10, 2025

Description

Improve coarse_alloc(): move the code responsible for handling the (curr == NULL) case just after curr = find_free_block(). Add coarse_add_free_block().

Now (before this change) when a user calls coarse_alloc(1, 2MB) (allocate 1 byte with 2MB alignment) and there is no suitable blok inside the coarse allocator, a new block is allocated from the provider of size coarse->page_size at least, but it is added to the inner lists with the requested size of only 1 byte and the rest of space will not be able to be utilized.

After this change, when a user calls coarse_alloc(1, 2MB) (allocate 1 byte with 2MB alignment) and there is no suitable blok inside the coarse allocator, a new block is allocated from the provider of size alignment (>= coarse->page_size) and it is added to the inner lists with the alignment size. The new free block will be used as a whole or split and the rest of space can be utilized later.

The sanitizers CI builds fail, because #1225 has not been merged yet.

Checklist

  • Code compiles without errors locally
  • All tests pass locally
  • CI workflows execute properly

@ldorau ldorau requested a review from a team as a code owner April 10, 2025 11:03
@ldorau ldorau requested review from bratpiorka and lplewa April 10, 2025 11:04
@ldorau ldorau changed the title Refactor coarse_alloc() Improve coarse_alloc() Apr 10, 2025
@ldorau
Copy link
Contributor Author

ldorau commented Apr 10, 2025

The sanitizers CI builds fail, because #1225 has not been merged yet.

*resultPtr = NULL;
// If the block that we want to reuse has a greater size, split it.
// Try to merge the split part with the successor if it is not used.
enum { ACTION_NONE = 0, ACTION_USE, ACTION_SPLIT } action = ACTION_NONE;
Copy link
Contributor

Choose a reason for hiding this comment

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

ACTION_NONE
AFAIK, starting enum entry is 0, so there is no need to set it up.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, but IMHO it is more readable as it is now

@ldorau ldorau force-pushed the Refactor_coarse_alloc branch from 609a850 to fd816f5 Compare April 11, 2025 09:32
@lplewa lplewa requested a review from Copilot April 11, 2025 15:21
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot wasn't able to review any files in this pull request.

Files not reviewed (1)
  • src/coarse/coarse.c: Language not supported

@ldorau ldorau force-pushed the Refactor_coarse_alloc branch from fd816f5 to 2718712 Compare April 14, 2025 10:21
@ldorau ldorau requested review from KFilipek, lplewa and Copilot April 14, 2025 10:21
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot wasn't able to review any files in this pull request.

Files not reviewed (1)
  • src/coarse/coarse.c: Language not supported

@ldorau ldorau force-pushed the Refactor_coarse_alloc branch from 2718712 to 7a075dd Compare April 14, 2025 11:07
Refactor coarse_alloc(): move the code responsible for handling
the `(curr == NULL)` case just after `curr = find_free_block()`.
Add coarse_add_free_block().

Now (before this change) when a user calls `coarse_alloc(1, 2MB)`
(allocate 1 byte with 2MB alignment) and there is no suitable blok
inside the coarse allocator, a new block is allocated from the provider
of size `coarse->page_size` at least, but it is added to the inner lists
with the requested size of only 1 byte and the rest of space
will not be able to be utilized.

After this change, when a user calls `coarse_alloc(1, 2MB)`
(allocate 1 byte with 2MB alignment) and there is no suitable blok
inside the coarse allocator, a new block is allocated from the provider
of size `alignment` (>= `coarse->page_size`) and it is added to the inner
lists with the `alignment` size. The new free block will be used
as a whole or split and the rest of space can be utilized later.

Signed-off-by: Lukasz Dorau <[email protected]>
@ldorau ldorau force-pushed the Refactor_coarse_alloc branch from 7a075dd to 3a8cbaa Compare April 14, 2025 14:37
@ldorau
Copy link
Contributor Author

ldorau commented Apr 14, 2025

@bratpiorka please review

@ldorau ldorau requested a review from Copilot April 15, 2025 08:03
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot wasn't able to review any files in this pull request.

Files not reviewed (1)
  • src/coarse/coarse.c: Language not supported

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.

3 participants