Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 14 additions & 2 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 @@ -30,7 +30,7 @@ once_cell = "1.10.0"
# - change branch
# - 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 = "df146b7af6cf41cc7d6996e1ca538fd2b32950f5" }
mmtk = { git = "https://github.com/ArberSephirotheca/mmtk-core.git", rev = "1bd9b39053f5596204c196746f50a2dd51128c7c" }
# Uncomment the following to build locally
# mmtk = { path = "../repos/mmtk-core" }

Expand Down
37 changes: 37 additions & 0 deletions mmtk/src/abi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -404,6 +404,43 @@ pub fn validate_memory_layouts() {
^ mem::size_of::<InstanceClassLoaderKlass>()
^ mem::size_of::<TypeArrayKlass>()
^ mem::size_of::<ObjArrayKlass>()
^ mem::size_of::<JavaThreadIteratorWithHandle>()
};
assert_eq!(vm_checksum, binding_checksum);
}

// hotspot/share/runtime/threadSMR.hpp
#[repr(C)]
pub struct SafeThreadsListPtr {
_previous: *mut libc::c_void,
_thread: *mut libc::c_void,
_list: *mut libc::c_void,
_has_ref_count: bool,
_needs_release: bool,
}

#[repr(C)]
pub struct ElapsedTimer {
_counter: i64,
_start_counter: i64,
_active: bool,
}

#[repr(C)]
pub struct ThreadsListHandle {
// This is speculative:
// SafeThreadsListPtr is 32 bytes, and ElapsedTimer is 24 bytes.
// But ThreadsListHandle in C++ has a size of 64 bytes in debug builds, but 56 bytes in release builds.
// I am guessing the compiler includes a vtable pointer in debug builds, and optimize it away in release builds.
#[cfg(debug_assertions)]
vtable: *mut libc::c_void,
_list_ptr: SafeThreadsListPtr,
_timer: ElapsedTimer,
}

#[repr(C)]
pub struct JavaThreadIteratorWithHandle {
vtable: *mut libc::c_void,
_tlh: ThreadsListHandle,
_index: u32,
}
58 changes: 37 additions & 21 deletions mmtk/src/active_plan.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,45 @@
use crate::abi::JavaThreadIteratorWithHandle;
use crate::OpenJDK;
use crate::SINGLETON;
use crate::UPCALLS;
use mmtk::util::opaque_pointer::*;
use mmtk::vm::ActivePlan;
use mmtk::Mutator;
use mmtk::Plan;
use std::sync::Mutex;
use std::marker::PhantomData;
use std::mem::MaybeUninit;

struct OpenJDKMutatorIterator<'a> {
handle: MaybeUninit<JavaThreadIteratorWithHandle>,
_p: PhantomData<&'a ()>,
}

impl<'a> OpenJDKMutatorIterator<'a> {
fn new() -> Self {
let mut iter = Self {
handle: MaybeUninit::uninit(),
_p: PhantomData,
};
// Create JavaThreadIteratorWithHandle
unsafe {
((*UPCALLS).new_java_thread_iterator)(iter.handle.as_mut_ptr());
}
iter
}
}

impl<'a> Iterator for OpenJDKMutatorIterator<'a> {
type Item = &'a mut Mutator<OpenJDK>;

fn next(&mut self) -> Option<Self::Item> {
let next = unsafe { ((*UPCALLS).java_thread_iterator_next)(self.handle.as_mut_ptr()) };
if next.is_null() {
None
} else {
Some(unsafe { &mut *next })
}
}
}

pub struct VMActivePlan {}

Expand All @@ -25,29 +59,11 @@ impl ActivePlan<OpenJDK> for VMActivePlan {
}
}

fn reset_mutator_iterator() {
unsafe {
((*UPCALLS).reset_mutator_iterator)();
}
}

fn get_next_mutator() -> Option<&'static mut Mutator<OpenJDK>> {
let _guard = MUTATOR_ITERATOR_LOCK.lock().unwrap();
unsafe {
let m = ((*UPCALLS).get_next_mutator)();
if m.is_null() {
None
} else {
Some(&mut *m)
}
}
fn mutators<'a>() -> Box<dyn Iterator<Item = &'a mut Mutator<OpenJDK>> + 'a> {
Box::new(OpenJDKMutatorIterator::new())
}

fn number_of_mutators() -> usize {
unsafe { ((*UPCALLS).number_of_mutators)() }
}
}

