Skip to content

Commit

Permalink
Introduce JikesRVM-specific object accessor types (#177)
Browse files Browse the repository at this point in the history
The main purpose of this PR is make a clear distinction between the
`ObjectReference` type in JikesRVM and the `ObjectReference` type in
mmtk-core.

This PR introduced `JikesObj`, a Rust type that represents the
JikesRVM-level `ObjectReference`. It needs an explicit conversion to
convert to/from the MMTk-level `ObjectReference` types.

The interface between mmtk-core and the mmtk-jikesrvm binding is
refactored to do fewer things with the MMTk-level `ObjectReference`.

- Trait methods that pass `ObjectReference` to the binding, notably the
methods in `ObjectModel`, now simply convert the MMTk-level
`ObjectReference` to `JikesObj`, and then call methods of `JikesObj`.
- Concrete methods for accessing object headers, fields, and layout
information are now implemented by `JikesObj` (and other wrapper types
including `TIB` and `RVMType`).
- The `JikesRVMSlot` trait now does the conversion between `JikesObj`
and the MMTk-level `ObjectReference` when loading or storing a slot.

This allows us to change the definition of the MMTk-level
`ObjectReference` in the future, while concrete methods of `JikesObj`
still use offset constants relative to the JikesRVM-level
`ObjectReference` which will not change.

The interface between the Rust part and the Java part of the binding are
refactored to involve `JikesObj` only.

- API functions in `api.rs` accept `JikesObj` parameters from JikesRVM
and return `JikeObj` to JikesRVM where JikesRVM uses the JikesRVM-level
`ObjectReference`.
- We wrap all JTOC calls into strongly-typed Rust functions, and make
the weakly-typed `jtoc_call!` macro private to the wrappers.

In this way, we ensure none of the API functions or JTOC calls leak the
MMTk-level `ObjectReference` values to JikesRVM, or accidentally
interpret a JikesRVM-level `ObjectReference` as an MMTk-level
`ObjectReference`.

We also do some obvious refactoring that makes the code more readable.:

- Encapsulated many field-loading statements in the form of `(addr +
XXXX_OFFSET)::load<T>()` into dedicated methods.
- Encapsulated the code for determining the overhead of hash fields into
a function `JikesObj::hashcode_overhead` and simplified many methods
that depend on that.
-   Renaming "edge" to "slot" in `RustScanThread.java`.

And obvious bug fixes:

- The call to `DO_REFERENCE_PROCESSING_HELPER_SCAN_METHOD_OFFSET` used
to erroneously interpret 0 as `true`. This has been fixed by relying on
the conversion trait.
- `scan_boot_image_sanity` used to declare an immutable array and let
unsafe `jtoc_call!` code modify it. The array is now defined as mutable.

Related issues and PRs:

- This PR is the 1st step of
#178
- It will ultimately allow mmtk/mmtk-core#1170
to be implemented.
  • Loading branch information
wks authored Sep 3, 2024
1 parent c5007a0 commit 7efbc5c
Show file tree
Hide file tree
Showing 14 changed files with 869 additions and 589 deletions.
60 changes: 29 additions & 31 deletions jikesrvm/rvm/src/org/jikesrvm/mm/mminterface/RustScanThread.java
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,8 @@
private GCMapIterator iterator;
@Untraced
private boolean processCodeLocations;
private Address report_edges;
private Address report_edges_extra_data;
private Address reportSlots;
private Address reportSlotsExtraData;
@Untraced
private RVMThread thread;
private Address ip, fp, prevFp, initialIPLoc, topFrame;
Expand All @@ -120,7 +120,7 @@
private boolean failed;
private boolean reinstallReturnBarrier;

private Address edges;
private Address slots;
private Word size = Word.zero();
// The buffer size agreed between the Rust part and the Java part of the binding.
// See the constant SLOTS_BUFFER_CAPACITY in scanning.rs.
Expand All @@ -135,15 +135,15 @@
* Scan a thread, placing the addresses of pointers into supplied buffers.
*
* @param thread The thread to be scanned
* @param report_edges The native call-back function to use for reporting locations.
* @param report_edges_extra_data Extra data passed to the report_edges call-back.
* @param reportSlots The native call-back function to use for reporting locations.
* @param reportSlotsExtraData Extra data passed to the reportSlots call-back.
* @param processCodeLocations Should code locations be processed?
* @param newRootsSufficient Is a partial stack scan sufficient, or must we do a full scan?
*/
@Entrypoint
public static void scanThread(RVMThread thread,
Address report_edges,
Address report_edges_extra_data,
Address reportSlots,
Address reportSlotsExtraData,
boolean processCodeLocations,
boolean newRootsSufficient) {
if (DEFAULT_VERBOSITY >= 1) {
Expand All @@ -158,7 +158,7 @@ public static void scanThread(RVMThread thread,
Address fp = regs.getInnermostFramePointer();
regs.clear();
regs.setInnermost(ip,fp);
scanThread(thread, report_edges, report_edges_extra_data, processCodeLocations, gprs, Address.zero(), newRootsSufficient);
scanThread(thread, reportSlots, reportSlotsExtraData, processCodeLocations, gprs, Address.zero(), newRootsSufficient);
}

/**
Expand All @@ -167,8 +167,8 @@ public static void scanThread(RVMThread thread,
* structure.
*
* @param thread The thread to be scanned
* @param report_edges The native call-back function to use for reporting locations.
* @param report_edges_extra_data Extra data passed to the report_edges call-back.
* @param reportSlots The native call-back function to use for reporting locations.
* @param reportSlotsExtraData Extra data passed to the reportSlots call-back.
* @param processCodeLocations Should code locations be processed?
* @param gprs The general purpose registers associated with the
* stack being scanned (normally extracted from the thread).
Expand All @@ -177,8 +177,8 @@ public static void scanThread(RVMThread thread,
* @param newRootsSufficent Is a partial stack scan sufficient, or must we do a full scan?
*/
private static void scanThread(RVMThread thread,
Address report_edges,
Address report_edges_extra_data,
Address reportSlots,
Address reportSlotsExtraData,
boolean processCodeLocations,
Address gprs,
Address topFrame,
Expand Down Expand Up @@ -211,25 +211,25 @@ private static void scanThread(RVMThread thread,
}

/* scan the stack */
scanner.startScan(report_edges, report_edges_extra_data, processCodeLocations, thread, gprs, ip, fp, initialIPLoc, topFrame, sentinelFp);
scanner.startScan(reportSlots, reportSlotsExtraData, processCodeLocations, thread, gprs, ip, fp, initialIPLoc, topFrame, sentinelFp);
}

@Inline
private void reportEdge(Address edge) {
private void reportSlot(Address slot) {
// Push value
Word cursor = this.size;
this.size = cursor.plus(Word.one());
if (VM.VerifyAssertions) VM._assert(!this.edges.isZero());
this.edges.plus(cursor.toInt() << LOG_BYTES_IN_WORD).store(edge);
if (VM.VerifyAssertions) VM._assert(!this.slots.isZero());
this.slots.plus(cursor.toInt() << LOG_BYTES_IN_WORD).store(slot);
// Flush if full
if (cursor.GE(SLOTS_BUFFER_CAPACITY)) {
flush();
}
}

private void flush() {
if (!edges.isZero() && !size.isZero()) {
edges = sysCall.sysDynamicCall3(report_edges, edges.toWord(), size, report_edges_extra_data.toWord());
if (!slots.isZero() && !size.isZero()) {
slots = sysCall.sysDynamicCall3(reportSlots, slots.toWord(), size, reportSlotsExtraData.toWord());
size = Word.zero();
}
}
Expand All @@ -244,8 +244,8 @@ private void flush() {
* The various state associated with stack scanning is captured by
* instance variables of this type, which are initialized here.
*
* @param report_edges The native call-back function to use for reporting locations.
* @param report_edges_extra_data Extra data passed to the report_edges call-back.
* @param reportSlots The native call-back function to use for reporting locations.
* @param reportSlotsExtraData Extra data passed to the reportSlots call-back.
* @param processCodeLocations whether to process parts of the thread
* that could point to code (e.g. exception registers).
* @param thread Thread for the thread whose stack is being scanned
Expand All @@ -261,16 +261,16 @@ private void flush() {
* if this is to be inferred from the thread (normally the case).
* @param sentinelFp The frame pointer at which the stack scan should stop.
*/
private void startScan(Address report_edges,
Address report_edges_extra_data,
private void startScan(Address reportSlots,
Address reportSlotsExtraData,
boolean processCodeLocations,
RVMThread thread, Address gprs, Address ip,
Address fp, Address initialIPLoc, Address topFrame,
Address sentinelFp) {
this.report_edges = report_edges;
this.report_edges_extra_data = report_edges_extra_data;
this.reportSlots = reportSlots;
this.reportSlotsExtraData = reportSlotsExtraData;
this.size = Word.zero();
this.edges = sysCall.sysDynamicCall3(report_edges, Word.zero(), Word.zero(), report_edges_extra_data.toWord());
this.slots = sysCall.sysDynamicCall3(reportSlots, Word.zero(), Word.zero(), reportSlotsExtraData.toWord());

this.processCodeLocations = processCodeLocations;
this.thread = thread;
Expand All @@ -289,8 +289,8 @@ private void startScan(Address report_edges,
VM.sysFail("Error encountered while scanning stack");
}
flush();
if (!edges.isZero()) {
sysCall.release_buffer(edges);
if (!slots.isZero()) {
sysCall.release_buffer(slots);
}
}

Expand Down Expand Up @@ -395,8 +395,7 @@ private void processCodeLocation(ObjectReference code, Address ipLoc) {
if (!failed) failed = true;
}
}
reportEdge(ipLoc);
// sysCall.sysProcessInteriorEdge(trace, code, ipLoc, true);
reportSlot(ipLoc);
}

/***********************************************************************
Expand Down Expand Up @@ -503,8 +502,7 @@ private void scanFrameForObjects(int verbosity) {
refaddr = iterator.getNextReferenceAddress()) {
if (VALIDATE_REFS) checkReference(refaddr, verbosity);
if (verbosity >= 4) dumpRef(refaddr, verbosity);
reportEdge(refaddr);
// reportDelayedRootEdge(trace, refaddr);
reportSlot(refaddr);
}
}

Expand Down
71 changes: 42 additions & 29 deletions mmtk/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion mmtk/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ log = {version = "0.4", features = ["max_level_trace", "release_max_level_off"]
# - change branch/rev
# - change repo name
# But other changes including adding/removing whitespaces in commented lines may break the CI.
mmtk = { git = "https://github.com/mmtk/mmtk-core.git", rev = "160b7702fccda133c9407234821ad35103623179" }
mmtk = { git = "https://github.com/mmtk/mmtk-core.git", rev = "b3385b85fe1adf96ce820b0ceae1efdcaafa9120" }
# Uncomment the following to build locally - if you change the path locally, do not commit the change in a PR
# mmtk = { path = "../repos/mmtk-core" }

Expand Down
Loading

0 comments on commit 7efbc5c

Please sign in to comment.