Skip to content

sbrk_aligned() #20785

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

Closed
wants to merge 1 commit into from
Closed

sbrk_aligned() #20785

wants to merge 1 commit into from

Conversation

juj
Copy link
Collaborator

@juj juj commented Nov 27, 2023

This PR shows how we could avoid the pessimistic size+alignment rounding up when sbrk()ing to acquire an aligned allocation. A follow-up to #20704.

@kripken
Copy link
Member

kripken commented Nov 27, 2023

Interesting idea! This should be more efficient since it lets sbrk increase the size only when actually necessary to accommodate the alignment.

Looks like it increases code size somewhat - do you think it's worth it? I'm on the fence myself, since the extra overhead that this removes is only in special cases (like mimalloc) that allocate with very high alignment, and even then the extra space can be added to the previous region at least.

@juj
Copy link
Collaborator Author

juj commented Nov 27, 2023

Looks like it increases code size somewhat - do you think it's worth it?

That is the question I was pondering also, and I thought to let the CI answer how big the size regression would actually be. But it looks like the CI is currently failing on code size tests in main branch already, given that the other PR #20784 is already failing, so not sure what the real delta would be. Is there a PR yet to rebaseline the code size tests on main?

@sbc100
Copy link
Collaborator

sbc100 commented Nov 27, 2023

Looks like it increases code size somewhat - do you think it's worth it?

That is the question I was pondering also, and I thought to let the CI answer how big the size regression would actually be. But it looks like the CI is currently failing on code size tests in main branch already, given that the other PR #20784 is already failing, so not sure what the real delta would be. Is there a PR yet to rebaseline the code size tests on main?

I just landed b7924c4

@juj
Copy link
Collaborator Author

juj commented Nov 28, 2023

Yeah, I am leaning as well that this is not worth it, so closing.

@juj juj closed this Nov 28, 2023
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