lazy_static! {
pub static ref MUTATOR_ITERATOR_LOCK: Mutex<()> = Mutex::new(());
}
5 changes: 3 additions & 2 deletions mmtk/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,6 @@ pub struct OpenJDK_Upcalls {
pub spawn_gc_thread: extern "C" fn(tls: VMThread, kind: libc::c_int, ctx: *mut libc::c_void),
pub block_for_gc: extern "C" fn(),
pub out_of_memory: extern "C" fn(tls: VMThread, err_kind: AllocationError),
pub get_next_mutator: extern "C" fn() -> *mut Mutator<OpenJDK>,
pub reset_mutator_iterator: extern "C" fn(),
pub scan_object: extern "C" fn(trace: *mut c_void, object: ObjectReference, tls: OpaquePointer),
pub dump_object: extern "C" fn(object: ObjectReference),
pub get_object_size: extern "C" fn(object: ObjectReference) -> usize,
Expand Down Expand Up @@ -95,6 +93,9 @@ pub struct OpenJDK_Upcalls {
pub schedule_finalizer: extern "C" fn(),
pub prepare_for_roots_re_scanning: extern "C" fn(),
pub enqueue_references: extern "C" fn(objects: *const ObjectReference, len: usize),
pub new_java_thread_iterator: extern "C" fn(iter: *mut abi::JavaThreadIteratorWithHandle),
pub java_thread_iterator_next:
extern "C" fn(iter: *mut abi::JavaThreadIteratorWithHandle) -> *mut Mutator<OpenJDK>,
Comment on lines +96 to +98
Copy link
Contributor

Choose a reason for hiding this comment

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

There is one missing upcall: the destructor. JavaThreadIteratorWithHandle has a destructor which should be called when the iterator is dropped.

}

pub static mut UPCALLS: *const OpenJDK_Upcalls = null_mut();
Expand Down
4 changes: 2 additions & 2 deletions openjdk/mmtk.h
Original file line number Diff line number Diff line change
Expand Up @@ -149,8 +149,6 @@ typedef struct {
void (*spawn_gc_thread) (void *tls, int kind, void *ctx);
void (*block_for_gc) ();
void (*out_of_memory) (void *tls, MMTkAllocationError err_kind);
void* (*get_next_mutator) ();
void (*reset_mutator_iterator) ();
void (*scan_object) (void* trace, void* object, void* tls);
void (*dump_object) (void* object);
size_t (*get_object_size) (void* object);
Expand Down Expand Up @@ -182,6 +180,8 @@ typedef struct {
void (*schedule_finalizer)();
void (*prepare_for_roots_re_scanning)();
void (*enqueue_references)(void** objects, size_t len);
void (*new_java_thread_iterator)(void*);
void* (*java_thread_iterator_next)(void*);
} OpenJDK_Upcalls;

extern void openjdk_gc_init(OpenJDK_Upcalls *calls);
Expand Down
28 changes: 10 additions & 18 deletions openjdk/mmtkUpcalls.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -190,27 +190,18 @@ struct MaybeUninit {
char _data[sizeof(T)];
};

static MaybeUninit<JavaThreadIteratorWithHandle> jtiwh;
static bool mutator_iteration_start = true;
static void mmtk_new_java_thread_iterator(void* iter) {
*(JavaThreadIteratorWithHandle*)iter = JavaThreadIteratorWithHandle();
Copy link
Contributor

Choose a reason for hiding this comment

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

This step moves the constructed JavaThreadIteratorWithHandle instance to another location.

However, part of the race-prevention algorithm in OpenJDK includes letting each Thread hold a pointer to the SafeThreadsListPtr instance inside JavaThreadIteratorWithHandle. If the SafeThreadsListPtr is moved, the field in Thread becomes a dangling pointer.

See

}

static void* mmtk_get_next_mutator() {
if (mutator_iteration_start) {
*jtiwh = JavaThreadIteratorWithHandle();
mutator_iteration_start = false;
}
JavaThread *thr = jtiwh->next();
static void* mmtk_java_thread_iterator_next(void* iter) {
JavaThread *thr = ((JavaThreadIteratorWithHandle*)iter)->next();
if (thr == NULL) {
mutator_iteration_start = true;
return NULL;
}
return (void*) &thr->third_party_heap_mutator;
}

static void mmtk_reset_mutator_iterator() {
mutator_iteration_start = true;
}


static void mmtk_scan_all_thread_roots(EdgesClosure closure) {
MMTkRootsClosure2 cl(closure);
MMTkHeap::heap()->scan_thread_roots(cl);
Expand Down Expand Up @@ -276,7 +267,8 @@ static size_t compute_klass_mem_layout_checksum() {
^ sizeof(InstanceMirrorKlass)
^ sizeof(InstanceClassLoaderKlass)
^ sizeof(TypeArrayKlass)
^ sizeof(ObjArrayKlass);
^ sizeof(ObjArrayKlass)
^ sizeof(JavaThreadIteratorWithHandle);
}

static int referent_offset() {
Expand Down Expand Up @@ -347,8 +339,6 @@ OpenJDK_Upcalls mmtk_upcalls = {
mmtk_spawn_gc_thread,
mmtk_block_for_gc,
mmtk_out_of_memory,
mmtk_get_next_mutator,
mmtk_reset_mutator_iterator,
mmtk_scan_object,
mmtk_dump_object,
mmtk_get_object_size,
Expand Down Expand Up @@ -379,5 +369,7 @@ OpenJDK_Upcalls mmtk_upcalls = {
mmtk_number_of_mutators,
mmtk_schedule_finalizer,
mmtk_prepare_for_roots_re_scanning,
mmtk_enqueue_references
mmtk_enqueue_references,
mmtk_new_java_thread_iterator,
mmtk_java_thread_iterator_next,
};