Skip to content

Fix CopySpace::is_live #1317

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 2 commits into from
Closed

Fix CopySpace::is_live #1317

wants to merge 2 commits into from

Conversation

k-sareen
Copy link
Collaborator

This commit fixes the CopySpace::is_live function to check if a to-space object's address is actually below the current page resource cursor. By definition, a to-space object cannot be live if it is beyond the cursor.

This commit fixes the `CopySpace::is_live` function to check if a to-space object's address is actually below the current page resource cursor. By definition, a to-space object cannot be live if it is beyond the cursor.
@k-sareen
Copy link
Collaborator Author

k-sareen commented May 10, 2025

Ran into this bug when I was using fixed extent semi-spaces. Since we don't zero pages on release, we could have stale data there from the last GC wherein the (dead) object is beyond the cursor but it was forwarded so both !self.is_from_space() and is_forwarded will return true. The fix now makes it clearer that it needs to be mutually exclusive, i.e. we can't have forwarded objects in to-space.

@qinsoon
Copy link
Member

qinsoon commented May 10, 2025

Ran into this bug when I was using fixed extent semi-spaces. Since we don't zero pages on release, we could have stale data there from the last GC wherein the (dead) object is beyond the cursor but it was forwarded so both !self.is_from_space() and is_forwarded will return true. The fix now makes it clearer that it needs to be mutually exclusive, i.e. we can't have forwarded objects in to-space.

The function is_live requires an object reference. But in this case, what gets passed to the function is not a valid object reference.

@k-sareen
Copy link
Collaborator Author

Okay sure -- I'm using it in a bit of a non-standard way since I'm using it to linear scan the heap.

@wks
Copy link
Collaborator

wks commented May 12, 2025

Okay sure -- I'm using it in a bit of a non-standard way since I'm using it to linear scan the heap.

The standard way to linearly scan the heap uses the VO bit. But right after tracing, some metadata may happen to be set precisely at valid objects. It is the mark bits for ImmixSpace. But for CopySpace, maybe it should be both the forwarding bits and the cursor.

You may make a customized linear-scanning function with a parameterized "is_valid_object" function, similar to slice::sort_by with the comparing function as a parameter.

@k-sareen
Copy link
Collaborator Author

But right after tracing, some metadata may happen to be set precisely at valid objects.

Right. I'm only doing it right after a GC so it should be all the valid objects. I just don't use the VO bit because I don't want to add it to the allocation fast-path in the general case. The above is mainly only for creating the bootimage.

@k-sareen k-sareen closed this May 14, 2025
@k-sareen k-sareen deleted the k-sareen-patch-2 branch May 14, 2025 13:59
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