Improve performance of answerGetBlockBodiesQuery (#2198)

Avoid decoding and re-encoding of the block body by using available
RLP-encoded block body from the database, combine it with the RLP-encoded
block hash to avoid unnecessary processing and extra memory allocations.

eth/protocols/eth/handler_test.go

@@ -17,6 +17,7 @@
 package eth
 
 import (
+	"fmt"
 	"math"
 	"math/big"
 	"math/rand"
@@ -536,3 +537,39 @@ func testGetBlockReceipts(t *testing.T, protocol uint) {
 		t.Errorf("receipts mismatch: %v", err)
 	}
 }
+
+func BenchmarkAnswerGetBlockBodiesQuery(b *testing.B) {
+	backend := newTestBackend(maxBodiesServe + 15)
+	defer backend.close()
+
+	for _, n_blocks := range []int{1, 10, 100} {
+		// Collect the hashes to request, and the response to expect
+		var (
+			hashes []common.Hash
+			seen   = make(map[int64]bool)
+		)
+
+		rand.Seed(0)
+
+		for {
+			num := rand.Int63n(int64(backend.chain.CurrentBlock().NumberU64()))
+
+			if !seen[num] {
+				seen[num] = true
+
+				block := backend.chain.GetBlockByNumber(uint64(num))
+				hashes = append(hashes, block.Hash())
+
+				if len(hashes) >= n_blocks {
+					name := fmt.Sprintf("GetBlockBodies-%d", n_blocks)
+					b.Run(name, func(b *testing.B) {
+						for n := 0; n < b.N; n++ {
+							answerGetBlockBodiesQuery(backend, GetBlockBodiesPacket(hashes), nil)
+						}
+					})
+					break
+				}
+			}
+		}
+	}
+}

eth/protocols/eth/handlers.go

@@ -153,14 +153,15 @@ func answerGetBlockBodiesQuery(backend Backend, query GetBlockBodiesPacket, peer
 			lookups >= 2*maxBodiesServe {
 			break
 		}
-		// Retrieve the requested block body, stopping if enough was found
-		if body := backend.Chain().GetBody(hash); body != nil {
-			bh := &blockBodyWithBlockHash{BlockHash: hash, BlockBody: body}
-			bhRLPbytes, err := rlp.EncodeToBytes(bh)
+		// Retrieve the requested block body RLP, stopping if enough was found
+		if body := backend.Chain().GetBodyRLP(hash); len(body) != 0 {
+			hashRLP, err := rlp.EncodeToBytes(hash)
+
 			if err != nil {
 				return nil, err
 			}
-			bhRLP := rlp.RawValue(bhRLPbytes)
+			bhRLP := rlp.Combine(hashRLP, body)
+
 			bodiesAndBlockHashes = append(bodiesAndBlockHashes, bhRLP)
 			bytes += len(bhRLP)
 		}

rlp/celo_raw.go

@@ -0,0 +1,58 @@
+package rlp
+
+import (
+	"encoding/binary"
+	"math"
+)
+
+// for payloads longer than 55 bytes for both strings and lists additional prefix signalizing
+// the total length of the payload is needed
+const payloadLengthThreshold = 0x37 // 55 in dec
+
+// indicates that the payload is a list
+const shortListEncodingByte = 0xC0
+
+// listEncodingByte (0xC0) + payloadLengthThreshold (0x37) = 0xF7
+const longListEncodingByte = 0xF7
+
+// Combine takes two RLP-encoded values one and two and produces a combined one
+// as if it was an RLP encoding of the [one, two] list
+// based on https://ethereum.org/en/developers/docs/data-structures-and-encoding/rlp/
+func Combine(one []byte, two []byte) []byte {
+	payloadLen := len(one) + len(two)
+	result := make([]byte, 0, ListSize(uint64(payloadLen)))
+
+	if payloadLen <= payloadLengthThreshold {
+		// If the total payload of a list (i.e. the combined length of all its items being RLP
+		// encoded) is 0-55 bytes long, the RLP encoding consists of a single byte with value
+		// 0xc0 plus the length of the payload followed by the concatenation of the RLP encodings
+		// of the items.
+		result = append(result, byte(shortListEncodingByte+payloadLen))
+	} else {
+		encodedPayloadLength := binaryEncode(uint64(payloadLen))
+
+		// If the total payload of a list is more than 55 bytes long, the RLP encoding consists
+		// of a single byte with value 0xf7 plus the length in bytes of the length of the payload
+		// in binary form, followed by the length of the payload, followed by the concatenation
+		// of the RLP encodings of the items.
+		result = append(result, byte(longListEncodingByte+len(encodedPayloadLength)))
+		result = append(result, encodedPayloadLength...)
+	}
+
+	result = append(result, one...)
+	result = append(result, two...)
+
+	return result
+}
+
+// binaryEncode returns a binary-encoded number without leading zeroes
+func binaryEncode(number uint64) []byte {
+	binaryEncoded := make([]byte, 8)
+
+	binary.BigEndian.PutUint64(binaryEncoded, number)
+
+	// we need to +1 as logarithm works for positive numbers
+	lengthInBytes := uint(math.Ceil(math.Log2(float64(number+1)) / 8))
+
+	return binaryEncoded[8-lengthInBytes:]
+}

rlp/celo_raw_test.go

@@ -0,0 +1,92 @@
+package rlp
+
+import (
+	"reflect"
+	"testing"
+)
+
+func TestCombine(t *testing.T) {
+	one_str, _ := EncodeToBytes("Hello")
+	two_str, _ := EncodeToBytes("World")
+	list_of_strs, _ := EncodeToBytes([]string{"Hello", "World"})
+
+	one_list, _ := EncodeToBytes([]string{"1"})
+	two_list, _ := EncodeToBytes([]string{"2"})
+	list_of_lists, _ := EncodeToBytes([][]string{{"1"}, {"2"}})
+
+	str_and_list, _ := EncodeToBytes([]interface{}{"Hello", []string{"2"}})
+	list_and_str, _ := EncodeToBytes([]interface{}{[]string{"1"}, "World"})
+
+	empty_list, _ := EncodeToBytes([]byte{})
+	str_and_empty_list, _ := EncodeToBytes([]interface{}{"Hello", []byte{}})
+	empty_list_and_str, _ := EncodeToBytes([]interface{}{[]byte{}, "World"})
+	two_empty_lists, _ := EncodeToBytes([]interface{}{[]byte{}, []byte{}})
+
+	empty_str, _ := EncodeToBytes("")
+	two_empty_str, _ := EncodeToBytes([]string{"", ""})
+
+	testCases := []struct {
+		name           string
+		one            []byte
+		two            []byte
+		expectedResult []byte
+	}{
+		{
+			name:           "Two strings",
+			one:            one_str,
+			two:            two_str,
+			expectedResult: list_of_strs,
+		},
+		{
+			name:           "Two empty strings",
+			one:            empty_str,
+			two:            empty_str,
+			expectedResult: two_empty_str,
+		},
+		{
+			name:           "Two lists",
+			one:            one_list,
+			two:            two_list,
+			expectedResult: list_of_lists,
+		},
+		{
+			name:           "Two empty lists",
+			one:            empty_list,
+			two:            empty_list,
+			expectedResult: two_empty_lists,
+		},
+		{
+			name:           "String and list",
+			one:            one_str,
+			two:            two_list,
+			expectedResult: str_and_list,
+		},
+		{
+			name:           "String and empty list",
+			one:            one_str,
+			two:            empty_list,
+			expectedResult: str_and_empty_list,
+		},
+		{
+			name:           "List and string",
+			one:            one_list,
+			two:            two_str,
+			expectedResult: list_and_str,
+		},
+		{
+			name:           "Empty list and string",
+			one:            empty_list,
+			two:            two_str,
+			expectedResult: empty_list_and_str,
+		},
+	}
+
+	for _, c := range testCases {
+		t.Run(c.name, func(t *testing.T) {
+			result := Combine(c.one, c.two)
+			if !reflect.DeepEqual(result, c.expectedResult) {
+				t.Error("Expected", c.expectedResult, "got", result)
+			}
+		})
+	}
+}