Skip to content

Commit

Permalink
Supporting moving immix (#93)
Browse files Browse the repository at this point in the history
This PR adds support for (partially) moving objects in Julia and should
be merged with mmtk/julia#27 and
mmtk/mmtk-core#897.
- It adds support for pinning/unpinning objects and checking if an
object is pinned (the implementation uses a pin bit).
(`mmtk_pin_object`, `mmtk_unpin_object` and `mmtk_is_pinned`)
- It adds support for providing transitively pinned (`tpinned`) roots .
- It implements the `copy` function in `object_model.rs`. Note that
arrays with inlined data must be treated specially, as their `a->data`
pointer needs to be updated after copying.
- It uses Julia's GC bits to store forwarding information and the
object's header to store the forwarding pointer.
- Currently, all stack roots are transitively pinned. Note that we also
need to traverse the `tls->live_tasks` to make sure that any stack root
from these tasks are transitively pinned.
- `scan_julia_object` had to be adapted to cover a few corner cases:
- when an array object contains a pointer to the owner of the data,
`a->data` needs to be updated in case the owner moves.
- the `using` field inside a `jl_module_t` may also be inlined inside
the module, and if that's the case, we need to make sure that field is
updated if the module moves.
- when marking finalizers, traversing the list of malloced arrays, and
the list of live tasks at the end of GC, we need to updated these lists
with objects that have possibly been moved.
- Added a few debug assertions to capture scanning of misaligned objects
and roots.

NB: I've only tested moving immix; sticky immix is still non-moving.

---------

Co-authored-by: Luis Eduardo de Souza Amorim <[email protected]>
Co-authored-by: Luis Eduardo de Souza Amorim <[email protected]>
Co-authored-by: mmtkgc-bot <[email protected]>
(cherry picked from commit 1622162)

# Conflicts:
#	julia/mmtk_julia.c
#	mmtk/Cargo.lock
#	mmtk/Cargo.toml
#	mmtk/api/mmtk.h
#	mmtk/src/julia_scanning.rs
#	mmtk/src/lib.rs
#	mmtk/src/scanning.rs
  • Loading branch information
udesou authored and mergify[bot] committed Feb 12, 2024
1 parent b78afd9 commit 1683e50
Show file tree
Hide file tree
Showing 16 changed files with 440 additions and 63 deletions.
5 changes: 4 additions & 1 deletion .github/scripts/ci-build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ fi
build_type=$1
# plan to use
plan=$2
# moving vs non-moving
is_moving=$3

# helloworld.jl
HELLO_WORLD_JL=$BINDING_PATH/.github/scripts/hello_world.jl
Expand All @@ -23,9 +25,10 @@ if [ "$build_type" == "release" ]; then
fi

plan_feature=${plan,,}
moving_feature=${is_moving,,}

cd $MMTK_JULIA_DIR/mmtk
cargo build --features $plan_feature $build_args
cargo build --features $plan_feature,$moving_feature $build_args

cd $JULIA_PATH

Expand Down
4 changes: 4 additions & 0 deletions .github/scripts/ci-test-patching.sh
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@ declare -a tests_to_skip=(
# This test checks GC logging
'@test occursin("GC: pause", read(tmppath, String))' "$JULIA_PATH/test/misc.jl"

# These tests check for the number of stock GC threads (which we set to 0 with mmtk)
'@test (cpu_threads == 1 ? "1" : string(div(cpu_threads, 2))) ==' "$JULIA_PATH/test/cmdlineargs.jl"
'@test read(`$exename --gcthreads=2 -e $code`, String) == "2"' "$JULIA_PATH/test/cmdlineargs.jl"
'@test read(`$exename -e $code`, String) == "2"' "$JULIA_PATH/test/cmdlineargs.jl"
# This seems to be a regression from upstream when we merge with upstream 43bf2c8.
# The required string int.jl does not appear in the output even if I test with the stock Julia code.
# I do not know what is wrong, but at this point, I dont want to spend time on it.
Expand Down
11 changes: 7 additions & 4 deletions .github/workflows/binding-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ on:
gc_plan:
required: true
type: string
moving:
required: true
type: string

jobs:
build-debug:
Expand All @@ -18,7 +21,7 @@ jobs:
./.github/scripts/ci-setup.sh
- name: Build Julia (Debug)
run: |
./.github/scripts/ci-build.sh debug ${{ inputs.gc_plan }}
./.github/scripts/ci-build.sh debug ${{ inputs.gc_plan }} ${{ inputs.moving }}
- name: Style check
run: |
./.github/scripts/ci-style.sh
Expand All @@ -37,7 +40,7 @@ jobs:
./.github/scripts/ci-test-patching.sh
- name: Build Julia (Release)
run: |
./.github/scripts/ci-build.sh release ${{ inputs.gc_plan }}
./.github/scripts/ci-build.sh release ${{ inputs.gc_plan }} ${{ inputs.moving }}
- name: Test Julia
run: |
./.github/scripts/ci-test-other.sh
Expand All @@ -56,7 +59,7 @@ jobs:
./.github/scripts/ci-test-patching.sh
- name: Build Julia (Release)
run: |
./.github/scripts/ci-build.sh release ${{ inputs.gc_plan }}
./.github/scripts/ci-build.sh release ${{ inputs.gc_plan }} ${{ inputs.moving }}
- name: Test Julia
run: |
./.github/scripts/ci-test-stdlib.sh
Expand All @@ -72,7 +75,7 @@ jobs:
./.github/scripts/ci-setup.sh
- name: Build Julia (Release)
run: |
./.github/scripts/ci-build.sh release ${{ inputs.gc_plan }}
./.github/scripts/ci-build.sh release ${{ inputs.gc_plan }} ${{ inputs.moving }}
- name: Test Julia
run: |
./.github/scripts/ci-test-LinearAlgebra.sh
2 changes: 2 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ jobs:
fail-fast: false
matrix:
gc_plan: [Immix, StickyImmix]
moving: [Default, Non_Moving]
uses: ./.github/workflows/binding-tests.yml
with:
gc_plan: ${{ matrix.gc_plan }}
moving: ${{ matrix.moving }}
83 changes: 82 additions & 1 deletion julia/mmtk_julia.c
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,13 @@ extern const unsigned pool_sizes[];
extern void mmtk_store_obj_size_c(void* obj, size_t size);
extern void jl_gc_free_array(jl_array_t *a);
extern size_t mmtk_get_obj_size(void* obj);
<<<<<<< HEAD
extern void jl_rng_split(uint64_t to[4], uint64_t from[4]);
=======
extern void jl_rng_split(uint64_t to[JL_RNG_SIZE], uint64_t from[JL_RNG_SIZE]);
extern void _jl_free_stack(jl_ptls_t ptls, void *stkbuf, size_t bufsz);
extern void free_stack(void *stkbuf, size_t bufsz);
>>>>>>> 1622162 (Supporting moving immix (#93))
extern jl_mutex_t finalizers_lock;
extern void jl_gc_wait_for_the_world(jl_ptls_t* gc_all_tls_states, int gc_n_threads);
extern void mmtk_block_thread_for_gc(int gc_n_threads);
Expand Down Expand Up @@ -121,6 +127,9 @@ static void mmtk_sweep_malloced_arrays(void) JL_NOTSAFEPOINT
continue;
}
if (mmtk_is_live_object(ma->a)) {
// if the array has been forwarded, the reference needs to be updated
jl_array_t *maybe_forwarded = (jl_array_t*)mmtk_get_possibly_forwared(ma->a);
ma->a = maybe_forwarded;
pma = &ma->next;
}
else {
Expand Down Expand Up @@ -277,6 +286,19 @@ static void add_node_to_roots_buffer(RootsWorkClosure* closure, RootsWorkBuffer*
}
}

static void add_node_to_tpinned_roots_buffer(RootsWorkClosure* closure, RootsWorkBuffer* buf, size_t* buf_len, void* root) {
if (root == NULL)
return;

buf->ptr[*buf_len] = root;
*buf_len += 1;
if (*buf_len >= buf->cap) {
RootsWorkBuffer new_buf = (closure->report_tpinned_nodes_func)(buf->ptr, *buf_len, buf->cap, closure->data, true);
*buf = new_buf;
*buf_len = 0;
}
}

void scan_vm_specific_roots(RootsWorkClosure* closure)
{
// Create a new buf
Expand Down Expand Up @@ -305,10 +327,15 @@ void scan_vm_specific_roots(RootsWorkClosure* closure)
// constants
add_node_to_roots_buffer(closure, &buf, &len, jl_emptytuple_type);
add_node_to_roots_buffer(closure, &buf, &len, cmpswap_names);
add_node_to_roots_buffer(closure, &buf, &len, jl_global_roots_table);

// jl_global_roots_table must be transitively pinned
RootsWorkBuffer tpinned_buf = (closure->report_tpinned_nodes_func)((void**)0, 0, 0, closure->data, true);
size_t tpinned_len = 0;
add_node_to_tpinned_roots_buffer(closure, &tpinned_buf, &tpinned_len, jl_global_roots_table);

// Push the result of the work.
(closure->report_nodes_func)(buf.ptr, len, buf.cap, closure->data, false);
(closure->report_tpinned_nodes_func)(tpinned_buf.ptr, tpinned_len, tpinned_buf.cap, closure->data, false);
}

JL_DLLEXPORT void scan_julia_exc_obj(void* obj_raw, void* closure, ProcessEdgeFn process_edge) {
Expand Down Expand Up @@ -350,9 +377,35 @@ JL_DLLEXPORT void scan_julia_exc_obj(void* obj_raw, void* closure, ProcessEdgeFn
}
}

<<<<<<< HEAD
// number of stacks to always keep available per pool
#define MIN_STACK_MAPPINGS_PER_POOL 5

=======
// number of stacks to always keep available per pool - from gc-stacks.c
#define MIN_STACK_MAPPINGS_PER_POOL 5

// if data is inlined inside the array object --- to->data needs to be updated when copying the array
void update_inlined_array(void* from, void* to) {
jl_value_t* jl_from = (jl_value_t*) from;
jl_value_t* jl_to = (jl_value_t*) to;

uintptr_t tag_to = (uintptr_t)jl_typeof(jl_to);
jl_datatype_t *vt = (jl_datatype_t*)tag_to;

if(vt->name == jl_array_typename) {
jl_array_t *a = (jl_array_t*)jl_from;
jl_array_t *b = (jl_array_t*)jl_to;
if (a->flags.how == 0 && mmtk_object_is_managed_by_mmtk(a->data)) { // a is inlined (a->data is an mmtk object)
size_t offset_of_data = ((size_t)a->data - a->offset*a->elsize) - (size_t)a;
if (offset_of_data > 0 && offset_of_data <= ARRAY_INLINE_NBYTES) {
b->data = (void*)((size_t) b + offset_of_data);
}
}
}
}

>>>>>>> 1622162 (Supporting moving immix (#93))
// modified sweep_stack_pools from gc-stacks.c
void mmtk_sweep_stack_pools(void)
{
Expand All @@ -367,13 +420,21 @@ void mmtk_sweep_stack_pools(void)
// bufsz = t->bufsz
// if (stkbuf)
// push(free_stacks[sz], stkbuf)
<<<<<<< HEAD
assert(gc_n_threads);
for (int i = 0; i < gc_n_threads; i++) {
=======
for (int i = 0; i < jl_n_threads; i++) {
>>>>>>> 1622162 (Supporting moving immix (#93))
jl_ptls_t ptls2 = jl_all_tls_states[i];

// free half of stacks that remain unused since last sweep
for (int p = 0; p < JL_N_STACK_POOLS; p++) {
<<<<<<< HEAD
small_arraylist_t *al = &ptls2->heap.free_stacks[p];
=======
arraylist_t *al = &ptls2->heap.free_stacks[p];
>>>>>>> 1622162 (Supporting moving immix (#93))
size_t n_to_free;
if (al->len > MIN_STACK_MAPPINGS_PER_POOL) {
n_to_free = al->len / 2;
Expand All @@ -384,12 +445,20 @@ void mmtk_sweep_stack_pools(void)
n_to_free = 0;
}
for (int n = 0; n < n_to_free; n++) {
<<<<<<< HEAD
void *stk = small_arraylist_pop(al);
=======
void *stk = arraylist_pop(al);
>>>>>>> 1622162 (Supporting moving immix (#93))
free_stack(stk, pool_sizes[p]);
}
}

<<<<<<< HEAD
small_arraylist_t *live_tasks = &ptls2->heap.live_tasks;
=======
arraylist_t *live_tasks = &ptls2->heap.live_tasks;
>>>>>>> 1622162 (Supporting moving immix (#93))
size_t n = 0;
size_t ndel = 0;
size_t l = live_tasks->len;
Expand All @@ -398,9 +467,17 @@ void mmtk_sweep_stack_pools(void)
continue;
while (1) {
jl_task_t *t = (jl_task_t*)lst[n];
<<<<<<< HEAD
assert(jl_is_task(t));
if (mmtk_is_live_object(t)) {
live_tasks->items[n] = t;
=======
if (mmtk_is_live_object(t)) {
jl_task_t *maybe_forwarded = (jl_task_t*)mmtk_get_possibly_forwared(t);
live_tasks->items[n] = maybe_forwarded;
t = maybe_forwarded;
assert(jl_is_task(t));
>>>>>>> 1622162 (Supporting moving immix (#93))
if (t->stkbuf == NULL)
ndel++; // jl_release_task_stack called
else
Expand Down Expand Up @@ -534,8 +611,11 @@ Julia_Upcalls mmtk_upcalls = (Julia_Upcalls) {
.mmtk_jl_throw_out_of_memory_error = mmtk_jl_throw_out_of_memory_error,
.sweep_malloced_array = mmtk_sweep_malloced_arrays,
.sweep_stack_pools = mmtk_sweep_stack_pools,
<<<<<<< HEAD
.clear_weak_refs = clear_weak_refs,
.sweep_weak_refs = sweep_weak_refs,
=======
>>>>>>> 1622162 (Supporting moving immix (#93))
.wait_in_a_safepoint = mmtk_wait_in_a_safepoint,
.exit_from_safepoint = mmtk_exit_from_safepoint,
.mmtk_jl_hrtime = mmtk_jl_hrtime,
Expand All @@ -547,6 +627,7 @@ Julia_Upcalls mmtk_upcalls = (Julia_Upcalls) {
.arraylist_grow = (void (*)(void*, long unsigned int))arraylist_grow,
.get_jl_gc_have_pending_finalizers = get_jl_gc_have_pending_finalizers,
.scan_vm_specific_roots = scan_vm_specific_roots,
.update_inlined_array = update_inlined_array,
.prepare_to_collect = jl_gc_prepare_to_collect,
.check_is_collection_disabled = check_is_collection_disabled,
};
Loading

0 comments on commit 1683e50

Please sign in to comment.