From f0bd45f271811f1e46cf7e787932e1db593b0dc3 Mon Sep 17 00:00:00 2001 From: Michael Darakananda Date: Thu, 7 Sep 2017 13:33:03 +1000 Subject: [PATCH] WIP spanner: optimize array decoding 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 https://github.com/GoogleCloudPlatform/google-cloud-go/commit/6e42463a2bb243d83a0dd4efa595fb913ff9e1ef 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 Reviewed-by: Jonathan Amsterdam --- spanner/value.go | 3 ++- spanner/value_benchmarks_test.go | 7 +++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/spanner/value.go b/spanner/value.go index 63d881865a0f..86d9f71b250a 100644 --- a/spanner/value.go +++ b/spanner/value.go @@ -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) } } diff --git a/spanner/value_benchmarks_test.go b/spanner/value_benchmarks_test.go index a8d89a487a2b..0d95ab9de259 100644 --- a/spanner/value_benchmarks_test.go +++ b/spanner/value_benchmarks_test.go @@ -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}, @@ -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()