From f4074ccca7dc5332e05c87f4b01b6b6d765736c8 Mon Sep 17 00:00:00 2001
From: Quentin McGaw <quentin.mcgaw@gmail.com>
Date: Wed, 9 Nov 2022 07:21:33 +0000
Subject: [PATCH] fix(trie): remove encoding buffers pool (#2929)

Trie node encoding sizes vary greatly from a few bytes to megabytes. Therefore using a `sync.Pool` of buffers is wrong and leads to a build up of memory usage. See the PR body for a more detailed explanation.
---
 internal/trie/node/branch_encode.go | 15 +--------------
 internal/trie/node/hash.go          | 12 ++----------
 internal/trie/pools/pools.go        |  9 ---------
 lib/trie/trie.go                    |  6 +-----
 4 files changed, 4 insertions(+), 38 deletions(-)

diff --git a/internal/trie/node/branch_encode.go b/internal/trie/node/branch_encode.go
index 9f2e48250b..78d5550243 100644
--- a/internal/trie/node/branch_encode.go
+++ b/internal/trie/node/branch_encode.go
@@ -9,7 +9,6 @@ import (
 	"io"
 	"runtime"
 
-	"github.com/ChainSafe/gossamer/internal/trie/pools"
 	"github.com/ChainSafe/gossamer/pkg/scale"
 )
 
@@ -21,11 +20,7 @@ type encodingAsyncResult struct {
 
 func runEncodeChild(child *Node, index int,
 	results chan<- encodingAsyncResult, rateLimit <-chan struct{}) {
-	buffer := pools.EncodingBuffers.Get().(*bytes.Buffer)
-	buffer.Reset()
-	// buffer is put back in the pool after processing its
-	// data in the select block below.
-
+	buffer := bytes.NewBuffer(nil)
 	err := encodeChild(child, buffer)
 
 	results <- encodingAsyncResult{
@@ -97,20 +92,12 @@ func encodeChildrenOpportunisticParallel(children []*Node, buffer io.Writer) (er
 				}
 			}
 
-			pools.EncodingBuffers.Put(resultBuffers[currentIndex])
 			resultBuffers[currentIndex] = nil
 
 			currentIndex++
 		}
 	}
 
-	for _, buffer := range resultBuffers {
-		if buffer == nil { // already emptied and put back in pool
-			continue
-		}
-		pools.EncodingBuffers.Put(buffer)
-	}
-
 	return err
 }
 
diff --git a/internal/trie/node/hash.go b/internal/trie/node/hash.go
index 8a97fa3f9f..2ea5806ead 100644
--- a/internal/trie/node/hash.go
+++ b/internal/trie/node/hash.go
@@ -145,21 +145,13 @@ func (n *Node) encodeIfNeeded() (encoding []byte, err error) {
 		return n.Encoding, nil // no need to copy
 	}
 
-	buffer := pools.EncodingBuffers.Get().(*bytes.Buffer)
-	buffer.Reset()
-	defer pools.EncodingBuffers.Put(buffer)
-
+	buffer := bytes.NewBuffer(nil)
 	err = n.Encode(buffer)
 	if err != nil {
 		return nil, fmt.Errorf("encoding: %w", err)
 	}
 
-	bufferBytes := buffer.Bytes()
-
-	// TODO remove this copying since it defeats the purpose of `buffer`
-	// and the sync.Pool.
-	n.Encoding = make([]byte, len(bufferBytes))
-	copy(n.Encoding, bufferBytes)
+	n.Encoding = buffer.Bytes()
 
 	return n.Encoding, nil // no need to copy
 }
diff --git a/internal/trie/pools/pools.go b/internal/trie/pools/pools.go
index 1bfe8f5a83..de95fc75f7 100644
--- a/internal/trie/pools/pools.go
+++ b/internal/trie/pools/pools.go
@@ -19,15 +19,6 @@ var DigestBuffers = &sync.Pool{
 	},
 }
 
-// EncodingBuffers is a sync pool of buffers of capacity 1.9MB.
-var EncodingBuffers = &sync.Pool{
-	New: func() interface{} {
-		const initialBufferCapacity = 1900000 // 1.9MB, from checking capacities at runtime
-		b := make([]byte, 0, initialBufferCapacity)
-		return bytes.NewBuffer(b)
-	},
-}
-
 // Hashers is a sync pool of blake2b 256 hashers.
 var Hashers = &sync.Pool{
 	New: func() interface{} {
diff --git a/lib/trie/trie.go b/lib/trie/trie.go
index 735d32c248..accf6e9bf9 100644
--- a/lib/trie/trie.go
+++ b/lib/trie/trie.go
@@ -9,7 +9,6 @@ import (
 
 	"github.com/ChainSafe/gossamer/internal/trie/codec"
 	"github.com/ChainSafe/gossamer/internal/trie/node"
-	"github.com/ChainSafe/gossamer/internal/trie/pools"
 	"github.com/ChainSafe/gossamer/lib/common"
 )
 
@@ -196,10 +195,7 @@ func (t *Trie) MustHash() common.Hash {
 
 // Hash returns the hashed root of the trie.
 func (t *Trie) Hash() (rootHash common.Hash, err error) {
-	buffer := pools.EncodingBuffers.Get().(*bytes.Buffer)
-	buffer.Reset()
-	defer pools.EncodingBuffers.Put(buffer)
-
+	buffer := bytes.NewBuffer(nil)
 	err = encodeRoot(t.root, buffer)
 	if err != nil {
 		return [32]byte{}, err