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 alloc_with_options #1218

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
Open

Add alloc_with_options #1218

wants to merge 14 commits into from

Conversation

qinsoon
Copy link
Member

@qinsoon qinsoon commented Oct 29, 2024

This PR adds an alternative allocation functions alloc_with_options. The attached AllocationOptions change the allocation behavior, such as avoiding GCs and allowing overcommit.

Our current alloc functions assume they are GC safe points and will trigger GCs internally. A runtime may have different assumptions. We see GHC has allocateMightFail (https://gitlab.haskell.org/ghc/ghc/-/blob/90746a591919fc51a0ec9dec58d8f1c8397040e3/rts/sm/Storage.c?page=2#L1089). Also Julia assumes perm alloc will not trigger a GC (mmtk/mmtk-julia#172).
Having a variant of alloc that is not GC safepoints could be generally useful.

@qinsoon qinsoon marked this pull request as ready for review October 30, 2024 02:37
Copy link
Collaborator

@wks wks left a comment

Choose a reason for hiding this comment

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

I changed parts of the doc comments of alloc, alloc_no_gc, alloc_slow and alloc_slow_no_gc. Particularly, I rewrote the comment of alloc to make it more ordered.

@qinsoon
Copy link
Member Author

qinsoon commented Oct 30, 2024

I changed the documentation based on the suggestions.

/// normally without panicking or throwing exceptions, this function will return zero.
///
/// This function in most cases returns a valid memory address.
/// This function may return a zero address iif 1. MMTk attempts at least one GC,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
/// This function may return a zero address iif 1. MMTk attempts at least one GC,
/// This function may return a zero address if 1. MMTk attempts at least one GC,

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually this paragraph should be removed.

@qinsoon qinsoon added the PR-extended-testing Run extended tests for the pull request label Oct 31, 2024
Copy link
Collaborator

@wks wks left a comment

Choose a reason for hiding this comment

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

LGTM

@qinsoon
Copy link
Member Author

qinsoon commented Nov 1, 2024

I’m awaiting feedback from @JunmingZhao42 and Ben before merging this PR, as they requested this feature.

@qinsoon qinsoon changed the title Add alloc_no_gc Add alloc_with_options Feb 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR-extended-testing Run extended tests for the pull request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants