From b2dffed4b2c76c5dfa7c4bb4683030c9c8d2f022 Mon Sep 17 00:00:00 2001
From: Emmanuel T Odeke <emmanuel@orijtech.com>
Date: Thu, 17 Dec 2020 01:23:13 -0800
Subject: [PATCH] convertVarIntToBytes: use reusable bytes array (#352)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Noticed while auditing and profiling dependencies of cosmos-sdk, that
convertVarIntToBytes, while reusing already implemented code, it
was expensively creating a bytes.Buffer (40B on 64-bit architectures)
returning a result and discarding it, yet that code was called
3 times successively at least.

By reusing a byte array (not a slice, to ensure bounds checks
eliminations by the compiler), we are able to dramatically improve
performance, taking it from ~4µs down to 850ns (~4.5X reduction),
reduce allocations by >=~80% in every dimension:

```shell
$ benchstat before.txt after.txt
name             old time/op    new time/op    delta
ConvertLeafOp-8    3.90µs ± 1%    0.85µs ± 4%  -78.12%  (p=0.000 n=10+10)

name             old alloc/op   new alloc/op   delta
ConvertLeafOp-8    5.18kB ± 0%    0.77kB ± 0%  -85.19%  (p=0.000 n=10+10)

name             old allocs/op  new allocs/op  delta
ConvertLeafOp-8       120 ± 0%        24 ± 0%  -80.00%  (p=0.000 n=10+10)
```

Fixes #344
---
 encoding_test.go | 34 ++++++++++++++++++++++++++++++++++
 proof_ics23.go   | 28 +++++++++++++---------------
 2 files changed, 47 insertions(+), 15 deletions(-)

diff --git a/encoding_test.go b/encoding_test.go
index 6fc32374f..ee03d01f0 100644
--- a/encoding_test.go
+++ b/encoding_test.go
@@ -86,3 +86,37 @@ func TestDecodeBytes_invalidVarint(t *testing.T) {
 	_, _, err := decodeBytes([]byte{0xff})
 	require.Error(t, err)
 }
+
+// sink is kept as a global to ensure that value checks and assignments to it can't be
+// optimized away, and this will help us ensure that benchmarks successfully run.
+var sink interface{}
+
+func BenchmarkConvertLeafOp(b *testing.B) {
+	var versions = []int64{
+		0,
+		1,
+		100,
+		127,
+		128,
+		1 << 29,
+		-0,
+		-1,
+		-100,
+		-127,
+		-128,
+		-1 << 29,
+	}
+
+	b.ReportAllocs()
+	b.ResetTimer()
+
+	for i := 0; i < b.N; i++ {
+		for _, version := range versions {
+			sink = convertLeafOp(version)
+		}
+	}
+	if sink == nil {
+		b.Fatal("Benchmark wasn't run")
+	}
+	sink = nil
+}
diff --git a/proof_ics23.go b/proof_ics23.go
index 293270daa..c598b2c33 100644
--- a/proof_ics23.go
+++ b/proof_ics23.go
@@ -1,7 +1,7 @@
 package iavl
 
 import (
-	"bytes"
+	"encoding/binary"
 	"fmt"
 
 	ics23 "github.com/confio/ics23/go"
@@ -94,10 +94,11 @@ func convertExistenceProof(p *RangeProof, key, value []byte) (*ics23.ExistencePr
 }
 
 func convertLeafOp(version int64) *ics23.LeafOp {
+	var varintBuf [binary.MaxVarintLen64]byte
 	// this is adapted from iavl/proof.go:proofLeafNode.Hash()
-	prefix := convertVarIntToBytes(0)
-	prefix = append(prefix, convertVarIntToBytes(1)...)
-	prefix = append(prefix, convertVarIntToBytes(version)...)
+	prefix := convertVarIntToBytes(0, varintBuf)
+	prefix = append(prefix, convertVarIntToBytes(1, varintBuf)...)
+	prefix = append(prefix, convertVarIntToBytes(version, varintBuf)...)
 
 	return &ics23.LeafOp{
 		Hash:         ics23.HashOp_SHA256,
@@ -114,13 +115,15 @@ func convertInnerOps(path PathToLeaf) []*ics23.InnerOp {
 	// lengthByte is the length prefix prepended to each of the sha256 sub-hashes
 	var lengthByte byte = 0x20
 
+	var varintBuf [binary.MaxVarintLen64]byte
+
 	// we need to go in reverse order, iavl starts from root to leaf,
 	// we want to go up from the leaf to the root
 	for i := len(path) - 1; i >= 0; i-- {
 		// this is adapted from iavl/proof.go:proofInnerNode.Hash()
-		prefix := convertVarIntToBytes(int64(path[i].Height))
-		prefix = append(prefix, convertVarIntToBytes(path[i].Size)...)
-		prefix = append(prefix, convertVarIntToBytes(path[i].Version)...)
+		prefix := convertVarIntToBytes(int64(path[i].Height), varintBuf)
+		prefix = append(prefix, convertVarIntToBytes(path[i].Size, varintBuf)...)
+		prefix = append(prefix, convertVarIntToBytes(path[i].Version, varintBuf)...)
 
 		var suffix []byte
 		if len(path[i].Left) > 0 {
@@ -147,12 +150,7 @@ func convertInnerOps(path PathToLeaf) []*ics23.InnerOp {
 	return steps
 }
 
-func convertVarIntToBytes(orig int64) []byte {
-	buf := new(bytes.Buffer)
-	err := encodeVarint(buf, orig)
-	// write should not fail
-	if err != nil {
-		panic(err)
-	}
-	return buf.Bytes()
+func convertVarIntToBytes(orig int64, buf [binary.MaxVarintLen64]byte) []byte {
+	n := binary.PutVarint(buf[:], orig)
+	return buf[:n]
 }