Minor performance improvements in reflector funcWrapper

Author: spy16

.github/workflows/code_change.yml

@@ -17,4 +17,7 @@ jobs:
         uses: actions/checkout@v2
 
       - name: Fetch dependencies, verify build and test
-        run: make build test benchmark
+        run: make build test
+
+      - name: Run Benchmarks
+        run: make benchmark

CHANGELOG.md

@@ -5,7 +5,13 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
-## [0.1.0] - 2020-10-14
+## v0.1.1 - 2020-10-14
+
+### Added
+
+- Minor performance improvements in reflector funcWrapper.
+
+## v0.1.0 - 2020-10-14
 
 ### Added
 

expr_test.go

@@ -7,7 +7,7 @@ import (
 	"testing"
 )
 
-var unknownErr = errors.New("failed")
+var errUnknown = errors.New("failed")
 
 func TestConstExpr_Eval(t *testing.T) {
 	t.Parallel()
@@ -56,10 +56,10 @@ func TestDoExpr_Eval(t *testing.T) {
 			title: "ExprEvalFail",
 			expr: func() (Expr, *Env) {
 				return DoExpr{Exprs: []Expr{
-					fakeExpr{Err: unknownErr},
+					fakeExpr{Err: errUnknown},
 				}}, nil
 			},
-			wantErr: unknownErr,
+			wantErr: errUnknown,
 		},
 		{
 			title: "MultipleExpr",
@@ -102,11 +102,11 @@ func TestDefExpr_Eval(t *testing.T) {
 				e := New()
 				return DefExpr{
 					Name:  "foo",
-					Value: fakeExpr{Err: unknownErr},
+					Value: fakeExpr{Err: errUnknown},
 					Env:   e,
 				}, e
 			},
-			wantErr: unknownErr,
+			wantErr: errUnknown,
 		},
 		{
 			title: "ExprValue",
@@ -170,10 +170,10 @@ func TestIfExpr_Eval(t *testing.T) {
 			title: "TestEvalErr",
 			expr: func() (Expr, *Env) {
 				return IfExpr{
-					Test: fakeExpr{Err: unknownErr},
+					Test: fakeExpr{Err: errUnknown},
 				}, New()
 			},
-			wantErr: unknownErr,
+			wantErr: errUnknown,
 		},
 		{
 			title: "Else",
@@ -195,10 +195,10 @@ func TestInvokeExpr_Eval(t *testing.T) {
 			title: "TargetEvalErr",
 			expr: func() (Expr, *Env) {
 				return &InvokeExpr{
-					Target: fakeExpr{Err: unknownErr},
+					Target: fakeExpr{Err: errUnknown},
 				}, New()
 			},
-			wantErr: unknownErr,
+			wantErr: errUnknown,
 		},
 		{
 			title: "NonInvokable",
@@ -238,11 +238,11 @@ func TestInvokeExpr_Eval(t *testing.T) {
 				return &InvokeExpr{
 					Target: ConstExpr{Const: fakeInvokable(nil)},
 					Args: []Expr{
-						fakeExpr{Err: unknownErr},
+						fakeExpr{Err: errUnknown},
 					},
 				}, New()
 			},
-			wantErr: unknownErr,
+			wantErr: errUnknown,
 		},
 	})
 }

reflector/func.go

