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

merge front/back padding after allocate current hole #54

Merged
merged 1 commit into from Oct 16, 2021

Conversation

yodalee
Copy link
Contributor

@yodalee yodalee commented Aug 25, 2021

When we allocate a memory chunk, there may be some memory paddings
in front/back of the allocated chunk. Before this change, these paddings
are dealed by calling function deallocate.
However the implementation is inefficient since deallocate function will
search from the beginning of the linked-list and find the right place to
insert these paddings.
With this change, the paddings are linked to linked-list right after
they are generated. We don't have to deal with merge continuous since
paddings are guaranteed to be separated between them and to previous and
next holes.

Fixes #53

Copy link
Member

@phil-opp phil-opp left a comment

Choose a reason for hiding this comment

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

Thanks a lot for the PR and sorry for the late review. Looks good to me!

Unfortunately I missed the time window to approve the CI run, which is required by GitHub because this is your first contribution here. Could you push some dummy update to this PR (e.g. an empty commit or a no-op rebase) so that we retry this? Please ping me when you do.

@yodalee
Copy link
Contributor Author

yodalee commented Oct 16, 2021

@phil-opp
Done. I use rebase/reword to change the commit. It is updated now.

@phil-opp phil-opp enabled auto-merge (squash) October 16, 2021 13:24
@phil-opp
Copy link
Member

Thanks a lot! Looks like there is a minor rustfmt issue, otherwise this seems good to go

When we allocate a memory chunk, there may be some memory paddings
in front/back of the allocated chunk. Before this change, these paddings
are dealed by calling function deallocate.
However the implementation is inefficient since deallocate function will
search from the beginning of the linked-list and find the right place to
insert these paddings.
With this change, the paddings are linked to linked-list right after
they are generated. We don't have to deal with merge continuous since
paddings are guaranteed to be separated between them and to previous and
next holes.
auto-merge was automatically disabled October 16, 2021 14:07

Head branch was pushed to by a user without write access

@yodalee
Copy link
Contributor Author

yodalee commented Oct 16, 2021

@phil-opp
Updated.
Out-of-curious, why the CI need approval to run? Usually it should automatically check PR that is being pushed.

@phil-opp phil-opp merged commit acd9867 into rust-osdev:master Oct 16, 2021
@phil-opp
Copy link
Member

Thanks!

Out-of-curious, why the CI need approval to run? Usually it should automatically check PR that is being pushed.

See https://github.blog/2021-04-22-github-actions-update-helping-maintainers-combat-bad-actors/. Basically, there were some people that abused the automatic CI runs to mine cryptocurrency, so GitHub made manual approvals for first-time contributors manditory.

phil-opp added a commit that referenced this pull request Oct 16, 2021
@phil-opp
Copy link
Member

I'll publish this together with #55 as soon as its merged.

@yodalee
Copy link
Contributor Author

yodalee commented Oct 16, 2021

See. Glad to being accepted.

@yodalee yodalee deleted the link-holes-after-allocation branch October 16, 2021 14:19
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.

Merge padding inside global allocate_first_fit
2 participants