Eliminate allocation of variables in transform_engine()

When an instruction with a variable number operands (such as
select_val) is seen of the left side of a transformation, the
'next_arg' instruction will allocate a buffer to fit all variables and
all operands will be copied into the buffer. Very often, the 'commit'
instruction will never be reached because of a test or predicate
failing or because of a short window; in that case, the variable
buffer will be deallocated.

Note that originally there were only few instructions with a variable
number of operands, but now common operations such as tuple building
also have a variable number of operands.

To avoid those frequent allocations and deallocations, modify the
'next_arg' instruction to only save a pointer to the first of the
"rest" arguments. Also move the deallocation of the instructions
on the left side from the 'commit' instruction to the 'end'
instruction to ensure that 'store_rest_args' will still work.

2 files changed, 49 insertions, 110 deletions

diff --git a/erts/emulator/beam/beam_load.c b/erts/emulator/beam/beam_load.c
index 2f2b433999..c6c35e74c9 100644
--- a/erts/emulator/beam/beam_load.c
+++ b/erts/emulator/beam/beam_load.c
@@ -4813,31 +4813,25 @@ transform_engine(LoaderState* st)
     Uint op;
     int ap;			/* Current argument. */
     Uint* restart;		/* Where to restart if current match fails. */
-    GenOpArg def_vars[TE_MAX_VARS]; /* Default buffer for variables. */
-    GenOpArg* var = def_vars;
-    int num_vars = 0;
+    GenOpArg var[TE_MAX_VARS];	/* Buffer for variables. */
+    GenOpArg* rest_args = NULL;
+    int num_rest_args = 0;
     int i;			/* General index. */
     Uint mask;
     GenOp* instr;
+    GenOp* first = st->genop;
+    GenOp* keep = NULL;
     Uint* pc;
-    int rval;
     static Uint restart_fail[1] = {TOP_fail};
 
-    ASSERT(gen_opc[st->genop->op].transform != -1);
-    pc = op_transform + gen_opc[st->genop->op].transform;
-    restart = pc;
+    ASSERT(gen_opc[first->op].transform != -1);
+    restart = op_transform + gen_opc[first->op].transform;
 
  restart:
-    if (var != def_vars) {
-	erts_free(ERTS_ALC_T_LOADER_TMP, (void *) var);
-	var = def_vars;
-    }
     ASSERT(restart != NULL);
     pc = restart;
     ASSERT(*pc < NUM_TOPS);	/* Valid instruction? */
-    instr = st->genop;
-
-#define RETURN(r) rval = (r); goto do_return;
+    instr = first;
 
 #ifdef DEBUG
     restart = NULL;
@@ -4855,7 +4849,7 @@ transform_engine(LoaderState* st)
 		 * We'll need at least one more instruction to decide whether
 		 * this combination matches or not.
 		 */
-		RETURN(TE_SHORT_WINDOW);
+		return TE_SHORT_WINDOW;
 	    }
 	    if (*pc++ != instr->op)
 		goto restart;
@@ -5017,19 +5011,9 @@ transform_engine(LoaderState* st)
 #if defined(TOP_rest_args)
 	case TOP_rest_args:
 	    {
-		int n = *pc++;
 		int formal_arity = gen_opc[instr->op].arity;
-		int j = formal_arity;
-
-		num_vars = n + (instr->arity - formal_arity);
-		var = erts_alloc(ERTS_ALC_T_LOADER_TMP,
-				 num_vars * sizeof(GenOpArg));
-		for (i = 0; i < n; i++) {
-		    var[i] = def_vars[i];
-		}
-		while (i < num_vars) {
-		    var[i++] = instr->a[j++];
-		}
+		num_rest_args = instr->arity - formal_arity;
+		rest_args = instr->a + formal_arity;
 	    }
 	    break;
 #endif
@@ -5038,16 +5022,8 @@ transform_engine(LoaderState* st)
 	    break;
 	case TOP_commit:
 	    instr = instr->next; /* The next_instr was optimized away. */
-
-	    /*
-	     * The left-hand side of this transformation matched.
-	     * Delete all matched instructions.
-	     */
-	    while (st->genop != instr) {
-		GenOp* next = st->genop->next;
-		FREE_GENOP(st, st->genop);
-		st->genop = next;
-	    }
+	    keep = instr;
+	    st->genop = instr;
 #ifdef DEBUG
 	    instr = 0;
 #endif
@@ -5077,22 +5053,19 @@ transform_engine(LoaderState* st)
 		    lastp = &((*lastp)->next);
 		}
 		 
-		instr = instr->next; /* The next_instr was optimized away. */
-
-		/*
-		 * The left-hand side of this transformation matched.
-		 * Delete all matched instructions.
-		 */
-		while (st->genop != instr) {
-		    GenOp* next = st->genop->next;
-		    FREE_GENOP(st, st->genop);
-		    st->genop = next;
-		}
-		*lastp = st->genop;
+		keep = instr->next; /* The next_instr was optimized away. */
+		*lastp = keep;
 		st->genop = new_instr;
 	    }
-	    RETURN(TE_OK);
+	    /* FALLTHROUGH */
 #endif