@@ -56,17 +56,37 @@ type funcWrapper struct {
 }
 
 func (fw *funcWrapper) Invoke(env *slurp.Env, args ...slurp.Any) (slurp.Any, error) {
-	argVals := reflectValues(args)
+	argCount := len(args)
 	if fw.passScope {
-		argVals = append([]reflect.Value{reflect.ValueOf(env)}, argVals...)
+		// we need to pass 'env' also to the underlying function.
+		// so apart from explicitly passed arguments, one extra
+		// is needed.
+		argCount++
 	}
 
+	// allocate argument slice, including the space for 'env' argument
+	// if the function needs it.
+	argVals := make([]reflect.Value, argCount, argCount)
+	if fw.passScope {
+		argVals[0] = reflect.ValueOf(env)
+	}
+
+	// populate reflect.Value version of each argument.
+	for i, arg := range args {
+		if fw.passScope {
+			// 0th index is reserved for passing env. so offset index
+			// by 1.
+			i++
+		}
+		argVals[i] = reflect.ValueOf(arg)
+	}
+
+	// verify number of args match the required function parameters.
 	if err := fw.checkArgCount(len(argVals)); err != nil {
 		return nil, err
 	}
 
-	argVals, err := fw.convertTypes(argVals...)
-	if err != nil {
+	if err := fw.convertTypes(argVals...); err != nil {
 		return nil, err
 	}
 
@@ -106,27 +126,28 @@ func (fw *funcWrapper) argNames() []string {
 	return argNames
 }
 
-func (fw *funcWrapper) convertTypes(args ...reflect.Value) ([]reflect.Value, error) {
-	var vals []reflect.Value
+func (fw *funcWrapper) convertTypes(args ...reflect.Value) error {
+	lastArgIdx := fw.rt.NumIn() - 1
+	isVariadic := fw.rt.IsVariadic()
 
 	for i := 0; i < fw.rt.NumIn(); i++ {
-		if fw.rt.IsVariadic() && i == fw.rt.NumIn()-1 {
+		if i == lastArgIdx && isVariadic {
 			c, err := convertArgsTo(fw.rt.In(i).Elem(), args[i:]...)
 			if err != nil {
-				return nil, err
+				return err
 			}
-			vals = append(vals, c...)
+			copy(args[i:], c)
 			break
 		}
 
 		c, err := convertArgsTo(fw.rt.In(i), args[i])
 		if err != nil {
-			return nil, err
+			return err
 		}
-		vals = append(vals, c...)
+		args[i] = c[0]
 	}
 
-	return vals, nil
+	return nil
 }
 
 func (fw *funcWrapper) checkArgCount(count int) error {
@@ -165,26 +186,31 @@ func (fw *funcWrapper) wrapReturns(vals ...reflect.Value) (slurp.Any, error) {
 		}
 	}
 
-	wrapped := slurpValues(vals[0 : fw.lastOutIdx+1])
-	if len(wrapped) == 1 {
+	retValCount := len(vals[0 : fw.lastOutIdx+1])
+	wrapped := make([]slurp.Any, retValCount, retValCount)
+	for i := 0; i < retValCount; i++ {
+		wrapped[i] = vals[i].Interface()
+	}
+
+	if retValCount == 1 {
 		return wrapped[0], nil
 	}
 
 	return slurp.NewList(wrapped...), nil
 }
 
 func convertArgsTo(expected reflect.Type, args ...reflect.Value) ([]reflect.Value, error) {
-	var converted []reflect.Value
-	for _, arg := range args {
+	converted := make([]reflect.Value, len(args), len(args))
+	for i, arg := range args {
 		actual := arg.Type()
-		switch {
-		case isAssignable(actual, expected):
-			converted = append(converted, arg)
-
-		case actual.ConvertibleTo(expected):
-			converted = append(converted, arg.Convert(expected))
-
-		default:
+		isAssignable := (actual == expected) ||
+			actual.AssignableTo(expected) ||
+			(expected.Kind() == reflect.Interface && actual.Implements(expected))
+		if isAssignable {
+			converted[i] = arg
+		} else if actual.ConvertibleTo(expected) {
+			converted[i] = arg.Convert(expected)
+		} else {
 			return args, fmt.Errorf(
 				"value of type '%s' cannot be converted to '%s'",
 				actual, expected,
@@ -195,11 +221,6 @@ func convertArgsTo(expected reflect.Type, args ...reflect.Value) ([]reflect.Valu
 	return converted, nil
 }
 
-func isAssignable(from, to reflect.Type) bool {
-	return (from == to) || from.AssignableTo(to) ||
-		(to.Kind() == reflect.Interface && from.Implements(to))
-}
-
 func reflectValues(args []slurp.Any) []reflect.Value {
 	var rvs []reflect.Value
 	for _, arg := range args {

reflector/func_test.go

@@ -0,0 +1,74 @@
+package reflector
+
+import (
+	"reflect"
+	"testing"
+
+	"github.com/spy16/slurp"
+)
+
+var (
+	int64Type = reflect.TypeOf(int64(0))
+	int8Type  = reflect.TypeOf(int8(0))
+	int8Vals  []reflect.Value
+)
+
+func init() {
+	int8Vals = make([]reflect.Value, 1000)
+	for i := 0; i < len(int8Vals); i++ {
+		int8Vals[i] = reflect.ValueOf(int8(0))
+	}
+}
+
+func Benchmark_funcWrapper_Invoke(b *testing.B) {
+	fw := Func("foo", func(a, b int) int { return a + b })
+
+	var res int
+	for i := 0; i < b.N; i++ {
+		ret, _ := fw.Invoke(nil, 1, 2)
+		res = ret.(int)
+	}
+	b.Logf("final result: %d", res)
+}
+
+func Benchmark_funcWrapper_Invoke_Variadic(b *testing.B) {
+	fw := Func("foo", func(args ...int32) int32 { return args[0] + args[len(args)-1] })
+
+	args := make([]slurp.Any, 1000)
+	for i := 0; i < len(args); i++ {
+		args[i] = 1
+	}
+
+	var res int32
+	for i := 0; i < b.N; i++ {
+		ret, _ := fw.Invoke(nil, args...)
+		res = ret.(int32)
+	}
+	b.Logf("final result: %d", res)
+}
+
+func Benchmark_convertArgsTo_Convertible(b *testing.B) {
+	var retVals []reflect.Value
+
+	var ret []reflect.Value
+	for i := 0; i < b.N; i++ {
+		ret, _ = convertArgsTo(int64Type, int8Vals...)
+		retVals = ret
+	}
+	dummyPrint(b, retVals)
+}
+
+func Benchmark_convertArgsTo_Assign(b *testing.B) {
+	var retVals []reflect.Value
+
+	var ret []reflect.Value
+	for i := 0; i < b.N; i++ {
+		ret, _ = convertArgsTo(int8Type, int8Vals...)
+		retVals = ret
+	}
+	dummyPrint(b, retVals)
+}
+
+func dummyPrint(b *testing.B, retVals []reflect.Value) {
+	b.Logf("return values of size=%d", len(retVals))
+}

slurp_test.go

@@ -83,8 +83,45 @@ func TestEnv_Resolve(t *testing.T) {
 	assert(t, got == true, "want=true got=%#v", got)
 }
 
-func assert(t *testing.T, cond bool, msg string, args ...interface{}) {
+func Benchmark_Eval_QuoteForm(b *testing.B) {
+	env := New()
+	quoteForm := QuoteExpr{Form: NewList(1, 2, 3)}
+
+	for i := 0; i < b.N; i++ {
+		_, err := env.Eval(quoteForm)
+		if err != nil {
+			b.Fatalf("eval failed: %v", err)
+		}
+	}
+}
+
+func Benchmark_Eval_Invocation(b *testing.B) {
+	env := New()
+	invocation := InvokeExpr{
+		Env:  env,
+		Name: "test",
+		Target: ConstExpr{Const: fakeInvokable(func(env *Env, args ...Any) (Any, error) {
+			return 10, nil
+		})},
+	}
+
+	var finalRes Any
+	for i := 0; i < b.N; i++ {
+		res, err := env.Eval(invocation)
+		if err != nil {
+			b.Fatalf("eval failed: %v", err)
+		}
+		finalRes = res
+	}
+	assert(b, finalRes == 10, "want=10, got=%#v", finalRes)
+}
+
+func assert(t testInstance, cond bool, msg string, args ...interface{}) {
 	if !cond {
 		t.Errorf(msg, args...)
 	}
 }
+
+type testInstance interface {
+	Errorf(msg string, args ...interface{})
+}