Skip to content

Commit bf55259

Browse files
qinsoonudesou
authored andcommitted
Trace pinned objects as roots (mmtk#91)
Following the suggestion in mmtk#89 (comment), trace recently added pinned objects as roots. When we pass those objects as 'nodes' to MMTk, MMTk does not know the slots so MMTk cannot update the reference, thus MMTk will not move those objects. This is essentially the same as pinning those objects before a GC, and unpinning after a GC.
1 parent b378651 commit bf55259

File tree

5 files changed

+33
-49
lines changed

5 files changed

+33
-49
lines changed

src/ast.c

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -134,18 +134,19 @@ typedef struct _jl_ast_context_t {
134134
value_t ssavalue_sym;
135135
value_t slot_sym;
136136
jl_module_t *module; // context module for `current-julia-module-counter`
137-
arraylist_t pinned_objects;
137+
// These are essentially roots for ast context.
138+
arraylist_t ast_roots;
138139
} jl_ast_context_t;
139140

140141
// FIXME: Ugly hack to get a pointer to the pinned objects
141-
arraylist_t *extract_pinned_objects_from_ast_ctx(void *ctx)
142+
arraylist_t *extract_ast_roots_from_ast_ctx(void *ctx)
142143
{
143144
// This is used to extract pinned objects from the context
144145
// for the purpose of pinning them in MMTk.
145146
if (ctx == NULL)
146147
return NULL;
147148
jl_ast_context_t *jl_ctx = (jl_ast_context_t*)ctx;
148-
return &jl_ctx->pinned_objects;
149+
return &jl_ctx->ast_roots;
149150
}
150151

151152
static jl_ast_context_t jl_ast_main_ctx;
@@ -288,7 +289,7 @@ static void jl_init_ast_ctx(jl_ast_context_t *ctx) JL_NOTSAFEPOINT
288289
ctx->slot_sym = symbol(fl_ctx, "slot");
289290
ctx->module = NULL;
290291
set(symbol(fl_ctx, "*scopewarn-opt*"), fixnum(jl_options.warn_scope));
291-
arraylist_new(&ctx->pinned_objects, 0);
292+
arraylist_new(&ctx->ast_roots, 0);
292293
}
293294

