Skip to content

Commit

Permalink
WIP spanner: optimize array decoding
Browse files Browse the repository at this point in the history
DO NOT SUBMIT; WORK IN PROGRESS

Previously BenchmarkDecodeArray did not call the library code
directly, but used a similar function to insulate itself against
library change.
However, the library code calls stringType() at every iteration,
while the benchmark calls the function only once.

This commit adds StringDecodeStringArray subbenchmark which calls
the library code directly to compare performance numbers.
According to the results, saving the result of stringType()
makes us 3x faster in the limit.

I think we should either copy this improvement to other types,
or, as jba@ suggested in
6e42463
move to a generic array decode function.
While the generic function is slower, it would make it harder for
such silly perf bugs to sneek through.

Benchmarks run on go1.9
benchmark                                                old ns/op     new ns/op     delta
BenchmarkDecodeArray/1/StringDecodeStringArray-12        85.1          89.1          +4.70%
BenchmarkDecodeArray/1/StringDirect-12                   80.1          83.1          +3.75%
BenchmarkDecodeArray/10/StringDecodeStringArray-12       547           239           -56.31%
BenchmarkDecodeArray/10/StringDirect-12                  233           236           +1.29%
BenchmarkDecodeArray/100/StringDecodeStringArray-12      4902          1518          -69.03%
BenchmarkDecodeArray/100/StringDirect-12                 1488          1503          +1.01%
BenchmarkDecodeArray/1000/StringDecodeStringArray-12     48642         13952         -71.32%
BenchmarkDecodeArray/1000/StringDirect-12                13808         13957         +1.08%

Change-Id: Ib2d5dfe1c490fe1dcd8f7cf240e46d60eba2b8aa
Reviewed-on: https://code-review.googlesource.com/16610
Reviewed-by: kokoro <[email protected]>
Reviewed-by: Jonathan Amsterdam <[email protected]>
  • Loading branch information
pongad authored and jba committed Sep 7, 2017
1 parent 8469772 commit f0bd45f
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 1 deletion.
3 changes: 2 additions & 1 deletion spanner/value.go
Original file line number Diff line number Diff line change
Expand Up @@ -872,8 +872,9 @@ func decodeStringArray(pb *proto3.ListValue) ([]string, error) {
return nil, errNilListValue("STRING")
}
a := make([]string, len(pb.Values))
st := stringType()
for i, v := range pb.Values {
if err := decodeValue(v, stringType(), &a[i]); err != nil {
if err := decodeValue(v, st, &a[i]); err != nil {
return nil, errDecodeArrayElement(i, v, "STRING", err)
}
}
Expand Down
7 changes: 7 additions & 0 deletions spanner/value_benchmarks_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ func BenchmarkDecodeArray(b *testing.B) {
{"DateDirect", decodeArray_Date_direct},
{"DateFunc", decodeArray_Date_func},
{"DateReflect", decodeArray_Date_reflect},
{"StringDecodeStringArray", decodeStringArrayWrap},
{"StringDirect", decodeArray_String_direct},
{"StringFunc", decodeArray_String_func},
{"StringReflect", decodeArray_String_reflect},
Expand Down Expand Up @@ -154,6 +155,12 @@ func decodeArray_Date_reflect(pb *proto3.ListValue) {
}
}

func decodeStringArrayWrap(pb *proto3.ListValue) {
if _, err := decodeStringArray(pb); err != nil {
panic(err)
}
}

func decodeArray_String_direct(pb *proto3.ListValue) {
a := make([]string, len(pb.Values))
t := stringType()
Expand Down

0 comments on commit f0bd45f

Please sign in to comment.