feat: Improve performance of substr by 84.44%

This pulls in the implementation of substr into native Go instead of interpretted Jsonnet.

benchmark                       old ns/op     new ns/op     delta
Benchmark_Builtin_substr-16     97121527      15115905      -84.44%

part of google#111

Author: ghostsquad

.gitignore

@@ -14,6 +14,16 @@ gojsonnet.egg-info/
 /bazel-out
 /bazel-testlogs
 /dumpstdlibast
+
+# built binaries
 /jsonnet
+/jsonnet.exe
+
+# "old" built binaries (for doing benchmark comparisons)
+/jsonnet-old
+/jsonnet-old.exe
+
 /linter/jsonnet-lint/jsonnet-lint
 /tests_path.source
+
+/builtin-benchmark-results

Makefile

@@ -42,10 +42,19 @@ build:
 	go build ./cmd/jsonnet
 .PHONY: build
 
+build.old:
+	go build ./cmd/jsonnet -o jsonnet-old
+.PHONY: build.old
+
 test:
 	./tests.sh
 .PHONY: test
 
+benchmark : FILTER="Builtin"
+benchmark: build
+	./benchmark.sh ${FILTER}
+.PHONY: benchmark
+
 generate:
 	go generate
 .PHONY: generate

README.md

@@ -55,6 +55,33 @@ Additionally if any files were moved around, see the section [Keeping the Bazel
 ./tests.sh  # Also runs `go test ./...`
 ```
 
+## Running Benchmarks
+
+Setup
+
+```bash
+go get golang.org/x/tools/cmd/benchcmp
+```
+
+1. Make sure you build a jsonnet binary _prior_ to making changes.
+
+```bash
+go build ./cmd/jsonnet -o jsonnet-old
+```
+
+2. Make changes (iterate as needed), and rebuild new binary
+
+```bash
+go build ./cmd/jsonnet
+```
+
+3. Run benchmark:
+
+```bash
+# e.g. ./benchmark.sh Builtin
+./benchmark.sh <TestNameFilter>
+```
+
 ## Implementation Notes
 
 We are generating some helper classes on types by using http://clipperhouse.github.io/gen/.  Do the following to regenerate these if necessary:

benchmark.sh

@@ -0,0 +1,18 @@
+#!/usr/bin/env bash
+
+set -e
+
+FILTER=${1:-Builtin}
+
+root_dir="$( cd "$( dirname "$0" )" && pwd )"
+
+(
+    cd "${root_dir}"
+    mkdir -p ./builtin-benchmark-results
+    rm -f ./builtin-benchmark-results/*
+    echo "Running Before Test... (10s)"
+    go test -bench="${FILTER}" -run=^$ -v -benchtime=10s -jsonnetPath=./jsonnet-old > ./builtin-benchmark-results/before.txt
+    echo "Running After Test... (10s)"
+    go test -bench="${FILTER}" -run=^$ -v -benchtime=10s -jsonnetPath=./jsonnet > ./builtin-benchmark-results/after.txt
+    benchcmp ./builtin-benchmark-results/before.txt ./builtin-benchmark-results/after.txt
+)

builtin-benchmarks/substr.jsonnet

@@ -0,0 +1,5 @@
+{
+    foo: [
+        std.substr("Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Sed turpis tincidunt id aliquet risus. Eget mauris pharetra et ultrices neque ornare aenean euismod. Diam quis enim lobortis scelerisque fermentum. Varius duis at consectetur lorem donec massa sapien. Diam sit amet nisl suscipit adipiscing bibendum est ultricies integer. Lectus urna duis convallis convallis tellus. Nibh ipsum consequat nisl vel pretium lectus quam id leo. Feugiat in ante metus dictum at tempor commodo. Velit dignissim sodales ut eu sem integer. Dictum sit amet justo donec. Scelerisque mauris pellentesque pulvinar pellentesque habitant morbi tristique senectus. Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Sed turpis tincidunt id aliquet risus. Eget mauris pharetra et ultrices neque ornare aenean euismod. Diam quis enim lobortis scelerisque fermentum. Varius duis at consectetur lorem donec massa sapien. Diam sit amet nisl suscipit adipiscing bibendum est ultricies integer. Lectus urna duis convallis convallis tellus. Nibh ipsum consequat nisl vel pretium lectus quam id leo. Feugiat in ante metus dictum at tempor commodo. Velit dignissim sodales ut eu sem integer. Dictum sit amet justo donec. Scelerisque mauris pellentesque pulvinar pellentesque habitant morbi tristique senectus. Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Sed turpis tincidunt id aliquet risus. Eget mauris pharetra et ultrices neque ornare aenean euismod. Diam quis enim lobortis scelerisque fermentum. Varius duis at consectetur lorem donec massa sapien. Diam sit amet nisl suscipit adipiscing bibendum est ultricies integer. Lectus urna duis convallis convallis tellus. Nibh ipsum consequat nisl vel pretium lectus quam id leo. Feugiat in ante metus dictum at tempor commodo. Velit dignissim sodales ut eu sem integer. Dictum sit amet justo donec. Scelerisque mauris pellentesque pulvinar pellentesque habitant morbi tristique senectus.Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Sed turpis tincidunt id aliquet risus. Eget mauris pharetra et ultrices neque ornare aenean euismod. Diam quis enim lobortis scelerisque fermentum. Varius duis at consectetur lorem donec massa sapien. Diam sit amet nisl suscipit adipiscing bibendum est ultricies integer. Lectus urna duis convallis convallis tellus. Nibh ipsum consequat nisl vel pretium lectus quam id leo. Feugiat in ante metus dictum at tempor commodo. Velit dignissim sodales ut eu sem integer. Dictum sit amet justo donec. Scelerisque mauris pellentesque pulvinar pellentesque habitant morbi tristique senectus. Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Sed turpis tincidunt id aliquet risus. Eget mauris pharetra et ultrices neque ornare aenean euismod. Diam quis enim lobortis scelerisque fermentum. Varius duis at consectetur lorem donec massa sapien. Diam sit amet nisl suscipit adipiscing bibendum est ultricies integer. Lectus urna duis convallis convallis tellus. Nibh ipsum consequat nisl vel pretium lectus quam id leo. Feugiat in ante metus dictum at tempor commodo. Velit dignissim sodales ut eu sem integer. Dictum sit amet justo donec. Scelerisque mauris pellentesque pulvinar pellentesque habitant morbi tristique senectus. Scelerisque mauris pellentesque pulvinar pellentesque habitant morbi tristique senectus. Scelerisque mauris pellentesque pulvinar pellentesque habitant morbi tristique senectus. Scelerisque mauris pellentesque pulvinar pellentesque habitant morbi tristique senectus. Scelerisque mauris pellentesque pulvinar pellentesque habitant morbi tristique senectus. Scelerisque mauris pellentesque pulvinar pellentesque habitant morbi tristique senectus. ", i, 800-i) for i in std.range(0,100)
+    ],
+}

builtins.go

@@ -839,6 +839,59 @@ func builtinPow(i *interpreter, trace traceElement, basev value, expv value) (va
 	return makeDoubleCheck(i, trace, math.Pow(base.value, exp.value))
 }
 
+func builtinSubstr(i *interpreter, trace traceElement, inputStr, inputFrom, inputLen value) (value, error) {
+	strV, err := i.getString(inputStr, trace)
+	if err != nil {
+		msg := fmt.Sprintf("substr first parameter should be a string, got %s", inputStr.getType().name)
+		return nil, makeRuntimeError(msg, i.getCurrentStackTrace(trace))
+	}
+
+	fromV, err := i.getNumber(inputFrom, trace)
+	if err != nil {
+		msg := fmt.Sprintf("substr second parameter should be a number, got %s", inputFrom.getType().name)
+		return nil, makeRuntimeError(msg, i.getCurrentStackTrace(trace))
+	}
+
+	if math.Mod(fromV.value, 1) != 0 {
+		msg := fmt.Sprintf("substr second parameter should be an integer, got %f", fromV.value)
+		return nil, makeRuntimeError(msg, i.getCurrentStackTrace(trace))
+	}
+
+	lenV, err := i.getNumber(inputLen, trace)
+	if err != nil {
+		msg := fmt.Sprintf("substr third parameter should be a number, got %s", inputLen.getType().name)
+		return nil, makeRuntimeError(msg, i.getCurrentStackTrace(trace))
+	}
+
+	lenInt, err := i.getInt(lenV, trace)
+
+	if err != nil {
+		msg := fmt.Sprintf("substr third parameter should be an integer, got %f", lenV.value)
+		return nil, makeRuntimeError(msg, i.getCurrentStackTrace(trace))
+	}
+
+	if lenInt < 0 {
+		msg := fmt.Sprintf("substr third parameter should be greater than zero, got %d", lenInt)
+		return nil, makeRuntimeError(msg, i.getCurrentStackTrace(trace))
+	}
+
+	fromInt := int(fromV.value)
+	strStr := strV.getGoString()
+
+	endIndex := fromInt + lenInt
+
+	if endIndex > len(strStr) {
+		endIndex = len(strStr)
+	}
+
+	if fromInt > len(strStr) {
+		return makeValueString(""), nil
+	}
+
+	runes := []rune(strStr)
+	return makeValueString(string(runes[fromInt:endIndex])), nil
+}
+
 func builtinSplitLimit(i *interpreter, trace traceElement, strv, cv, maxSplitsV value) (value, error) {
 	str, err := i.getString(strv, trace)
 	if err != nil {
@@ -1243,6 +1296,7 @@ var funcBuiltins = buildBuiltinMap([]builtin{
 	&binaryBuiltin{name: "pow", function: builtinPow, parameters: ast.Identifiers{"base", "exp"}},
 	&binaryBuiltin{name: "modulo", function: builtinModulo, parameters: ast.Identifiers{"x", "y"}},
 	&unaryBuiltin{name: "md5", function: builtinMd5, parameters: ast.Identifiers{"x"}},
+	&ternaryBuiltin{name: "substr", function: builtinSubstr, parameters: ast.Identifiers{"str", "from", "len"}},
 	&ternaryBuiltin{name: "splitLimit", function: builtinSplitLimit, parameters: ast.Identifiers{"str", "c", "maxsplits"}},
 	&ternaryBuiltin{name: "strReplace", function: builtinStrReplace, parameters: ast.Identifiers{"str", "from", "to"}},
 	&unaryBuiltin{name: "parseJson", function: builtinParseJSON, parameters: ast.Identifiers{"str"}},

builtins_benchmark_test.go

@@ -0,0 +1,31 @@
+package jsonnet
+
+import (
+	"flag"
+	"os"
+	"os/exec"
+	"testing"
+)
+
+var jsonnetPath string
+var outputPassthru bool
+
+func init() {
+	flag.StringVar(&jsonnetPath, "jsonnetPath", "./jsonnet", "Path to jsonnet binary")
+	flag.BoolVar(&outputPassthru, "outputPassthru", false, "Pass stdout/err from jsonnet")
+}
+
+func Benchmark_Builtin_substr(b *testing.B) {
+	for n := 0; n < b.N; n++ {
+		cmd := exec.Command(jsonnetPath, "./builtin-benchmarks/substr.jsonnet")
+		if outputPassthru {
+			cmd.Stdout = os.Stdout
+			cmd.Stderr = os.Stderr
+		}
+
+		err := cmd.Run()
+		if err != nil {
+			b.Fail()
+		}
+	}
+}

testdata/builtinSubStr_first_param_not_string.golden

@@ -0,0 +1,10 @@
+RUNTIME ERROR: substr first parameter should be a string, got number
+-------------------------------------------------
+	testdata/builtinSubStr_first_param_not_string:1:1-20	builtin function <substr>
+
+std.substr(1, 0, 1)
+
+-------------------------------------------------
+	During evaluation	
+
+

testdata/builtinSubStr_first_param_not_string.jsonnet

@@ -0,0 +1 @@
+std.substr(1, 0, 1)

testdata/builtinSubStr_length_larger.golden

@@ -0,0 +1 @@
+"hello"

testdata/builtinSubStr_length_larger.jsonnet

@@ -0,0 +1 @@
+std.substr("hello", 0 , 10)

testdata/builtinSubStr_second_parameter_not_integer.golden

@@ -0,0 +1,10 @@
+RUNTIME ERROR: substr second parameter should be an integer, got 1.200000
+-------------------------------------------------
+	testdata/builtinSubStr_second_parameter_not_integer:1:1-28	builtin function <substr>
+
+std.substr("hello", 1.2, 5)
+
+-------------------------------------------------
+	During evaluation	
+
+

testdata/builtinSubStr_second_parameter_not_integer.jsonnet

@@ -0,0 +1 @@
+std.substr("hello", 1.2, 5)

testdata/builtinSubStr_second_parameter_not_number.golden

@@ -0,0 +1,10 @@
+RUNTIME ERROR: substr second parameter should be a number, got string
+-------------------------------------------------
+	testdata/builtinSubStr_second_parameter_not_number:1:1-30	builtin function <substr>
+
+std.substr("hello", "foo", 5)
+
+-------------------------------------------------
+	During evaluation	
+
+

testdata/builtinSubStr_second_parameter_not_number.jsonnet

@@ -0,0 +1 @@
+std.substr("hello", "foo", 5)

testdata/builtinSubStr_start_larger_then_size.golden

@@ -0,0 +1 @@
+""

testdata/builtinSubStr_start_larger_then_size.jsonnet

@@ -0,0 +1 @@
+std.substr("hello", 10, 5)

testdata/builtinSubStr_third_parameter_less_then_zero.golden

@@ -0,0 +1,10 @@
+RUNTIME ERROR: substr third parameter should be greater than zero, got -1
+-------------------------------------------------
+	testdata/builtinSubStr_third_parameter_less_then_zero:1:1-27	builtin function <substr>
+
+std.substr("hello", 0, -1)
+
+-------------------------------------------------
+	During evaluation	
+
+

testdata/builtinSubStr_third_parameter_less_then_zero.jsonnet

@@ -0,0 +1 @@
+std.substr("hello", 0, -1)

testdata/builtinSubStr_third_parameter_not_integer.golden

@@ -0,0 +1,10 @@
+RUNTIME ERROR: substr third parameter should be an integer, got 1.200000
+-------------------------------------------------
+	testdata/builtinSubStr_third_parameter_not_integer:1:1-28	builtin function <substr>
+
+std.substr("hello", 0, 1.2)
+
+-------------------------------------------------
+	During evaluation	
+
+

testdata/builtinSubStr_third_parameter_not_integer.jsonnet

@@ -0,0 +1 @@
+std.substr("hello", 0, 1.2)

testdata/builtinSubStr_third_parameter_not_number.golden

@@ -0,0 +1,10 @@
+RUNTIME ERROR: substr third parameter should be a number, got string
+-------------------------------------------------
+	testdata/builtinSubStr_third_parameter_not_number:1:1-30	builtin function <substr>
+
+std.substr("hello", 0, "foo")
+
+-------------------------------------------------
+	During evaluation	
+
+

testdata/builtinSubStr_third_parameter_not_number.jsonnet

@@ -0,0 +1 @@
+std.substr("hello", 0, "foo")

testdata/builtinSubstr.golden

@@ -0,0 +1 @@
+"hello"

testdata/builtinSubstr.jsonnet

@@ -0,0 +1 @@
+std.substr("hello", 0, 5)