+	case TOP_end:
+	    while (first != keep) {
+		GenOp* next = first->next;
+		FREE_GENOP(st, first);
+		first = next;
+	    }
+	    return TE_OK;
 	case TOP_new_instr:
 	    /*
 	     * Note that the instructions are generated in reverse order.
@@ -5123,14 +5096,10 @@ transform_engine(LoaderState* st)
 #if defined(TOP_store_rest_args)
 	case TOP_store_rest_args:
 	    {
-		int n = *pc++;
-		int num_extra = num_vars - n;
-
-		ASSERT(n <= num_vars);
-		GENOP_ARITY(instr, instr->arity+num_extra);
+		GENOP_ARITY(instr, instr->arity+num_rest_args);
 		memcpy(instr->a, instr->def_args, ap*sizeof(GenOpArg));
-		memcpy(instr->a+ap, var+n, num_extra*sizeof(GenOpArg));
-		ap += num_extra;
+		memcpy(instr->a+ap, rest_args, num_rest_args*sizeof(GenOpArg));
+		ap += num_rest_args;
 	    }
 	    break;
 #endif
@@ -5142,21 +5111,12 @@ transform_engine(LoaderState* st)
 	case TOP_try_me_else_fail:
 	    restart = restart_fail;
 	    break;
-	case TOP_end:
-	    RETURN(TE_OK);
 	case TOP_fail:
-	    RETURN(TE_FAIL);
+	    return TE_FAIL;
 	default:
 	    ASSERT(0);
 	}
     }
-#undef RETURN
-
- do_return:
-    if (var != def_vars) {
-	erts_free(ERTS_ALC_T_LOADER_TMP, (void *) var);
-    }
-    return rval;
 }
 
 static void
diff --git a/erts/emulator/utils/beam_makeops b/erts/emulator/utils/beam_makeops
index f805e7cc64..86bfb5d746 100755
--- a/erts/emulator/utils/beam_makeops
+++ b/erts/emulator/utils/beam_makeops
@@ -1504,8 +1504,6 @@ sub tr_gen_from {
     my($var_num) = 0;
     my(@code);
     my($min_window) = 0;
-    my(@fix_rest_args);
-    my(@fix_pred_funcs);
     my($op, $ref);		# Loop variables.
     my $where = "left side of transformation in line $line: ";
     my %var_used = %$used_ref;
@@ -1530,8 +1528,17 @@ sub tr_gen_from {
 	    my $var;
 	    my(@args);
 
-	    push(@fix_pred_funcs, scalar(@code));
-	    push(@code, [$name, @ops]);
+	    foreach $var (@ops) {
+		error($where, "variable '$var' unbound")
+		    unless defined $var{$var};
+		if ($var_type{$var} eq 'scalar') {
+		    push(@args, "var[$var{$var}]");
+		} else {
+		    push(@args, "rest_args");
+		}
+	    }
+	    my $pi = tr_next_index(\@pred_table, \%pred_table, $name, @args);
+	    push(@code, make_op("$name()", 'pred', $pi));
 	    next;
 	}
 
@@ -1595,12 +1602,16 @@ sub tr_gen_from {
 		    $may_fail = 1;
 		    push(@code, &make_op($var, 'is_same_var', $var{$var}));
 		} elsif ($type eq '*') {
-		    #
-		    # Reserve a hole for a 'rest_args' instruction.
-		    #
+		    foreach my $type (values %var_type) {
+			error("only one use of a '*' variable is " .
+			      "allowed on the left hand side of " .
+			      "a transformation")
+			    if $type eq 'array';
+		    }
 		    $ignored_var = '';
-		    push(@fix_rest_args, scalar(@code));
-		    push(@code, $var);
+		    $var{$var} = 'unnumbered';
+		    $var_type{$var} = 'array';
+		    push(@code, make_op($var, 'rest_args'));
 		} elsif ($var_used{$var}) {
 		    $ignored_var = '';
 		    $var_type{$var} = 'scalar';
@@ -1629,38 +1640,6 @@ sub tr_gen_from {
     #
     push(@code, make_op($may_fail ? '' : 'always reached', 'commit'));
 
-    #
-    # If there is an rest_args instruction, we must insert its correct
-    # variable number (higher than any other).
-    #
-    my $index;
-    &error("only one use of a '*' variable is allowed on the left hand side of a transformation")
-	if @fix_rest_args > 1;
-    foreach $index (@fix_rest_args) {
-	my $var = $code[$index];
-	$var{$var} = $var_num++;
-	$var_type{$var} = 'array';
-	splice(@code, $index, 1, &make_op($var, 'rest_args', $var{$var}));
-    }
-
-    foreach $index (@fix_pred_funcs) {
-	my($name, @ops) = @{$code[$index]};
-	my(@args);
-	my $var;
-
-	foreach $var (@ops) {
-	    &error($where, "variable '$var' unbound")
-		unless defined $var{$var};
-	    if ($var_type{$var} eq 'scalar') {
-		push(@args, "var[$var{$var}]");
-	    } else {
-		push(@args, "var+$var{$var}");
-	    }
-	}
-	my $pi = tr_next_index(\@pred_table, \%pred_table, $name, @args);
-	splice(@code, $index, 1, make_op("$name()", 'pred', $pi));
-    }
-
     $te_max_vars = $var_num
 	if $te_max_vars < $var_num;
     [$min_window, \%var, \%var_type, \@code];
@@ -1697,7 +1676,7 @@ sub tr_gen_to {
 		if ($var_type{$var} eq 'scalar') {
 		    push(@args, "var[$var{$var}]");
 		} else {
-		    push(@args, "var+$var{$var}");
+		    push(@args, "rest_args");
 		}
 	    }
 	    pop(@code);	# Get rid of 'commit' instruction
@@ -1725,7 +1704,7 @@ sub tr_gen_to {
 	    my($var, $type, $type_val) = @$op;
 
 	    if ($type eq '*') {
-		push(@code, make_op($var, 'store_rest_args', $var{$var}));
+		push(@code, make_op($var, 'store_rest_args'));
 	    } elsif ($var ne '') {
 		&error($where, "variable '$var' unbound")
 		    unless defined $var{$var};