Simplified LXR GC - #358
Conversation
This reverts commit 3be9379.
Co-authored-by: Copilot <copilot@github.com>
| MMTkRootsClosure cl(closure); | ||
| MarkingCodeBlobClosure cb_cl(&cl, false, true); | ||
| thread->oops_do(&cl, NULL); | ||
| thread->oops_do(&cl, &cb_cl); |
There was a problem hiding this comment.
This causes duplicate edges assertions to fail in MMTk.
With non-null CodeBlobClosure, oops_do will scan nmethod with this thread. We also cache nmethod with NURSERY/MATURE_CODE_CACHE_ROOTS. This causes duplicated edges.
We could create our own MarkingCodeBlockClosurefrom https://github.com/mmtk/openjdk/blob/b557520f04b35d2a7a040d43f02efc1c4911ebb7/src/hotspot/share/memory/iterator.cpp#L58 -- do other stuff, but do not scan nmethod.
There was a problem hiding this comment.
wenyuzhao#77. Merge when we get most plans work first so we know if the patch introduces any regression.
There was a problem hiding this comment.
This needs to be fixed for correctness. This caused nmethod slots to be traced twice. For the forwarding phase of mark compact, when MMTks trace the nmethod slots for the second time, MMTk sees to-space objects, and does not behave correctly.
There was a problem hiding this comment.
d61c340 added MMTkBookKeepingCodeBlobClosure which does not scan nmethod slots again.
There was a problem hiding this comment.
I think if we want to scan the code blobs as part of object scanning and ensure we don't visit a slot twice, we should use the "claim" mechanism of nmethod. It is like a mark bit for nmethod so that each nmethod is not visited twice during a single transitive closure (or during any given period of time since the bits are cleared).
But if we really want to do it this way, we do it in another PR, unless LXR mandates that. This PR does not do class unloading or weak reference processing.
Don't acquire the `Heap_lock`. The VM companion thread is already holding the lock, so the GC worker thread can't (and doesn't need to) acquire it again. LXR never calls `mmtk_enqueue_references`, so we can remove it for STW plans. Removed the assertion of `has_reference_pending_list`. That function requires the current thread to be either the VM thead or a thread that is holding the `Heap_lock`, but in STW GC, the VM companion thread is holding that lock, and a GC worker thread cannot do the assertion.
| log_debug(gc)("Mutators stopped. Now enumerate threads for scanning..."); | ||
| MMTkHeap::heap()->set_is_gc_active(true); | ||
|
|
||
| oop_storage_set_strong_par_state.init(); |
There was a problem hiding this comment.
I haven't resolved all the issues about markcompact/compressor. But this is one of the issues: we need to re-initiate them in mmtk_prepare_for_roots_re_scanning.
markcompact/compressor
| if (o == NULL) { | ||
| return; | ||
| } | ||
| if (mmtk_is_live((void*) o) == 0) { |
There was a problem hiding this comment.
For mark compact space, is_live is implemented wrong -- it only checks whether the object is marked. So if an object is already forwarded, mmtk does not consider it as 'live' and the slot will be cleared here.
There was a problem hiding this comment.
Fixing is_live is not sufficient here. I ended up not using this MMTkUpdateClosure in mmtk_fix_oop_relocations in d61c340. Things seem to work fine.
mmtk_fix_oop_relocations
|
STW plans mostly passed now. But we saw this error in CI with STW plans on different benchmarks: # A fatal error has been detected by the Java Runtime Environment:
#
# Internal Error (/home/runner/work/mmtk-openjdk/mmtk-openjdk/git/openjdk/src/hotspot/share/runtime/mutex.cpp:81), pid=2931, tid=2956
# assert(!thread->is_active_Java_thread() || _rank <= nosafepoint) failed: This lock should always have a safepoint check for Java threads: JNICritical_lock
#
# JRE version: OpenJDK Runtime Environment (21.0.9) (fastdebug build 21.0.9-internal-adhoc.runner.openjdk)
# Java VM: OpenJDK 64-Bit Server VM (fastdebug 21.0.9-internal-adhoc.runner.openjdk, mixed mode, sharing, tiered, compressed oops, compressed class ptrs, third-party gc, linux-amd64)
# Problematic frame:
# V [libjvm.so+0x149b651] Mutex::check_no_safepoint_state(Thread*) [clone .part.0]+0x21
#
# Core dump will be written. Default location: Core dumps may be processed with "/lib/systemd/systemd-coredump %P %u %g %s %t 9223372036854775808 %h %d" (or dumping to /tmp/runbms-8cotupy4/core.2931)
#
# An error report file with more information is saved as:
# /tmp/runbms-8cotupy4/hs_err_pid2931.log
[16.098s][warning][os] Loading hsdis library failed
#
# If you would like to submit a bug report, please visit:
# https://bugreport.java.com/bugreport/crash.jsp
# |
This bug seems to be fixed with dcc02e1. |
|
For dacapo chopin tests, we see this assertion fail with LXR: MMTK_NO_REFERENCE_TYPES=true MMTK_PLAN=LXR /home/runner/work/mmtk-openjdk/mmtk-openjdk/bundles/jdk/bin/java --add-exports java.base/jdk.internal.ref=ALL-UNNAMED -XX:MetaspaceSize=500M -XX:+DisableExplicitGC -server -XX:+CrashOnOutOfMemoryError -XX:+UseThirdPartyHeap -Xms138M -Xmx138M -jar /home/runner/work/mmtk-openjdk/mmtk-openjdk/dacapo/dacapo-23.11-MR2-chopin.jar -n 1 tomcat -preserve
...
thread '<unnamed>' (3936) panicked at /home/runner/.cargo/git/checkouts/mmtk-core-91cf05d634be0a1e/4f608cc/src/policy/immix/immixspace.rs:1023:13:
assertion `left != right` failed: ERROR Invalid 0x47acd140 rc=0
left: 0
right: 0
note: run with `RUST_BACKTRACE=1` environment variable to display a backtraceOther chopin tests seem to pass. Dacapo 2006 tests are still failing due to OOM. |
…lxr-x/jdk21-simplified
Co-authored-by: mmtkgc-bot <mmtkgc.bot@gmail.com>
We keep the existing API unchanged, while letting the OpenJDK binding use the experimental API for scanning weak and young code cache roots.
4 participants
The mmtk-core PR: mmtk/mmtk-core#1508
Important features that has been removed from the main lxr branch: