[前][次][番号順一覧][スレッド一覧]

ruby-changes:60347

From: Takashi <ko1@a...>
Date: Tue, 10 Mar 2020 17:00:24 +0900 (JST)
Subject: [ruby-changes:60347] 4bcd5981e8 (master): Capture inlined iseq's cc entries in root iseq's

https://git.ruby-lang.org/ruby.git/commit/?id=4bcd5981e8

From 4bcd5981e80d3e1852c8723741a0069779464128 Mon Sep 17 00:00:00 2001
From: Takashi Kokubun <takashikkbn@g...>
Date: Tue, 10 Mar 2020 00:19:19 -0700
Subject: Capture inlined iseq's cc entries in root iseq's

jit_unit to avoid marking wrong cc entries when inlined iseq is compiled
multiple times, resolving the TODO added by daf7c48d88.

This obviates pseudo jit_unit in inlined iseq introduced by 7ec2359374
and fixes memory leak of the adhoc unit.

diff --git a/mjit.c b/mjit.c
index a546141..514d04f 100644
--- a/mjit.c
+++ b/mjit.c
@@ -26,7 +26,6 @@ https://github.com/ruby/ruby/blob/trunk/mjit.c#L26
 #include "internal/warnings.h"
 
 #include "mjit_worker.c"
-#include "vm_callinfo.h"
 
 // Copy ISeq's states so that race condition does not happen on compilation.
 static void
@@ -53,18 +52,6 @@ mjit_copy_job_handler(void *data) https://github.com/ruby/ruby/blob/trunk/mjit.c#L52
     }
 
     const struct rb_iseq_constant_body *body = job->iseq->body;
-    const unsigned int ci_size = body->ci_size;
-    if (ci_size > 0) {
-        VM_ASSERT(body->jit_unit != NULL);
-        VM_ASSERT(body->jit_unit->cc_entries != NULL);
-
-        const struct rb_callcache **cc_entries = body->jit_unit->cc_entries;
-
-        for (unsigned int i=0; i<ci_size; i++) {
-            cc_entries[i] = body->call_data[i].cc;
-        }
-    }
-
     if (job->is_entries) {
         memcpy(job->is_entries, body->is_entries, sizeof(union iseq_inline_storage_entry) * body->is_size);
     }
@@ -293,9 +280,6 @@ create_unit(const rb_iseq_t *iseq) https://github.com/ruby/ruby/blob/trunk/mjit.c#L280
 
     unit->id = current_unit_num++;
     unit->iseq = (rb_iseq_t *)iseq;
-    if (iseq->body->ci_size > 0) {
-        unit->cc_entries = ZALLOC_N(const struct rb_callcache *, iseq->body->ci_size);
-    }
     iseq->body->jit_unit = unit;
 }
 
diff --git a/mjit_compile.c b/mjit_compile.c
index c0e5b18..a75e3fe 100644
--- a/mjit_compile.c
+++ b/mjit_compile.c
@@ -32,12 +32,6 @@ https://github.com/ruby/ruby/blob/trunk/mjit_compile.c#L32
 #define NOT_COMPILED_STACK_SIZE -1
 #define ALREADY_COMPILED_P(status, pos) (status->stack_size_for_pos[pos] != NOT_COMPILED_STACK_SIZE)
 