294295
// There should be no GC allocation while holding this lock
@@ -319,7 +320,7 @@ static void jl_ast_ctx_leave(jl_ast_context_t *ctx)
319320
{
320321
uv_mutex_lock(&flisp_lock);
321322
ctx->module = NULL;
322-
ctx->pinned_objects.len = 0; // clear pinned objects
323+
ctx->ast_roots.len = 0; // clear root objects
323324
arraylist_pop(&jl_ast_ctx_used);
324325
arraylist_push(&jl_ast_ctx_freed, ctx);
325326
uv_mutex_unlock(&flisp_lock);
@@ -798,7 +799,7 @@ static value_t julia_to_scm_(jl_ast_context_t *ctx, jl_value_t *v, int check_val
798799
{
799800
// The following code will take internal pointers to v's fields. We need to make sure
800801
// that v will not be moved by GC.
801-
arraylist_push(&ctx->pinned_objects, v);
802+
arraylist_push(&ctx->ast_roots, v);
802803
value_t retval;
803804
fl_context_t *fl_ctx = &ctx->fl;
804805
if (julia_to_scm_noalloc1(fl_ctx, v, &retval))

src/engine.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -64,15 +64,15 @@ jl_code_instance_t *jl_engine_reserve(jl_method_instance_t *m, jl_value_t *owner
6464
auto tid = jl_atomic_load_relaxed(&ct->tid);
6565
if (([tid, m, owner, ci] () -> bool { // necessary scope block / lambda for unique_lock
6666
jl_unique_gcsafe_lock lock(engine_lock);
67-
arraylist_push(&gc_pinned_objects, owner);
67+
arraylist_push(&extra_gc_roots, owner);
6868
InferKey key{m, owner};
6969
if ((signed)Awaiting.size() < tid + 1)
7070
Awaiting.resize(tid + 1);
7171
while (1) {
7272
auto record = Reservations.find(key);
7373
if (record == Reservations.end()) {
7474
Reservations[key] = ReservationInfo{tid, ci};
75-
arraylist_pop(&gc_pinned_objects);
75+
arraylist_pop(&extra_gc_roots);
7676
return false;
7777
}
7878
// before waiting, need to run deadlock/cycle detection
@@ -81,7 +81,7 @@ jl_code_instance_t *jl_engine_reserve(jl_method_instance_t *m, jl_value_t *owner
8181
auto wait_tid = record->second.tid;
8282
while (1) {
8383
if (wait_tid == tid) {
84-
arraylist_pop(&gc_pinned_objects);
84+
arraylist_pop(&extra_gc_roots);
8585
return true;
8686
}
8787
if ((signed)Awaiting.size() <= wait_tid)
@@ -99,7 +99,7 @@ jl_code_instance_t *jl_engine_reserve(jl_method_instance_t *m, jl_value_t *owner
9999
lock.wait(engine_wait);
100100
Awaiting[tid] = InferKey{};
101101
}
102-
arraylist_pop(&gc_pinned_objects);
102+
arraylist_pop(&extra_gc_roots);
103103
})())
104104
ct->ptls->engine_nqueued--;
105105
JL_GC_POP();

src/gc-common.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -687,7 +687,7 @@ JL_DLLEXPORT int jl_gc_enable(int on)
687687
// MISC
688688
// =========================================================================== //
689689

690-
arraylist_t gc_pinned_objects;
690+
arraylist_t extra_gc_roots;
691691

692692
JL_DLLEXPORT jl_weakref_t *jl_gc_new_weakref(jl_value_t *value)
693693
{

src/gc-mmtk.c

Lines changed: 17 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ void jl_gc_init(void) {
7878

7979
jl_set_check_alive_type(mmtk_is_reachable_object);
8080

81-
arraylist_new(&gc_pinned_objects, 0);
81+
arraylist_new(&extra_gc_roots, 0);
8282
arraylist_new(&to_finalize, 0);
8383
arraylist_new(&finalizer_list_marked, 0);
8484
gc_num.interval = default_collect_interval;
@@ -258,36 +258,6 @@ JL_DLLEXPORT void jl_gc_collect(jl_gc_collection_t collection) {
258258
// print_fragmentation();
259259
}
260260

261-
void gc_pin_objects_from_compiler_frontend(arraylist_t *objects_pinned_by_call)
262-
{
263-
for (size_t i = 0; i < gc_pinned_objects.len; i++) {
264-
void *obj = gc_pinned_objects.items[i];
265-
unsigned char got_pinned = mmtk_pin_object(obj);
266-
if (got_pinned) {
267-
arraylist_push(objects_pinned_by_call, obj);
268-
}
269-
}
270-
for (size_t i = 0; i < jl_ast_ctx_used.len; i++) {
271-
void *ctx = jl_ast_ctx_used.items[i];
272-
arraylist_t *pinned_objects = extract_pinned_objects_from_ast_ctx(ctx);
273-
for (size_t j = 0; j < pinned_objects->len; j++) {
274-
void *obj = pinned_objects->items[j];
275-
unsigned char got_pinned = mmtk_pin_object(obj);
276-
if (got_pinned) {
277-
arraylist_push(objects_pinned_by_call, obj);
278-
}
279-
}
280-
}
281-
}
282-
283-
void gc_unpin_objects_from_compiler_frontend(arraylist_t *objects_pinned_by_call)
284-
{
285-
for (size_t i = 0; i < objects_pinned_by_call->len; i++) {
286-
void *obj = objects_pinned_by_call->items[i];
287-
mmtk_unpin_object(obj);
288-
}
289-
}
290-
291261
// Based on jl_gc_collect from gc-stock.c
292262
// called when stopping the thread in `mmtk_block_for_gc`
293263
JL_DLLEXPORT void jl_gc_prepare_to_collect(void)
@@ -350,12 +320,7 @@ JL_DLLEXPORT void jl_gc_prepare_to_collect(void)
350320
jl_gc_notify_thread_yield(ptls, NULL);
351321
JL_LOCK_NOGC(&finalizers_lock); // all the other threads are stopped, so this does not make sense, right? otherwise, failing that, this seems like plausibly a deadlock
352322
#ifndef __clang_gcanalyzer__
353-
arraylist_t objects_pinned_by_call;
354-
arraylist_new(&objects_pinned_by_call, 0);
355-
gc_pin_objects_from_compiler_frontend(&objects_pinned_by_call);
356323
mmtk_block_thread_for_gc();
357-
gc_unpin_objects_from_compiler_frontend(&objects_pinned_by_call);
358-
arraylist_free(&objects_pinned_by_call);
359324
#endif
360325
JL_UNLOCK_NOGC(&finalizers_lock);
361326
}
@@ -842,6 +807,22 @@ JL_DLLEXPORT void jl_gc_scan_vm_specific_roots(RootsWorkClosure* closure)
842807
}
843808
}
844809

810+
// Trace objects in extra_gc_roots
811+
for (size_t i = 0; i < extra_gc_roots.len; i++) {
812+
void* obj = extra_gc_roots.items[i];
813+
add_node_to_roots_buffer(closure, &buf, &len, obj);
814+
}
815+
816+
// Trace objects in jl_ast_ctx_used
817+
for (size_t i = 0; i < jl_ast_ctx_used.len; i++) {
818+
void *ctx = jl_ast_ctx_used.items[i];
819+
arraylist_t *ast_roots = extract_ast_roots_from_ast_ctx(ctx);
820+
for (size_t j = 0; j < ast_roots->len; j++) {
821+
void *obj = ast_roots->items[j];
822+
add_node_to_roots_buffer(closure, &buf, &len, obj);
823+
}
824+
}
825+
845826
// // add module
846827
// add_node_to_roots_buffer(closure, &buf, &len, jl_main_module);
847828

src/julia.h

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1377,7 +1377,9 @@ JL_DLLEXPORT JL_CONST_FUNC jl_gcframe_t **(jl_get_pgcstack)(void) JL_GLOBALLY_RO
13771377

13781378
// object pinning ------------------------------------------------------------
13791379

1380-
extern arraylist_t gc_pinned_objects;
1380+
// These 'new roots' are added for moving GCs.
1381+
// We could consider merging this list with global roots list if we can push and pop from global roots list in the same way.
1382+
extern arraylist_t extra_gc_roots;
13811383
typedef bool (*check_alive_fn_type)(void *);
13821384
JL_DLLEXPORT void jl_set_check_alive_type(check_alive_fn_type fn);
13831385
JL_DLLEXPORT void jl_log_pinning_event(void *pinned_object, const char *filename, int lineno);
@@ -2460,7 +2462,7 @@ JL_DLLEXPORT void jl_register_newmeth_tracer(void (*callback)(jl_method_t *trace
24602462

24612463
// AST access
24622464
JL_DLLEXPORT jl_value_t *jl_copy_ast(jl_value_t *expr JL_MAYBE_UNROOTED);
2463-
arraylist_t *extract_pinned_objects_from_ast_ctx(void *ctx);
2465+
arraylist_t *extract_ast_roots_from_ast_ctx(void *ctx);
24642466
extern arraylist_t jl_ast_ctx_used;
24652467

24662468
// IR representation

0 commit comments

Comments
 (0)