Introduce JikesRVM-specific object accessor types #177
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Change variable names for CopySemantics to `semantics` Not use the variable name `region` for the result of `alloc` because it is not a region (power-of-two sized/aligned range). Annotate unused functions.
This ensures we don't accidentally leak MMTk-level ObjectReference values to JikesRVM.
Add logs to methods of `ObjectModel<VM>` so that we know which callbacks mmtk-core calls in a given configuration. Remove other logs.
This will ensure we don't accidentally use the weakly-typed macros in other modules except `jikesrvm_calls` itself.
|
The other PR #179 demonstrates that we can modify MMTk-level ObjectReference to point to JavaHeader and it works. I have marked this PR as ready for reviewing. |
qinsoon
approved these changes
Sep 2, 2024
mmtk/src/api.rs
Outdated
| bytes: usize, | ||
| allocator: AllocationSemantics, | ||
| ) { | ||
| memory_manager::post_alloc::<JikesRVM>(unsafe { &mut *mutator }, refer, bytes, allocator) | ||
| debug_assert!(!refer.is_null()); |
There was a problem hiding this comment.
The null check seems unnecessary. There is always try_into/from afterwards, which will fail if you have a null reference.
There was a problem hiding this comment.
I removed all debug_assert!(!jikes_obj.is_null()); from API functions if the JikesObj parameter is converted to ObjectReference using try_from or try_into.
`ObjectReference::try_from(jikes_obj)` already performs null check, so we don't need an additional debug_assert for that.
|
I can't use moma machines for now. I ran lusearch in DaCapo 2006 locally, with RFastAdaptiveSemiSpace, and see no obvious performance difference. |
wks
added a commit
that referenced
this pull request
Sep 3, 2024
This PR changes the definition of MMTk-level `ObjectReference` for the JikesRVM binding so that it now points to the JavaHeader, and is different from the JikesRVM-level `ObjectReference` (a.k.a. `JikesObj`). This will guarantee that the MMTk-level ObjectReference is always inside an object. Note that this PR does not involve a change in mmtk-core. It changes `ObjectModel::IN_OBJECT_ADDRESS_OFFSET` to 0 so that the "in-object address" is identical to the raw address of `ObjectReference`. It demonstrates the JikesRVM binding can work well with MMTk-level `ObjectReference` being different from JikesRVM-level `ObjectReference`. Related issues and PRs. - This PR is based on #177 - This PR is the 2nd step of #178 - It will ultimately allow mmtk/mmtk-core#1170 to be implemented.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The main purpose of this PR is make a clear distinction between the
ObjectReferencetype in JikesRVM and theObjectReferencetype in mmtk-core.This PR introduced
JikesObj, a Rust type that represents the JikesRVM-levelObjectReference. It needs an explicit conversion to convert to/from the MMTk-levelObjectReferencetypes.The interface between mmtk-core and the mmtk-jikesrvm binding is refactored to do fewer things with the MMTk-level
ObjectReference.ObjectReferenceto the binding, notably the methods inObjectModel, now simply convert the MMTk-levelObjectReferencetoJikesObj, and then call methods ofJikesObj.JikesObj(and other wrapper types includingTIBandRVMType).JikesRVMSlottrait now does the conversion betweenJikesObjand the MMTk-levelObjectReferencewhen loading or storing a slot.This allows us to change the definition of the MMTk-level
ObjectReferencein the future, while concrete methods ofJikesObjstill use offset constants relative to the JikesRVM-levelObjectReferencewhich will not change.The interface between the Rust part and the Java part of the binding are refactored to involve
JikesObjonly.api.rsacceptJikesObjparameters from JikesRVM and returnJikeObjto JikesRVM where JikesRVM uses the JikesRVM-levelObjectReference.jtoc_call!macro private to the wrappers.In this way, we ensure none of the API functions or JTOC calls leak the MMTk-level
ObjectReferencevalues to JikesRVM, or accidentally interpret a JikesRVM-levelObjectReferenceas an MMTk-levelObjectReference.We also do some obvious refactoring that makes the code more readable.:
(addr + XXXX_OFFSET)::load<T>()into dedicated methods.JikesObj::hashcode_overheadand simplified many methods that depend on that.RustScanThread.java.And obvious bug fixes:
DO_REFERENCE_PROCESSING_HELPER_SCAN_METHOD_OFFSETused to erroneously interpret 0 astrue. This has been fixed by relying on the conversion trait.scan_boot_image_sanityused to declare an immutable array and let unsafejtoc_call!code modify it. The array is now defined as mutable.Related issues and PRs: