Lazy Proc allocation for block parameters

ko1 · ko1 · commit 5ee9513a7104 · 2017-10-24T11:13:49.000Z
[Feature #14045] * insns.def (getblockparam, setblockparam): add special access instructions for block parameters. getblockparam checks VM_FRAME_FLAG_MODIFIED_BLOCK_PARAM and if it is not set this instruction creates a Proc object from a given blcok and set VM_FRAME_FLAG_MODIFIED_BLOCK_PARAM. setblockparam is similar to setlocal, but set VM_FRAME_FLAG_MODIFIED_BLOCK_PARAM. * compile.c: use get/setblockparm instead get/setlocal instructions. Note that they are used for method local block parameters (def m(&b)), not for block local method parameters (iter{|&b|). * proc.c (get_local_variable_ptr): creates Proc object for Binding#local_variable_get/set. * safe.c (safe_setter): we need to create Proc objects for postponed block parameters when $SAFE is changed. * vm_args.c (args_setup_block_parameter): used only for block local blcok parameters. * vm_args.c (vm_caller_setup_arg_block): if called with VM_CALL_ARGS_BLOCKARG_BLOCKPARAM flag then passed block values should be a block handler. * test/ruby/test_optimization.rb: add tests. * benchmark/bm_vm1_blockparam*: added. git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@60397 b2dd03c8-39d4-4d8f-98ff-823fe69b080e
diff --git a/benchmark/bm_vm1_blockparam.rb b/benchmark/bm_vm1_blockparam.rb
@@ -0,0 +1,9 @@
+def m &b
+end
+
+i = 0
+while i<30_000_000 # while loop 1
+  i += 1
+  m{}
+end
+
diff --git a/benchmark/bm_vm1_blockparam_call.rb b/benchmark/bm_vm1_blockparam_call.rb
@@ -0,0 +1,9 @@
+def m &b
+  b.call
+end
+
+i = 0
+while i<30_000_000 # while loop 1
+  i += 1
+  m{}
+end
diff --git a/benchmark/bm_vm1_blockparam_pass.rb b/benchmark/bm_vm1_blockparam_pass.rb
@@ -0,0 +1,13 @@
+def bp_yield
+  yield
+end
+
+def bp_pass &b
+  bp_yield &b
+end
+
+i = 0
+while i<30_000_000 # while loop 1
+  i += 1
+  bp_pass{}
+end
diff --git a/benchmark/bm_vm1_blockparam_yield.rb b/benchmark/bm_vm1_blockparam_yield.rb
@@ -0,0 +1,9 @@
+def bp_yield &b
+  yield
+end
+
+i = 0
+while i<30_000_000 # while loop 1
+  i += 1
+  bp_yield{}
+end
diff --git a/compile.c b/compile.c
@@ -300,15 +300,11 @@ struct iseq_compile_data_ensure_node_stack {
       } \
   } while (0)
 
-#define ADD_GETLOCAL(seq, line, idx, level) \
-  do { \
-      ADD_INSN2((seq), (line), getlocal, INT2FIX((idx) + VM_ENV_DATA_SIZE - 1), INT2FIX(level)); \
-  } while (0)
+static void iseq_add_getlocal(rb_iseq_t *iseq, LINK_ANCHOR *const seq, int line, int idx, int level);
+static void iseq_add_setlocal(rb_iseq_t *iseq, LINK_ANCHOR *const seq, int line, int idx, int level);
 
-#define ADD_SETLOCAL(seq, line, idx, level) \
-  do { \
-      ADD_INSN2((seq), (line), setlocal, INT2FIX((idx) + VM_ENV_DATA_SIZE - 1), INT2FIX(level)); \
-  } while (0)
+#define ADD_GETLOCAL(seq, line, idx, level) iseq_add_getlocal(iseq, (seq), (line), (idx), (level))
+#define ADD_SETLOCAL(seq, line, idx, level) iseq_add_setlocal(iseq, (seq), (line), (idx), (level))
 
 /* add label */
 #define ADD_LABEL(seq, label) \
@@ -976,6 +972,18 @@ LIST_SIZE_ZERO(LINK_ANCHOR *const anchor)
     }
 }
 
