From 3b580145f449b58bc0dd9ec8eab9813e969a4496 Mon Sep 17 00:00:00 2001 From: Wes McNamee Date: Sun, 15 Dec 2019 13:43:53 -0800 Subject: [PATCH] feat: improve std.reverse performance by 73.39% Implements std.reverse in native Go, improving performance benchmark old ns/op new ns/op delta Benchmark_Builtin_reverse-16 869191619 231309458 -73.39% part of #111 --- builtin-benchmarks/reverse.jsonnet | 5 +++++ builtins.go | 18 ++++++++++++++++++ builtins_benchmark_test.go | 13 +++++++++++-- testdata/builtinReverse.golden | 4 ++++ testdata/builtinReverse.jsonnet | 1 + testdata/builtinReverse_empty.golden | 1 + testdata/builtinReverse_empty.jsonnet | 1 + testdata/builtinReverse_many.golden | 7 +++++++ testdata/builtinReverse_many.jsonnet | 1 + testdata/builtinReverse_not_array.golden | 10 ++++++++++ testdata/builtinReverse_not_array.jsonnet | 1 + testdata/builtinReverse_single.golden | 3 +++ testdata/builtinReverse_single.jsonnet | 1 + 13 files changed, 64 insertions(+), 2 deletions(-) create mode 100644 builtin-benchmarks/reverse.jsonnet create mode 100644 testdata/builtinReverse.golden create mode 100644 testdata/builtinReverse.jsonnet create mode 100644 testdata/builtinReverse_empty.golden create mode 100644 testdata/builtinReverse_empty.jsonnet create mode 100644 testdata/builtinReverse_many.golden create mode 100644 testdata/builtinReverse_many.jsonnet create mode 100644 testdata/builtinReverse_not_array.golden create mode 100644 testdata/builtinReverse_not_array.jsonnet create mode 100644 testdata/builtinReverse_single.golden create mode 100644 testdata/builtinReverse_single.jsonnet diff --git a/builtin-benchmarks/reverse.jsonnet b/builtin-benchmarks/reverse.jsonnet new file mode 100644 index 0000000..7fffe17 --- /dev/null +++ b/builtin-benchmarks/reverse.jsonnet @@ -0,0 +1,5 @@ +{ + foo: [ + std.reverse(std.range(0, 5000)) for i in std.range(0,100) + ], +} \ No newline at end of file diff --git a/builtins.go b/builtins.go index 0c03d0d..7839f93 100644 --- a/builtins.go +++ b/builtins.go @@ -345,6 +345,23 @@ func builtinJoin(i *interpreter, trace traceElement, sep, arrv value) (value, er } } +func builtinReverse(i *interpreter, trace traceElement, arrv value) (value, error) { + arr, err := i.getArray(arrv, trace) + if err != nil { + return nil, err + } + + lenArr := len(arr.elements) // lenx holds the original array length + reversed_array := make([]*cachedThunk, lenArr) // creates a slice that refer to a new array of length lenx + + for i := 0; i < lenArr; i++ { + j := lenArr - (i + 1) // j initially holds (lenx - 1) and decreases to 0 while i initially holds 0 and increase to (lenx - 1) + reversed_array[i] = arr.elements[j] + } + + return makeValueArray(reversed_array), nil +} + func builtinFilter(i *interpreter, trace traceElement, funcv, arrv value) (value, error) { arr, err := i.getArray(arrv, trace) if err != nil { @@ -1271,6 +1288,7 @@ var funcBuiltins = buildBuiltinMap([]builtin{ &binaryBuiltin{name: "makeArray", function: builtinMakeArray, parameters: ast.Identifiers{"sz", "func"}}, &binaryBuiltin{name: "flatMap", function: builtinFlatMap, parameters: ast.Identifiers{"func", "arr"}}, &binaryBuiltin{name: "join", function: builtinJoin, parameters: ast.Identifiers{"sep", "arr"}}, + &unaryBuiltin{name: "reverse", function: builtinReverse, parameters: ast.Identifiers{"arr"}}, &binaryBuiltin{name: "filter", function: builtinFilter, parameters: ast.Identifiers{"func", "arr"}}, &binaryBuiltin{name: "range", function: builtinRange, parameters: ast.Identifiers{"from", "to"}}, &binaryBuiltin{name: "primitiveEquals", function: primitiveEquals, parameters: ast.Identifiers{"x", "y"}}, diff --git a/builtins_benchmark_test.go b/builtins_benchmark_test.go index 1555417..db30ffc 100644 --- a/builtins_benchmark_test.go +++ b/builtins_benchmark_test.go @@ -2,6 +2,7 @@ package jsonnet import ( "flag" + "fmt" "os" "os/exec" "testing" @@ -15,9 +16,9 @@ func init() { flag.BoolVar(&outputPassthru, "outputPassthru", false, "Pass stdout/err from jsonnet") } -func Benchmark_Builtin_substr(b *testing.B) { +func RunBenchmark(b *testing.B, name string) { for n := 0; n < b.N; n++ { - cmd := exec.Command(jsonnetPath, "./builtin-benchmarks/substr.jsonnet") + cmd := exec.Command(jsonnetPath, fmt.Sprintf("./builtin-benchmarks/%s.jsonnet", name)) if outputPassthru { cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr @@ -29,3 +30,11 @@ func Benchmark_Builtin_substr(b *testing.B) { } } } + +func Benchmark_Builtin_substr(b *testing.B) { + RunBenchmark(b, "substr") +} + +func Benchmark_Builtin_reverse(b *testing.B) { + RunBenchmark(b, "reverse") +} diff --git a/testdata/builtinReverse.golden b/testdata/builtinReverse.golden new file mode 100644 index 0000000..e5c0e48 --- /dev/null +++ b/testdata/builtinReverse.golden @@ -0,0 +1,4 @@ +[ + 2, + 1 +] diff --git a/testdata/builtinReverse.jsonnet b/testdata/builtinReverse.jsonnet new file mode 100644 index 0000000..86ad8e4 --- /dev/null +++ b/testdata/builtinReverse.jsonnet @@ -0,0 +1 @@ +std.reverse([1, 2]) \ No newline at end of file diff --git a/testdata/builtinReverse_empty.golden b/testdata/builtinReverse_empty.golden new file mode 100644 index 0000000..1e3ec72 --- /dev/null +++ b/testdata/builtinReverse_empty.golden @@ -0,0 +1 @@ +[ ] diff --git a/testdata/builtinReverse_empty.jsonnet b/testdata/builtinReverse_empty.jsonnet new file mode 100644 index 0000000..2f6b0a8 --- /dev/null +++ b/testdata/builtinReverse_empty.jsonnet @@ -0,0 +1 @@ +std.reverse([]) \ No newline at end of file diff --git a/testdata/builtinReverse_many.golden b/testdata/builtinReverse_many.golden new file mode 100644 index 0000000..07c1e5e --- /dev/null +++ b/testdata/builtinReverse_many.golden @@ -0,0 +1,7 @@ +[ + "tester", + "is", + "name", + "my", + "hello" +] diff --git a/testdata/builtinReverse_many.jsonnet b/testdata/builtinReverse_many.jsonnet new file mode 100644 index 0000000..d69f7c5 --- /dev/null +++ b/testdata/builtinReverse_many.jsonnet @@ -0,0 +1 @@ +std.reverse(["hello", "my", "name", "is", "tester"]) \ No newline at end of file diff --git a/testdata/builtinReverse_not_array.golden b/testdata/builtinReverse_not_array.golden new file mode 100644 index 0000000..1603ec4 --- /dev/null +++ b/testdata/builtinReverse_not_array.golden @@ -0,0 +1,10 @@ +RUNTIME ERROR: Unexpected type string, expected array +------------------------------------------------- + testdata/builtinReverse_not_array:1:1-20 builtin function + +std.reverse("asdf") + +------------------------------------------------- + During evaluation + + diff --git a/testdata/builtinReverse_not_array.jsonnet b/testdata/builtinReverse_not_array.jsonnet new file mode 100644 index 0000000..12f2b5c --- /dev/null +++ b/testdata/builtinReverse_not_array.jsonnet @@ -0,0 +1 @@ +std.reverse("asdf") \ No newline at end of file diff --git a/testdata/builtinReverse_single.golden b/testdata/builtinReverse_single.golden new file mode 100644 index 0000000..0029f84 --- /dev/null +++ b/testdata/builtinReverse_single.golden @@ -0,0 +1,3 @@ +[ + "hello" +] diff --git a/testdata/builtinReverse_single.jsonnet b/testdata/builtinReverse_single.jsonnet new file mode 100644 index 0000000..cb21d40 --- /dev/null +++ b/testdata/builtinReverse_single.jsonnet @@ -0,0 +1 @@ +std.reverse(["hello"]) \ No newline at end of file