Skip to content

Add pointer pinning to handle internal pointers. Use jl_active_task_stack. #217

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

Merged
merged 12 commits into from
Feb 5, 2025

Conversation

qinsoon
Copy link
Member

@qinsoon qinsoon commented Jan 8, 2025

This PR adds pinning that handles internal pointers. It also uses jl_active_task_stack for stack scanning instead of jl_task_stack_buffer.

@qinsoon qinsoon changed the title Remove transitive pinning of global roots Add pointer pinning to handle internal pointers. Use jl_active_task_stack. Jan 29, 2025
@qinsoon qinsoon requested a review from udesou January 29, 2025 03:05
Updating `dev`. Merge with mmtk/julia#85.

---------

Co-authored-by: Stephen Kell <srk31-github@srcf.ucam.org>
Co-authored-by: Yi Lin <qinsoon@gmail.com>
Co-authored-by: Luis Eduardo de Souza Amorim <eduardo@rat.moma>
@qinsoon
Copy link
Member Author

qinsoon commented Feb 3, 2025

julia-version
JULIA_REPO=qinsoon/Julia
JULIA_REF=33f571889f7f569ad5f42ddb457c62d243feb870

mmtk/src/api.rs Outdated
pub extern "C" fn mmtk_pin_pointer(addr: Address) -> bool {
crate::early_return_for_non_moving_build!(false);
if mmtk_object_is_managed_by_mmtk(addr.as_usize()) {
if !crate::object_model::is_addr_in_immixspace(addr) {
Copy link
Contributor

Choose a reason for hiding this comment

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

You only need to pin objects in the immixspace, correct? Wouldn't be enough to do a single check if crate::object_model::is_addr_in_immixspace(addr) instead of having the check mmtk_object_is_managed_by_mmtk(addr.as_usize()) ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. It should be fine. I made the change.

Copy link
Contributor

@udesou udesou 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 qinsoon merged commit 36b2245 into mmtk:dev Feb 5, 2025
26 checks passed
qinsoon added a commit to qinsoon/mmtk-julia that referenced this pull request Feb 6, 2025
…stack`. (mmtk#217)

This PR adds pinning that handles internal pointers. It also uses
`jl_active_task_stack` for stack scanning instead of `jl_task_stack_buffer`.
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.

2 participants