summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authork0kubun <k0kubun@b2dd03c8-39d4-4d8f-98ff-823fe69b080e>2018-10-20 07:43:50 +0000
committerk0kubun <k0kubun@b2dd03c8-39d4-4d8f-98ff-823fe69b080e>2018-10-20 07:43:50 +0000
commit8449f4992b9eee9a4948a90a24faa47c59d7f0db (patch)
tree139326978207a90fdd10f36c044bb410428590a1
parentb640b21d9c6698d2246d2e0eb785653b570adcea (diff)
vm_insnhelper.c: never cache getinstancevariable twice
We have several options to ensure there's no race condition between main thread and MJIT thead about IC reference: 1) Give up caching ivar for multiple classes (or multiple versions of the same class) in the same getinstancevariable (This commit's approach) 2) Allocate new inline cache every time Other ideas we could think of couldn't eliminate possibilities of race condition. In 2, it's memory allocation would be slow and it may trigger JIT cancellation frequently. So 1 would be fast for both VM and JIT situations. git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@65213 b2dd03c8-39d4-4d8f-98ff-823fe69b080e
-rw-r--r--tool/ruby_vm/views/_mjit_compile_ivar.erb2
-rw-r--r--vm.c2
-rw-r--r--vm_core.h5
-rw-r--r--vm_insnhelper.c20
4 files changed, 21 insertions, 8 deletions
diff --git a/tool/ruby_vm/views/_mjit_compile_ivar.erb b/tool/ruby_vm/views/_mjit_compile_ivar.erb
index ab8e885..cd5e81c 100644
--- a/tool/ruby_vm/views/_mjit_compile_ivar.erb
+++ b/tool/ruby_vm/views/_mjit_compile_ivar.erb
@@ -15,7 +15,7 @@
% end
%
% # compiler: Consider cfp->self as T_OBJECT if ic->ic_serial is set
- if (ic->ic_serial) {
+ if (ic->ic_serial >= RUBY_VM_CLASS_SERIAL_MIN_VALID_VALUE) {
% # JIT: optimize away motion of sp and pc. This path does not call rb_warning() and so it's always leaf and not `handles_sp`.
% # <%= render 'mjit_compile_pc_and_sp', locals: { insn: insn } -%>
%
diff --git a/vm.c b/vm.c
index fababaa..88a7ef3 100644
--- a/vm.c
+++ b/vm.c
@@ -340,7 +340,7 @@ rb_event_flag_t ruby_vm_event_flags;
rb_event_flag_t ruby_vm_event_enabled_flags;
rb_serial_t ruby_vm_global_method_state = 1;
rb_serial_t ruby_vm_global_constant_state = 1;
-rb_serial_t ruby_vm_class_serial = 1;
+rb_serial_t ruby_vm_class_serial = RUBY_VM_CLASS_SERIAL_MIN_VALID_VALUE;
static void thread_free(void *ptr);
diff --git a/vm_core.h b/vm_core.h
index 34217f7..26f00bd 100644
--- a/vm_core.h
+++ b/vm_core.h
@@ -1846,4 +1846,9 @@ void rb_postponed_job_flush(rb_vm_t *vm);
RUBY_SYMBOL_EXPORT_END
+/* special values for ruby_vm_class_serial */
+#define RUBY_VM_CLASS_SERIAL_UNSET 0 /* Not cached yet. Fields in `is_entries` and `cc_entries` start from 0 due to ZALLOC_N. */
+#define RUBY_VM_CLASS_SERIAL_INVALID 1 /* Cache invalidated and never cached again. */
+#define RUBY_VM_CLASS_SERIAL_MIN_VALID_VALUE 2 /* Actual class serials are larger than this value. */
+
#endif /* RUBY_VM_CORE_H */
diff --git a/vm_insnhelper.c b/vm_insnhelper.c
index 3f09991..8c01a62 100644
--- a/vm_insnhelper.c
+++ b/vm_insnhelper.c
@@ -976,12 +976,20 @@ vm_getivar(VALUE obj, ID id, IC ic, struct rb_call_cache *cc, int is_attr)
if (index < ROBJECT_NUMIV(obj)) {
val = ROBJECT_IVPTR(obj)[index];
}
- if (!is_attr) {
- ic->ic_value.index = index;
- ic->ic_serial = RCLASS_SERIAL(RBASIC(obj)->klass);
- }
- else { /* call_info */
- cc->aux.index = (int)index + 1;
+ if (is_attr) { /* call_info */
+ cc->aux.index = (int)index + 1;
+ }
+ else { /* getinstancevariable */
+ if (ic->ic_serial == RUBY_VM_CLASS_SERIAL_UNSET) {
+ /* set ic_serial only for the first time */
+ ic->ic_value.index = index;
+ ic->ic_serial = RCLASS_SERIAL(RBASIC(obj)->klass);
+ }
+ else if (ic->ic_serial != RUBY_VM_CLASS_SERIAL_INVALID) {
+ /* never use cache for another class, to avoid race condition with MJIT worker
+ and to reduce the number of JIT cancellations by code generated for IC hit. */
+ ic->ic_serial = RUBY_VM_CLASS_SERIAL_INVALID;
+ }
}
}
}