+static int
+LIST_SIZE_ONE(const LINK_ANCHOR *const anchor)
+{
+    if (anchor->anchor.next != NULL &&
+	anchor->anchor.next->next == NULL) {
+	return 1;
+    }
+    else {
+	return 0;
+    }
+}
+
 /*
  * anc1: e1, e2, e3
  * anc2: e4, e5
@@ -1298,6 +1306,47 @@ get_dyna_var_idx(const rb_iseq_t *iseq, ID id, int *level, int *ls)
     return idx;
 }
 
+static int
+iseq_local_block_param_p(const rb_iseq_t *iseq, unsigned int idx, unsigned int level)
+{
+    while (level > 0) {
+	iseq = iseq->body->parent_iseq;
+	level--;
+    }
+    if (iseq->body->local_iseq == iseq && /* local variables */
+	iseq->body->param.flags.has_block &&
+	iseq->body->local_table_size - iseq->body->param.block_start == idx) {
+	return TRUE;
+    }
+    else {
+	return FALSE;
+    }
+}
+
+static void
+iseq_add_getlocal(rb_iseq_t *iseq, LINK_ANCHOR *const seq, int line, int idx, int level)
+{
+    if (iseq_local_block_param_p(iseq, idx, level)) {
+	ADD_INSN2(seq, line, getblockparam, INT2FIX((idx) + VM_ENV_DATA_SIZE - 1), INT2FIX(level));
+    }
+    else {
+	ADD_INSN2(seq, line, getlocal, INT2FIX((idx) + VM_ENV_DATA_SIZE - 1), INT2FIX(level));
+    }
+}
+
+static void
+iseq_add_setlocal(rb_iseq_t *iseq, LINK_ANCHOR *const seq, int line, int idx, int level)
+{
+    if (iseq_local_block_param_p(iseq, idx, level)) {
+	ADD_INSN2(seq, line, setblockparam, INT2FIX((idx) + VM_ENV_DATA_SIZE - 1), INT2FIX(level));
+    }
+    else {
+	ADD_INSN2(seq, line, setlocal, INT2FIX((idx) + VM_ENV_DATA_SIZE - 1), INT2FIX(level));
+    }
+}
+
+
+
 static void
 iseq_calc_param_size(rb_iseq_t *iseq)
 {
@@ -4173,6 +4222,16 @@ setup_args(rb_iseq_t *iseq, LINK_ANCHOR *const args, NODE *argn,
     }
 
     if (*flag & VM_CALL_ARGS_BLOCKARG) {
+	if (LIST_SIZE_ONE(arg_block)) {
+	    LINK_ELEMENT *elem = FIRST_ELEMENT(arg_block);
+	    if (elem->type == ISEQ_ELEMENT_INSN) {
+		INSN *iobj = (INSN *)elem;
+		if (iobj->insn_id == BIN(getblockparam)) {
+		    iobj->insn_id = BIN(getlocal);
+		    *flag |= VM_CALL_ARGS_BLOCKARG_BLOCKPARAM;
+		}
+	    }
+	}
 	ADD_SEQ(args, arg_block);
     }
     return argc;
diff --git a/insns.def b/insns.def
@@ -80,6 +80,53 @@ setlocal
     (void)RB_DEBUG_COUNTER_INC_IF(lvar_set_dynamic, level > 0);
 }
 
+/**
+  @c variable
+  @e Get a block parameter.
+  @j ブロックパラメータを取得する。
+ */
+DEFINE_INSN
+getblockparam
+(lindex_t idx, rb_num_t level)
+()
+(VALUE val)
+{
+    const VALUE *ep = vm_get_ep(GET_EP(), level);
+    VM_ASSERT(VM_ENV_LOCAL_P(ep));
+
+    if (!VM_ENV_FLAGS(ep, VM_FRAME_FLAG_MODIFIED_BLOCK_PARAM)) {
+	val = rb_vm_bh_to_procval(th, VM_ENV_BLOCK_HANDLER(ep));
+	vm_env_write(ep, -(int)idx, val);
+	VM_ENV_FLAGS_SET(ep, VM_FRAME_FLAG_MODIFIED_BLOCK_PARAM);
+    }
+    else {
+	val = *(ep - idx);
+	RB_DEBUG_COUNTER_INC(lvar_get);
+	(void)RB_DEBUG_COUNTER_INC_IF(lvar_get_dynamic, level > 0);
+    }
+}
+
+/**
+  @c variable
+  @e Set block parameter.
+  @j ブロックパラメータを設定する。
+ */
+DEFINE_INSN
+setblockparam
+(lindex_t idx, rb_num_t level)
+(VALUE val)
+()
+{
+    const VALUE *ep = vm_get_ep(GET_EP(), level);
+    VM_ASSERT(VM_ENV_LOCAL_P(ep));
+
+    vm_env_write(ep, -(int)idx, val);
+    RB_DEBUG_COUNTER_INC(lvar_set);
+    (void)RB_DEBUG_COUNTER_INC_IF(lvar_set_dynamic, level > 0);
+
+    VM_ENV_FLAGS_SET(ep, VM_FRAME_FLAG_MODIFIED_BLOCK_PARAM);
+}
+
 /**
   @c variable
   @e Get value of special local variable ($~, $_, ..).
diff --git a/proc.c b/proc.c
@@ -384,6 +384,8 @@ bind_eval(int argc, VALUE *argv, VALUE bindval)
     return rb_f_eval(argc+1, args, Qnil /* self will be searched in eval */);
 }
 
