Conversation
| /// To avoid the envelope start being outside of the block where the object is in, we reserve the leading 'slack' | ||
| /// bytes of each block for the envelope reservation. See `ImmixAllocator::block_usable_start`. | ||
| /// | ||
| /// TODO: Currently both LOS and Immix objects in LXR are using object envelope. But we don't reserve 'slack' for |
There was a problem hiding this comment.
This is no longer true. LOS also reserves some bytes.
wks
left a comment
wks
left a comment
There was a problem hiding this comment.
I think it is good to use helper types to make the logic of RC-related address calculation clear.
I have some suggestions on the implementation of ObjectEnvelope. I think it is easier to let ObjectEnvelope encapsulate ObjectRefrerence instead of the starting address, and it is clearer to let ObjectEnvelope represent a range instead of just the starting address.
And I think ObjectEnvelope deserves its own module because it is not only relevant to RC or LXR, but also tracing-based Immix, too.
I also have some thoughts of opting out the optimization of "using constants and envelope to estimate the object start" if the VM does not have an UPPER_BOUND (in other words, the UPPER_BOUND is infinite) in the case of, for example, the SableVM layout. We should discuss it with Steve tomorrow.
| // The doc comment above links to `crate::util::rc::ObjectEnvelope`. The `util::rc` module is | ||
| // private (not re-exported at the crate root), so that item is unreachable from outside the | ||
| // crate even though it is declared `pub`. `cargo doc --document-private-items` (used by GC | ||
| // implementers and by `ci-doc.sh`) documents it anyway, so the link does resolve there. |
There was a problem hiding this comment.
I think we can move ObjectEnvelop to a dedicated pub(crate) or pub mod, such as util::envelope. It is not only useful for RC, but also useful for tracing-based Immix. In tracing-based Immix, it is still useful for deciding the first line to mark given an object reference.
It could also possibly be used by other places to find the first region an object could possibly overlap with.
There was a problem hiding this comment.
For now we just keep ObjectEnvelope with the rc module -- only rc is using it. If we plan to use ObjectEnvelope in other places, we can move it out of rc.
The document unnecessarily mentions ObjectEnvelope (Claude wrote it). What this constant defines is more general than ObjectEnvelope -- ObjectEnvelope is just the first consumer of this constant. I changed the doc here.
| /// large objects space. | ||
| #[repr(transparent)] | ||
| #[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord)] | ||
| pub struct ObjectEnvelope(Address); |
There was a problem hiding this comment.
It is a bit awkward if ObjectEnvelope only represents the starting address of the envelope. Intuitively it should represent an address range, conceptually including both the start and the end.
I suggest we let ObjectEnvelope wrap the ObjectReference itself, while it still provides query functions to find the envelope start and end addresses. In this case, ObjectEnvelope::start() will just return objref - UPPER_BOUND.
There was a problem hiding this comment.
I changed to ObjectEnvelope(ObjectReference). However, I still kept a few places where we take size as an argument (including end()). LXR tries to avoid redundantly reading object size, and passes the object size around (e.g. promote and promote_with_size). I kept the same convention for ObjectEnvelope.
| /// Every guard that decides whether to mark, unmark, or assert goes through here, so they | ||
| /// cannot disagree: a mark that unmark does not clear is a stray count that outlives its | ||
| /// object. | ||
| pub fn needs_straddle_marks<VM: VMBinding>(size: usize) -> bool { |
There was a problem hiding this comment.
This method is highly specific to the Immix algorithm. I think it is more appropriate to move this method to Line or a dedicated LineMarkTable type. It can call Envelope::slack::<VM>() to get the slack value for the VM.
There was a problem hiding this comment.
I moved it to RefCountHelper instead. I do want to constrain the changes of this PR to RC only at the moment -- we haven't tested if/how this works outside RC.
| fn straddle_line_range(&self, o: ObjectReference, size: usize) -> (Line, Line) { | ||
| let envelope = ObjectEnvelope::of::<VM>(o); | ||
| let start_line = envelope.line().next(); | ||
| let end_line = Line::from_unaligned_address(envelope.end::<VM>(size)); |
There was a problem hiding this comment.
The last overlapping region of an envelope can be computed using Line::from_unaligned_address(envelope.end::<VM>(size) - 1) because envelope.end::<VM>(size) - 1 must be a byte inside the envelope, and the line constructed from Line::from_unaligned_address(...) must contain that byte. This will make the subsequent check if end_line > block_end_line redundant (but we can assert that just to be safe).
We can implement this as Envelope::last_overlapping_region::<VM, R>() for convenience. I think this can be generalized to all non-empty address ranges, but we currently don't have a dedicated "address range" type.
| self.bump_pointer.cursor = start_line | ||
| .start() | ||
| .max(self.block_usable_start(line.block())); |
There was a problem hiding this comment.
I suggested skipping the first slack() bytes of each block so that the envelope never crosses block boundaries. But after a second thought, we may think of alternatives because
- If the
slack()is large, we may end up wasting too much memory. - The upper bound may be infinite, such as the SableVM layout.
In the latter case, we may give MMTk core an option to opt out the optimization of using constants to estimate the object start, but instead always use ObjectModel::ref_to_object_start. One way to do it is to encapsulate related computation to a dedicated type, such as LineMarkTable or LineRCTable or others. Operations include:
- Find the first line to mark.
- Find the address to store the RC.
- How many bytes to skip in the beginning of each Block?
- Should we skip the first line of each hole when allocating?
- Should we skip the last line occupied by an object when marking lines?
MMTk core may provide different strategies depending on whether the slack is finite and whether the VM is able to identify small objects (envelope < Line::BYTES) at all.
We should discuss this in tomorrow's meeting and ask Steve for his opinion.
|
Wrapping around an
On the contrary, I was considering naming |
Well, that's still OK. We can make it specific to LXR, and later generalize it if we find appropriate. |
|
mmtk/mmtk-openjdk#378 is testing this PR. |
|
Using |
This branch has not been deployed
2 participants
No description provided.