ko1 / atdot.net wrote:
> Eric Wong wrote:
> >  Any comment?  I think the improvement is worth it since proc allocation
> >  only happens in 2 places, and env allocation in 1 place.
> 
> For me, +1 for Env, but -1 for Proc.

OK, committed env.

> For Env, allocation part is merged, and easy to be careful.
> However, rb_proc_alloc() can be called far from defenition.
> 
> Or inline allocation code for rb_proc_alloc()?

Yes, I think the following API is OK.  rb_proc_t is big.
The new inline rb_proc_alloc() takes 7(!) parameters.
Maybe we can drop klass since that is always rb_cProc.

--- a/proc.c
+++ b/proc.c
@@ -84,10 +84,11 @@ static const rb_data_type_t proc_data_type = {
 };
 
 VALUE
-rb_proc_alloc(VALUE klass)
+rb_proc_wrap(VALUE klass, rb_proc_t *proc)
 {
-    rb_proc_t *proc;
-    return TypedData_Make_Struct(klass, rb_proc_t, &proc_data_type, proc);
+    proc->block.proc = TypedData_Wrap_Struct(klass, &proc_data_type, proc);
+
+    return proc->block.proc;
 }
 
 VALUE
@@ -105,19 +106,12 @@ rb_obj_is_proc(VALUE proc)
 static VALUE
 proc_dup(VALUE self)
 {
-    VALUE procval = rb_proc_alloc(rb_cProc);
-    rb_proc_t *src, *dst;
-    GetProcPtr(self, src);
-    GetProcPtr(procval, dst);
+    rb_proc_t *src;
 
-    dst->block = src->block;
-    dst->block.proc = procval;
-    dst->blockprocval = src->blockprocval;
-    dst->envval = src->envval;
-    dst->safe_level = src->safe_level;
-    dst->is_lambda = src->is_lambda;
+    GetProcPtr(self, src);
 
-    return procval;
+    return rb_proc_alloc(rb_cProc, &src->block, src->envval, src->blockprocval,
+			src->safe_level, src->is_from_method, src->is_lambda);
 }
 
 /* :nodoc: */
diff --git a/vm.c b/vm.c
index f5fb5a7..7c68f6e 100644
--- a/vm.c
+++ b/vm.c
@@ -655,7 +655,6 @@ VALUE
 rb_vm_make_proc(rb_thread_t *th, const rb_block_t *block, VALUE klass)
 {
     VALUE procval, envval, blockprocval = 0;
-    rb_proc_t *proc;
     rb_control_frame_t *cfp = RUBY_VM_GET_CFP_FROM_BLOCK_PTR(block);
 
     if (block->proc) {
@@ -667,16 +666,9 @@ rb_vm_make_proc(rb_thread_t *th, const rb_block_t *block, VALUE klass)
     if (PROCDEBUG) {
 	check_env_value(envval);
     }
-    procval = rb_proc_alloc(klass);
-    GetProcPtr(procval, proc);
-    proc->blockprocval = blockprocval;
-    proc->block.self = block->self;
-    proc->block.klass = block->klass;
-    proc->block.ep = block->ep;
-    proc->block.iseq = block->iseq;
-    proc->block.proc = procval;
-    proc->envval = envval;
-    proc->safe_level = th->safe_level;
+
+    procval = rb_proc_alloc(klass, block, envval, blockprocval,
+			    th->safe_level, 0, 0);
 
     if (VMDEBUG) {
 	if (th->stack < block->ep && block->ep < th->stack + th->stack_size) {
diff --git a/vm_core.h b/vm_core.h
index 1c3d0cc..e34a755 100644
--- a/vm_core.h
+++ b/vm_core.h
@@ -884,7 +884,32 @@ rb_block_t *rb_vm_control_frame_block_ptr(rb_control_frame_t *cfp);
 
 /* VM related object allocate functions */
 VALUE rb_thread_alloc(VALUE klass);
-VALUE rb_proc_alloc(VALUE klass);
+VALUE rb_proc_wrap(VALUE klass, rb_proc_t *);
+
+static inline VALUE
+rb_proc_alloc(VALUE klass, const rb_block_t *block,
+		VALUE envval, VALUE blockprocval,
+		int8_t safe_level, int8_t is_from_method, int8_t is_lambda)
+{
+    VALUE procval;
+    rb_proc_t *proc = ALLOC(rb_proc_t);
+
+    proc->block = *block;
+    proc->safe_level = safe_level;
+    proc->is_from_method = is_from_method;
+    proc->is_lambda = is_lambda;
+
+    procval = rb_proc_wrap(klass, proc);
+
+    /*
+     * ensure VALUEs are markable here as rb_proc_wrap may trigger allocation
+     * and clobber envval + blockprocval
+     */
+    proc->envval = envval;
+    proc->blockprocval = blockprocval;
+
+    return procval;
+}
 
 /* for debug */
 extern void rb_vmdebug_stack_dump_raw(rb_thread_t *, rb_control_frame_t *);