From ae9c5b1ca0c77ea7203bbfa4f1cd68cd6d480e17 Mon Sep 17 00:00:00 2001
From: Emmanuel Odeke <emm.odeke@gmail.com>
Date: Wed, 26 Jul 2017 20:43:27 -0600
Subject: [PATCH] hd: optimize ReverseBytes + add tests
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* Optimized ReverseBytes to:
a) Minimally allocate --> 60.0% reduction in the number of allocations
b) Only walk halfway the length of the string thus performing
byte swaps from left to right. Improves the performance as well.
Complexity is O(n/2) instead of O(n) which is still O(n) but
benchmarks show the new time is in deed 1/2 of the original time.

* Added unit tests and some common cases to ensure correctness.

* Benchmark shoot out results:
```shell
name            old time/op    new time/op    delta
ReverseBytes-4     554ns ± 4%     242ns ± 3%  -56.20%  (p=0.000 n=10+10)

name            old alloc/op   new alloc/op   delta
ReverseBytes-4      208B ± 0%      114B ± 0%  -45.19%  (p=0.000 n=10+10)

name            old allocs/op  new allocs/op  delta
ReverseBytes-4      10.0 ± 0%       4.0 ± 0%  -60.00%  (p=0.000 n=10+10)
```
---
 hd/address.go | 17 ++++++++++++++---
 hd/hd_test.go | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 67 insertions(+), 3 deletions(-)

diff --git a/hd/address.go b/hd/address.go
index 28f70a98f..d7553a4d7 100644
--- a/hd/address.go
+++ b/hd/address.go
@@ -282,9 +282,20 @@ func CalcSha512(buf []byte) []byte {
 }
 
 func ReverseBytes(buf []byte) []byte {
-	res := []byte{}
-	for i := len(buf) - 1; i >= 0; i-- {
-		res = append(res, buf[i])
+	var res []byte
+	if len(buf) == 0 {
+		return res
+	}
+
+	// Walk till mid-way, swapping bytes from each end:
+	// b[i] and b[len-i-1]
+	blen := len(buf)
+	res = make([]byte, blen)
+	mid := blen / 2
+	for left := 0; left <= mid; left++ {
+		right := blen - left - 1
+		res[left] = buf[right]
+		res[right] = buf[left]
 	}
 	return res
 }
diff --git a/hd/hd_test.go b/hd/hd_test.go
index e771f14b6..b2f7d2e8f 100644
--- a/hd/hd_test.go
+++ b/hd/hd_test.go
@@ -89,6 +89,26 @@ func TestHDToAddr(t *testing.T) {
 	}
 }
 
+func TestReverseBytes(t *testing.T) {
+	tests := [...]struct {
+		v    []byte
+		want []byte
+	}{
+		{[]byte(""), []byte("")},
+		{nil, nil},
+		{[]byte("Tendermint"), []byte("tnimredneT")},
+		{[]byte("T"), []byte("T")},
+		{[]byte("Te"), []byte("eT")},
+	}
+
+	for i, tt := range tests {
+		got := ReverseBytes(tt.v)
+		if !bytes.Equal(got, tt.want) {
+			t.Errorf("#%d:\ngot= (%x)\nwant=(%x)", i, got, tt.want)
+		}
+	}
+}
+
 func ifExit(err error, n int) {
 	if err != nil {
 		fmt.Println(n, err)
@@ -187,3 +207,36 @@ func tylerSmith(seed []byte) ([]byte, []byte, []byte) {
 	pub := k.PublicKey().Key
 	return masterKey.Key, priv, pub
 }
+
+// Benchmarks
+
+var revBytesCases = [][]byte{
+	nil,
+	[]byte(""),
+
+	[]byte("12"),
+
+	// 16byte case
+	[]byte("abcdefghijklmnop"),
+
+	// 32byte case
+	[]byte("abcdefghijklmnopqrstuvwxyz123456"),
+
+	// 64byte case
+	[]byte("abcdefghijklmnopqrstuvwxyz123456abcdefghijklmnopqrstuvwxyz123456"),
+}
+
+func BenchmarkReverseBytes(b *testing.B) {
+	var sink []byte
+	for i := 0; i < b.N; i++ {
+		for _, tt := range revBytesCases {
+			sink = ReverseBytes(tt)
+		}
+	}
+	b.ReportAllocs()
+
+	// sink is necessary to ensure if the compiler tries
+	// to smart, that it won't optimize away the benchmarks.
+	if sink != nil {
+	}
+}