-static size_t
-call_data_index(CALL_DATA cd, const struct rb_iseq_constant_body *body)
-{
-    return cd - body->call_data;
-}
-
 // For propagating information needed for lazily pushing a frame.
 struct inlined_call_context {
     int orig_argc; // ci->orig_argc
@@ -55,8 +49,12 @@ struct compile_status { https://github.com/ruby/ruby/blob/trunk/mjit_compile.c#L49
     // If true, JIT-ed code will use local variables to store pushed values instead of
     // using VM's stack and moving stack pointer.
     bool local_stack_p;
-    // Safely-accessible cache entries copied from main thread.
+    // Safely-accessible ivar cache entries copied from main thread.
     union iseq_inline_storage_entry *is_entries;
+    // Safely-accessible call cache entries captured to compiled_iseq to be marked on GC
+    const struct rb_callcache **cc_entries;
+    // A pointer to root (i.e. not inlined) iseq being compiled.
+    const struct rb_iseq_constant_body *compiled_iseq;
     // Mutated optimization levels
     struct rb_mjit_compile_info *compile_info;
     // If `inlined_iseqs[pos]` is not NULL, `mjit_compile_body` tries to inline ISeq there.
@@ -78,6 +76,12 @@ struct case_dispatch_var { https://github.com/ruby/ruby/blob/trunk/mjit_compile.c#L76
     VALUE last_value;
 };
 
+static size_t
+call_data_index(CALL_DATA cd, const struct rb_iseq_constant_body *body)
+{
+    return cd - body->call_data;
+}
+
 // Returns true if call cache is still not obsoleted and vm_cc_cme(cc)->def->type is available.
 static bool
 has_valid_method_type(CALL_CACHE cc)
@@ -273,6 +277,9 @@ compile_cancel_handler(FILE *f, const struct rb_iseq_constant_body *body, struct https://github.com/ruby/ruby/blob/trunk/mjit_compile.c#L277
     fprintf(f, "    return Qundef;\n");
 }
 
+extern const struct rb_callcache **
+mjit_capture_cc_entries(const struct rb_iseq_constant_body *compiled_iseq, const struct rb_iseq_constant_body *captured_iseq);
+
 extern bool mjit_copy_cache_from_main_thread(const rb_iseq_t *iseq,
                                              union iseq_inline_storage_entry *is_entries);
 
@@ -368,6 +375,9 @@ inlinable_iseq_p(const struct rb_iseq_constant_body *body) https://github.com/ruby/ruby/blob/trunk/mjit_compile.c#L375
             alloca(sizeof(const struct rb_iseq_constant_body *) * body->iseq_size) : NULL, \
         .is_entries = (body->is_size > 0) ? \
             alloca(sizeof(union iseq_inline_storage_entry) * body->is_size) : NULL, \
+        .cc_entries = (body->ci_size > 0) ? \
+            mjit_capture_cc_entries(status.compiled_iseq, body) : NULL, \
+        .compiled_iseq = status.compiled_iseq, \
         .compile_info = compile_root_p ? \
             rb_mjit_iseq_compile_info(body) : alloca(sizeof(struct rb_mjit_compile_info)) \
     }; \
@@ -393,7 +403,7 @@ precompile_inlinable_iseqs(FILE *f, const rb_iseq_t *iseq, struct compile_status https://github.com/ruby/ruby/blob/trunk/mjit_compile.c#L403
         if (insn == BIN(opt_send_without_block)) { // `compile_inlined_cancel_handler` supports only `opt_send_without_block`
             CALL_DATA cd = (CALL_DATA)body->iseq_encoded[pos + 1];
             const struct rb_callinfo *ci = cd->ci;
-            const struct rb_callcache *cc = mjit_iseq_cc_entries(iseq->body)[call_data_index(cd, body)]; // use copy to avoid race condition
+            const struct rb_callcache *cc = status->cc_entries[call_data_index(cd, body)]; // use copy to avoid race condition
 
             const rb_iseq_t *child_iseq;
             if (has_valid_method_type(cc) &&
@@ -411,7 +421,7 @@ precompile_inlinable_iseqs(FILE *f, const rb_iseq_t *iseq, struct compile_status https://github.com/ruby/ruby/blob/trunk/mjit_compile.c#L421
                             RSTRING_PTR(child_iseq->body->location.label),
                             RSTRING_PTR(rb_iseq_path(child_iseq)), FIX2INT(child_iseq->body->location.first_lineno));
 
-                struct compile_status child_status;
+                struct compile_status child_status = { .compiled_iseq = status->compiled_iseq };
                 INIT_COMPILE_STATUS(child_status, child_iseq->body, false);
                 child_status.inline_context = (struct inlined_call_context){
                     .orig_argc = vm_ci_argc(ci),
@@ -419,9 +429,10 @@ precompile_inlinable_iseqs(FILE *f, const rb_iseq_t *iseq, struct compile_status https://github.com/ruby/ruby/blob/trunk/mjit_compile.c#L429
                     .param_size = child_iseq->body->param.size,
                     .local_size = child_iseq->body->local_table_size
                 };
-                if ((child_iseq->body->ci_size > 0 || child_status.is_entries != NULL)
-                        && !mjit_copy_cache_from_main_thread(child_iseq, child_status.is_entries))
+                if ((child_iseq->body->ci_size > 0 && child_status.cc_entries == NULL)
+                    || (child_status.is_entries != NULL && !mjit_copy_cache_from_main_thread(child_iseq, child_status.is_entries))) {
                     return false;
+                }
 
                 fprintf(f, "ALWAYS_INLINE(static VALUE _mjit_inlined_%d(rb_execution_context_t *ec, rb_control_frame_t *reg_cfp, const VALUE orig_self, const rb_iseq_t *original_iseq));\n", pos);
                 fprintf(f, "static inline VALUE\n_mjit_inlined_%d(rb_execution_context_t *ec, rb_control_frame_t *reg_cfp, const VALUE orig_self, const rb_iseq_t *original_iseq)\n{\n", pos);
@@ -449,10 +460,10 @@ mjit_compile(FILE *f, const rb_iseq_t *iseq, const char *funcname) https://github.com/ruby/ruby/blob/trunk/mjit_compile.c#L460
         fprintf(f, "#define OPT_CHECKED_RUN 0\n\n");
     }
 
-    struct compile_status status;
+    struct compile_status status = { .compiled_iseq = iseq->body };
     INIT_COMPILE_STATUS(status, iseq->body, true);
-    if ((iseq->body->ci_size > 0 || status.is_entries != NULL)
-        && !mjit_copy_cache_from_main_thread(iseq, status.is_entries)) {
+    if ((iseq->body->ci_size > 0 && status.cc_entries == NULL)
+        || (status.is_entries != NULL && !mjit_copy_cache_from_main_thread(iseq, status.is_entries))) {
         return false;
     }
 
diff --git a/mjit_worker.c b/mjit_worker.c
index fcc75a3..e9ba24b 100644
--- a/mjit_worker.c
+++ b/mjit_worker.c
@@ -86,6 +86,7 @@ https://github.com/ruby/ruby/blob/trunk/mjit_worker.c#L86
 #endif
 
 #include "vm_core.h"
+#include "vm_callinfo.h"
 #include "mjit.h"
 #include "gc.h"
 #include "ruby_assert.h"
@@ -160,7 +161,8 @@ struct rb_mjit_unit { https://github.com/ruby/ruby/blob/trunk/mjit_worker.c#L161
     // mjit_compile's optimization switches
     struct rb_mjit_compile_info compile_info;
     // captured CC values, they should be marked with iseq.
-    const struct rb_callcache **cc_entries; // size: iseq->body->ci_size
+    const struct rb_callcache **cc_entries;
+    unsigned int cc_entries_size; // iseq->body->ci_size + ones of inlined iseqs
 };
 
 // Linked list of struct rb_mjit_unit.
@@ -1149,6 +1151,40 @@ static void mjit_copy_job_handler(void *data); https://github.com/ruby/ruby/blob/trunk/mjit_worker.c#L1151
 // vm_trace.c
 int rb_workqueue_register(unsigned flags, rb_postponed_job_func_t , void *);
 
+// Capture cc entries of `captured_iseq` and append them to `compiled_iseq->jit_unit->cc_entries`.
+// This is needed when `captured_iseq` is inlined by `compiled_iseq` and GC needs to mark inlined cc.
+//
+// This assumes that it's safe to reference cc without acquiring GVL.
+const struct rb_callcache **
+mjit_capture_cc_entries(const struct rb_iseq_constant_body *compiled_iseq, const struct rb_iseq_constant_body *captured_iseq)
+{
+    struct rb_mjit_unit *unit = compiled_iseq->jit_unit;
+    unsigned int new_entries_size = unit->cc_entries_size + captured_iseq->ci_size;
+    VM_ASSERT(captured_iseq->ci_size > 0);
+
+    // Allocat (... truncated)

--
ML: ruby-changes@q...
Info: http://www.atdot.net/~ko1/quickml/

[前][次][番号順一覧][スレッド一覧]