Skip to content

Conversation

@qinsoon
Copy link
Member

@qinsoon qinsoon commented Sep 23, 2020

Add an API: pub fn is_in_space(mmtk, obj, allocator) -> bool. It checks if the object is located in a space with the given allocation type (allocation semantics). Its implementation should be consistent with the alloc() implementation for each plan (i.e. it should check whether an object is in a space in the same way as how alloc() maps allocation types to each space).

Currently no VM binding is using it, so it is not covered in any tests. The V8 binding needs this API.

@qinsoon qinsoon marked this pull request as ready for review September 23, 2020 06:40
@qinsoon qinsoon added the PR-ready-for-review Pull request ready for review label Sep 23, 2020
Copy link
Contributor

@javadamiri javadamiri left a comment

Choose a reason for hiding this comment

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

As far as I see, is_in_space() is doing what it should do.
Yi and I had a discussion about the supported spaces in mmtk-NOGC, which we thought should be discussed in our meeting too.

Meanwhile, for nogc to work on V8, I added the support for the required spaces to the binding

@wenyuzhao
Copy link
Member

The changes look good to me. Just one question: Why can't we use the SFTMap (https://github.com/mmtk/mmtk-core/blob/master/src/policy/space.rs#L99) to test which space a given object locates in? The SFTMap provides a direct mapping between memory address (chunks) to spaces.

@caizixian
Copy link
Member

The changes look good to me. Just one question: Why can't we use the SFTMap (https://github.com/mmtk/mmtk-core/blob/master/src/policy/space.rs#L99) to test which space a given object locates in? The SFTMap provides a direct mapping between memory address (chunks) to spaces.

Good point. The pattern matching/switch is to figure out which space an object is supposed to be allocated into. However, with SFTMap we can already figure this out without checking the allocation type. In fact, at the call site of is_in_space, we might not know the the allocation type.

@qinsoon
Copy link
Member Author

qinsoon commented Oct 6, 2020

The reason that I am using switch is that the switching pattern is consistent with alloc(), which maps the allocation type to allocators/spaces. To use SFT, we probably need to add a function to SFT to get the space descriptor from an object, then compare that with each spaces in that plan, and reverse-map to the allocation type.

Performance-wise, I don't think there will be a big difference - both require comparing with each space. And both implementations need to be updated with the mutator refactoring work.

@caizixian
Copy link
Member

The reason that I am using switch is that the switching pattern is consistent with alloc(), which maps the allocation type to allocators/spaces. To use SFT, we probably need to add a function to SFT to get the space descriptor from an object, then compare that with each spaces in that plan, and reverse-map to the allocation type.

Performance-wise, I don't think there will be a big difference - both require comparing with each space. And both implementations need to be updated with the mutator refactoring work.

I wasn't too concern about the performance.
I point is that if we use SFT, we don't have to make sure alloc and is_in_space are consistent.
Since we know the address of an object, it can only be in one (or zero) space and we just need to call the corresponding method on the space to check.
In addition, if we want to look up whether an object is in a space, for example during GC, there's no way we can tell whether it's originally allocated using ALLOC_DEFAULT or ALLOC_NONMOVING

@caizixian caizixian closed this Oct 7, 2020
@caizixian caizixian reopened this Oct 7, 2020
@caizixian
Copy link
Member

Accidentally closed the PR. Reopened.

@qinsoon
Copy link
Member Author

qinsoon commented Oct 7, 2020

After a discussion with Steve on this (see #142), I feel we should redo the PR, basically with two changes:

  1. allocation semantics are a set of properties, rather than just one property.
  2. after allocation, space returns the semantics for an object (as suggested in this PR).

I suggest we close this PR or put it on hold, and it will be included in another PR for #142.

@qinsoon qinsoon added PR-under-review Pull request currently in review process S-stale:end: Status: Stale (will be closed soon) and removed PR-ready-for-review Pull request ready for review labels Oct 7, 2020
@qinsoon
Copy link
Member Author

qinsoon commented Jan 25, 2021

Closing this for now. This PR has been inactive for a while.

@qinsoon qinsoon closed this Jan 25, 2021
qinsoon pushed a commit to mmtk/mmtk-openjdk that referenced this pull request Sep 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

PR-under-review Pull request currently in review process S-stale:end: Status: Stale (will be closed soon)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants