rpcclient: add GetTxOutProof{,Async} methods, speed up string parsing

[guggero]

Adds a new method to fetch transaction output inclusion proofs.

While observing this code and specifically the GetBlock method on mainnet, I observed a lot of JSON-related memory allocations for basically turning "<hex_string>" into <hex_string>. So I changed those JSON parse calls with simple prefix/suffix string replacements, which should hopefully be much faster and require fewer allocations.

[Roasbeef]

Changes look good. Two main comments:

1. Extract to helper function
2. Tests for the expected output

[guggero]

Thanks for the review! I guess I was a bit lazy with this PR... Addressed your comments.

[kcalvinalvin]

Looks good. dd9b5e7 Also tested with a simple benchmark:

+func BenchmarkParseJSONString(b *testing.B) {
+       // Simulate a ~1 MiB serialized block hex-encoded and JSON-quoted,
+       // matching what bitcoind's getblock verbosity=0 returns.
+       raw := make([]byte, 1024*1024)
+       rand.New(rand.NewSource(1)).Read(raw)
+       payload := []byte(`"` + hex.EncodeToString(raw) + `"`)
+
+       b.Run("unmarshal", func(b *testing.B) {
+               b.ReportAllocs()
+               for i := 0; i < b.N; i++ {
+                       var s string
+                       if err := json.Unmarshal(payload, &s); err != nil {
+                               b.Fatal(err)
+                       }
+               }
+       })
+
+       b.Run("parseJSONString", func(b *testing.B) {
+               b.ReportAllocs()
+               for i := 0; i < b.N; i++ {
+                       _ = parseJSONString(payload)
+               }
+       })
+}
+

It's way faster:

[I] calvin@nixos ~/b/b/btcd ((dd9b5e72))> go test -bench=BenchmarkParseJSONString -benchmem -run=XXX ./rpcclient/
goos: linux
goarch: amd64
pkg: github.com/btcsuite/btcd/rpcclient
cpu: AMD Ryzen 9 7950X3D 16-Core Processor
BenchmarkParseJSONString/unmarshal-32                200           5950644 ns/op         2097328 B/op          3 allocs/op
BenchmarkParseJSONString/parseJSONString-32         5748            237475 ns/op         2105376 B/op          1 allocs/op
PASS
ok      github.com/btcsuite/btcd/rpcclient      3.194s

[guibressan]

Nice work @guggero

The cast from []byte to string does a single allocation, using bytes.TrimPrefix
and bytes.TrimSuffix enables us to do zero allocations in this procedure.

The previous optimization gave us 49x speedup

My suggestion enables 1770330x speedup

goos: linux
goarch: amd64
pkg: github.com/btcsuite/btcd/rpcclient
cpu: Intel(R) Core(TM) i5-3337U CPU @ 1.80GHz
BenchmarkParseJSONString/unmarshal-4                  50          29741572 ns/op         2097312 B/op          3 allocs/op
BenchmarkParseJSONString/parseJSONString-4          1844            587827 ns/op         2105351 B/op          1 allocs/op
BenchmarkParseJSONString/parseJSONString2-4     74190529                16.80 ns/op            0 B/op          0 allocs/op
PASS

// parseJSONString2 parses a JSON byte slice as a JSON string by removing leading
// and trailing double quotes.
func parseJSONString2(jsonWithQuotes []byte) []byte {
	// The result is just a single hex string. So we don't need to unmarshal
	// it into a string, replacing the quotes achieves the same result, just
	// much faster and with fewer allocations.
	return bytes.TrimPrefix(
		bytes.TrimSuffix(jsonWithQuotes, []byte("\"")), []byte("\""),
	)
}

The bench:

func BenchmarkParseJSONString(b *testing.B) {
       // Simulate a ~1 MiB serialized block hex-encoded and JSON-quoted,
       // matching what bitcoind's getblock verbosity=0 returns.
       raw := make([]byte, 1024*1024)
       rand.New(rand.NewSource(1)).Read(raw)
       payload := []byte(`"` + hex.EncodeToString(raw) + `"`)

       b.Run("unmarshal", func(b *testing.B) {
               b.ReportAllocs()
               for i := 0; i < b.N; i++ {
                       var s string
                       if err := json.Unmarshal(payload, &s); err != nil {
                               b.Fatal(err)
                       }
               }
       })

       b.Run("parseJSONString", func(b *testing.B) {
               b.ReportAllocs()
               for i := 0; i < b.N; i++ {
                       _ = parseJSONString(payload)
               }
       })

       b.Run("parseJSONString2", func(b *testing.B) {
               b.ReportAllocs()
               for i := 0; i < b.N; i++ {
                       _ = parseJSONString2(payload)
               }
       })
}

Full diff:

diff --git a/rpcclient/chain.go b/rpcclient/chain.go
index bb85284a..c0d60cfa 100644
--- a/rpcclient/chain.go
+++ b/rpcclient/chain.go
@@ -6,9 +6,9 @@
 package rpcclient

 import (
+       "bytes"
        "encoding/hex"
        "encoding/json"
-       "strings"

        "github.com/btcsuite/btcd/btcjson"
        "github.com/btcsuite/btcd/chaincfg/chainhash"
@@ -27,7 +27,7 @@ func (r FutureGetBestBlockHashResult) Receive() (*chainhash.Hash, error) {
                return nil, err
        }

-       return chainhash.NewHashFromStr(parseJSONString(res))
+       return chainhash.NewHash(parseJSONString(res))
 }

 // GetBestBlockHashAsync returns an instance of a type that can be used to get
