From bc8b1ca32003df666c37f6b99aff7a68b0d0a577 Mon Sep 17 00:00:00 2001 From: Evan Jones Date: Fri, 16 Jun 2023 15:50:45 -0400 Subject: [PATCH] remove the single backing string optimization MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is a bit slower than using this optimization, but ensures this version does not change garbage collection behavior. This does still using a single []string for all the *string value pointers because that is what text parsing already does. This makes the two behave similarly. benchstat results of master versus this version: │ orig.txt │ new-binary-no-share-string.txt │ │ sec/op │ sec/op vs base │ HstoreScan/databasesql.Scan-10 82.11µ ± 1% 81.71µ ± 2% ~ (p=0.280 n=10) HstoreScan/text-10 83.30µ ± 1% 82.45µ ± 1% -1.02% (p=0.000 n=10) HstoreScan/binary-10 15.99µ ± 2% 10.12µ ± 1% -36.67% (p=0.000 n=10) geomean 47.82µ 40.86µ -14.56% │ orig.txt │ new-binary-no-share-string.txt │ │ B/op │ B/op vs base │ HstoreScan/databasesql.Scan-10 56.23Ki ± 0% 56.23Ki ± 0% ~ (p=0.128 n=10) HstoreScan/text-10 65.12Ki ± 0% 65.12Ki ± 0% ~ (p=0.541 n=10) HstoreScan/binary-10 21.09Ki ± 0% 19.87Ki ± 0% -5.75% (p=0.000 n=10) geomean 42.58Ki 41.75Ki -1.95% │ orig.txt │ new-binary-no-share-string.txt │ │ allocs/op │ allocs/op vs base │ HstoreScan/databasesql.Scan-10 744.0 ± 0% 744.0 ± 0% ~ (p=1.000 n=10) ¹ HstoreScan/text-10 743.0 ± 0% 743.0 ± 0% ~ (p=1.000 n=10) ¹ HstoreScan/binary-10 464.0 ± 0% 316.0 ± 0% -31.90% (p=0.000 n=10) geomean 635.4 559.0 -12.02% ¹ all samples are equal benchstat results of the version with one string and this version: │ new-binary-share-everything.txt │ new-binary-no-share-string.txt │ │ sec/op │ sec/op vs base │ HstoreScan/databasesql.Scan-10 81.80µ ± 1% 81.71µ ± 2% ~ (p=1.000 n=10) HstoreScan/text-10 82.77µ ± 1% 82.45µ ± 1% ~ (p=0.063 n=10) HstoreScan/binary-10 7.330µ ± 2% 10.124µ ± 1% +38.13% (p=0.000 n=10) geomean 36.75µ 40.86µ +11.18% │ new-binary-share-everything.txt │ new-binary-no-share-string.txt │ │ B/op │ B/op vs base │ HstoreScan/databasesql.Scan-10 56.23Ki ± 0% 56.23Ki ± 0% ~ (p=0.232 n=10) HstoreScan/text-10 65.12Ki ± 0% 65.12Ki ± 0% ~ (p=0.218 n=10) HstoreScan/binary-10 20.73Ki ± 0% 19.87Ki ± 0% -4.11% (p=0.000 n=10) geomean 42.34Ki 41.75Ki -1.39% │ new-binary-share-everything.txt │ new-binary-no-share-string.txt │ │ allocs/op │ allocs/op vs base │ HstoreScan/databasesql.Scan-10 744.0 ± 0% 744.0 ± 0% ~ (p=1.000 n=10) ¹ HstoreScan/text-10 743.0 ± 0% 743.0 ± 0% ~ (p=1.000 n=10) ¹ HstoreScan/binary-10 41.00 ± 0% 316.00 ± 0% +670.73% (p=0.000 n=10) geomean 283.0 559.0 +97.53% ¹ all samples are equal --- pgtype/hstore.go | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/pgtype/hstore.go b/pgtype/hstore.go index 5ca39ecb..9befabd0 100644 --- a/pgtype/hstore.go +++ b/pgtype/hstore.go @@ -191,10 +191,8 @@ func (scanPlanBinaryHstoreToHstoreScanner) Scan(src []byte, dst any) error { rp += uint32Len hstore := make(Hstore, pairCount) - // one allocation for all strings, rather than one per string + // one allocation for all *string, rather than one per string, just like text parsing valueStrings := make([]string, pairCount) - // one shared string for all key/value strings - keyValueString := string(src[rp:]) for i := 0; i < pairCount; i++ { if len(src[rp:]) < uint32Len { @@ -206,7 +204,7 @@ func (scanPlanBinaryHstoreToHstoreScanner) Scan(src []byte, dst any) error { if len(src[rp:]) < keyLen { return fmt.Errorf("hstore incomplete %v", src) } - key := string(keyValueString[rp-uint32Len : rp-uint32Len+keyLen]) + key := string(src[rp : rp+keyLen]) rp += keyLen if len(src[rp:]) < uint32Len { @@ -216,7 +214,7 @@ func (scanPlanBinaryHstoreToHstoreScanner) Scan(src []byte, dst any) error { rp += 4 if valueLen >= 0 { - valueStrings[i] = string(keyValueString[rp-uint32Len : rp-uint32Len+valueLen]) + valueStrings[i] = string(src[rp : rp+valueLen]) rp += valueLen hstore[key] = &valueStrings[i]