+VALUE rb_vm_bh_to_procval(rb_thread_t *th, VALUE block_handler);
+
 static const VALUE *
 get_local_variable_ptr(const rb_env_t **envp, ID lid)
 {
@@ -397,6 +399,16 @@ get_local_variable_ptr(const rb_env_t **envp, ID lid)
 
 	    for (i=0; i<iseq->body->local_table_size; i++) {
 		if (iseq->body->local_table[i] == lid) {
+		    if (iseq->body->local_iseq == iseq &&
+			iseq->body->param.flags.has_block &&
+			(unsigned int)iseq->body->param.block_start == i) {
+			const VALUE *ep = env->ep;
+			if (!VM_ENV_FLAGS(ep, VM_FRAME_FLAG_MODIFIED_BLOCK_PARAM)) {
+			    RB_OBJ_WRITE(env, &env->env[i], rb_vm_bh_to_procval(GET_THREAD(), VM_ENV_BLOCK_HANDLER(ep)));
+			    VM_ENV_FLAGS_SET(ep, VM_FRAME_FLAG_MODIFIED_BLOCK_PARAM);
+			}
+		    }
+		    
 		    *envp = env;
 		    return &env->env[i];
 		}
diff --git a/safe.c b/safe.c
@@ -65,17 +65,25 @@ safe_getter(void)
 static void
 safe_setter(VALUE val)
 {
-    int level = NUM2INT(val);
     rb_thread_t *th = GET_THREAD();
+    int current_level = th->ec.safe_level;
+    int level = NUM2INT(val);
 
-    if (level < th->ec.safe_level) {
+    if (level == current_level) {
+	return;
+    }
+    else if (level < current_level) {
 	rb_raise(rb_eSecurityError,
 		 "tried to downgrade safe level from %d to %d",
-		 th->ec.safe_level, level);
+		 current_level, level);
     }
-    if (level > SAFE_LEVEL_MAX) {
+    else if (level > SAFE_LEVEL_MAX) {
 	rb_raise(rb_eArgError, "$SAFE=2 to 4 are obsolete");
     }
+
+    /* block parameters */
+    rb_vm_stack_to_heap(th);
+
     th->ec.safe_level = level;
 }
 
diff --git a/test/ruby/test_optimization.rb b/test/ruby/test_optimization.rb
@@ -575,4 +575,69 @@ def test_retry_label_in_unreachable_chunk
       def t; if false; case 42; when s {}; end; end; end
     end;
   end
+
+  def bptest_yield &b
+    yield
+  end
+
+  def bptest_yield_pass &b
+    bptest_yield(&b)
+  end
+
+  def bptest_bp_value &b
+    b
+  end
+
+  def bptest_bp_pass_bp_value &b
+    bptest_bp_value(&b)
+  end
+
+  def bptest_binding &b
+    binding
+  end
+
+  def bptest_set &b
+    b = Proc.new{2}
+  end
+
+  def test_block_parameter
+    assert_equal(1, bptest_yield{1})
+    assert_equal(1, bptest_yield_pass{1})
+    assert_equal(1, send(:bptest_yield){1})
+
+    assert_equal(Proc, bptest_bp_value{}.class)
+    assert_equal nil, bptest_bp_value
+    assert_equal(Proc, bptest_bp_pass_bp_value{}.class)
+    assert_equal nil, bptest_bp_pass_bp_value
+
+    assert_equal Proc, bptest_binding{}.local_variable_get(:b).class
+
+    assert_equal 2, bptest_set{1}.call
+  end
+
+  def test_block_parameter_should_not_create_objects
+    assert_separately [], <<-END
+      #
+      def foo &b
+      end
+      h1 = {}; h2 = {}
+      ObjectSpace.count_objects(h1) # reharsal
+      ObjectSpace.count_objects(h1)
+      foo{}
+      ObjectSpace.count_objects(h2)
+
+      assert_equal 0, h2[:TOTAL] - h1[:TOTAL]
+    END
+  end
+
+  def test_block_parameter_should_restore_safe_level
+    assert_separately [], <<-END
+      #
+      def foo &b
+        $SAFE = 1
+        b.call
+      end
+      assert_equal 0, foo{$SAFE}
+    END
+  end
 end
diff --git a/vm_args.c b/vm_args.c
diff --git a/vm_core.h b/vm_core.h
diff --git a/vm_insnhelper.c b/vm_insnhelper.c

-Original file line number
+Diff line change
@@ @@ -0,0 +1,9 @@ @@
 +def m &b
 +end
++
 +i = 0
 +while i<30_000_000 # while loop 1
 +  i += 1
 +  m{}
 +end
++