@@ -107,7 +107,7 @@ func (r FutureGetBlockResult) Receive() (*wire.MsgBlock, error) {

        // Deserialize the block and return it.
        var msgBlock wire.MsgBlock
-       err = msgBlock.Deserialize(hex.NewDecoder(strings.NewReader(
+       err = msgBlock.Deserialize(hex.NewDecoder(bytes.NewReader(
                parseJSONString(res),
        )))
        if err != nil {
@@ -541,7 +541,7 @@ func (r FutureGetBlockHashResult) Receive() (*chainhash.Hash, error) {
                return nil, err
        }

-       return chainhash.NewHashFromStr(parseJSONString(res))
+       return chainhash.NewHash(parseJSONString(res))
 }

 // GetBlockHashAsync returns an instance of a type that can be used to get the
@@ -574,7 +574,7 @@ func (r FutureGetBlockHeaderResult) Receive() (*wire.BlockHeader, error) {

        // Deserialize the blockheader and return it.
        var bh wire.BlockHeader
-       err = bh.Deserialize(hex.NewDecoder(strings.NewReader(
+       err = bh.Deserialize(hex.NewDecoder(bytes.NewReader(
                parseJSONString(res),
        )))
        if err != nil {
@@ -1083,7 +1083,7 @@ func (r FutureGetTxOutProofResult) Receive() (*wire.MsgMerkleBlock, error) {

        var merkleBlock wire.MsgMerkleBlock
        err = merkleBlock.BtcDecode(
-               hex.NewDecoder(strings.NewReader(parseJSONString(res))),
+               hex.NewDecoder(bytes.NewReader(parseJSONString(res))),
                wire.ProtocolVersion, wire.WitnessEncoding,
        )
        if err != nil {
@@ -1212,7 +1212,7 @@ func (r FutureGetCFilterResult) Receive() (*wire.MsgCFilter, error) {
        }

        // Decode the serialized cf hex to raw bytes.
-       serializedFilter, err := hex.DecodeString(parseJSONString(res))
+       serializedFilter, err := hex.AppendDecode(nil, parseJSONString(res))
        if err != nil {
                return nil, err
        }
@@ -1260,7 +1260,7 @@ func (r FutureGetCFilterHeaderResult) Receive() (*wire.MsgCFHeaders, error) {
        }

        // Assign the decoded header into a hash
-       headerHash, err := chainhash.NewHashFromStr(parseJSONString(res))
+       headerHash, err := chainhash.NewHash(parseJSONString(res))
        if err != nil {
                return nil, err
        }
@@ -1454,11 +1454,12 @@ func (c *Client) ReconsiderBlock(blockHash *chainhash.Hash) error {

 // parseJSONString parses a JSON byte slice as a JSON string by removing leading
 // and trailing double quotes.
-func parseJSONString(jsonWithQuotes []byte) string {
+func parseJSONString(jsonWithQuotes []byte) []byte {
        // The result is just a single hex string. So we don't need to unmarshal
        // it into a string, replacing the quotes achieves the same result, just
        // much faster and with fewer allocations.
-       return strings.TrimPrefix(
-               strings.TrimSuffix(string(jsonWithQuotes), "\""), "\"",
+       return bytes.TrimPrefix(
+               bytes.TrimSuffix(jsonWithQuotes, []byte("\"")), []byte("\""),
        )
 }
+
diff --git a/rpcclient/chain_test.go b/rpcclient/chain_test.go
index b37bb24c..4016e6fa 100644
--- a/rpcclient/chain_test.go
+++ b/rpcclient/chain_test.go
@@ -1,6 +1,7 @@
 package rpcclient

 import (
+       "bytes"
        "encoding/json"
        "errors"
        "net/http"
@@ -274,7 +275,7 @@ func TestJSONStringParsing(t *testing.T) {

                return resultStr, nil
        }
-       newMethod := func(res []byte) (string, error) {
+       newMethod := func(res []byte) ([]byte, error) {
                return parseJSONString(res), nil
        }

@@ -286,7 +287,10 @@ func TestJSONStringParsing(t *testing.T) {
                        newResult, err := newMethod([]byte(testCase))
                        require.NoError(t, err)

-                       require.Equal(t, oldResult, newResult)
+                       if !bytes.Equal([]byte(oldResult), newResult) {
+                               t.Errorf("\nexpected %s\ngot %s", oldResult,
+                                       newResult)
+                       }
                })
        }
 }

[guggero]

Thanks @guibressan, that's a good point. I've added a parseJSONStringReader function that does more or less what you suggested. I'm using it for the places where it really matters (when we parse a full block or block header).

For the other occurrences, your code actually subtly breaks everything, because there is a semantic difference between chainhash.NewHashFromStr() and chainhash.NewHash() (one reverses the bytes, the other one doesn't).
So I left those with the initial method.

[guibressan]

Nice. In fact, the tests caught my mistake about this semantic difference. But now all the tests are passing.