Skip to content

Commit f4074cc

Browse files
authored
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.
1 parent 7131290 commit f4074cc

File tree

4 files changed

+4
-38
lines changed

4 files changed

+4
-38
lines changed

internal/trie/node/branch_encode.go

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import (
99
"io"
1010
"runtime"
1111

12-
"github.com/ChainSafe/gossamer/internal/trie/pools"
1312
"github.com/ChainSafe/gossamer/pkg/scale"
1413
)
1514

@@ -21,11 +20,7 @@ type encodingAsyncResult struct {
2120

2221
func runEncodeChild(child *Node, index int,
2322
results chan<- encodingAsyncResult, rateLimit <-chan struct{}) {
24-
buffer := pools.EncodingBuffers.Get().(*bytes.Buffer)
25-
buffer.Reset()
26-
// buffer is put back in the pool after processing its
27-
// data in the select block below.
28-
23+
buffer := bytes.NewBuffer(nil)
2924
err := encodeChild(child, buffer)
3025

3126
results <- encodingAsyncResult{
@@ -97,20 +92,12 @@ func encodeChildrenOpportunisticParallel(children []*Node, buffer io.Writer) (er
9792
}
9893
}
9994

100-
pools.EncodingBuffers.Put(resultBuffers[currentIndex])
10195
resultBuffers[currentIndex] = nil
10296

10397
currentIndex++
10498
}
10599
}
106100

107-
for _, buffer := range resultBuffers {
108-
if buffer == nil { // already emptied and put back in pool
109-
continue
110-
}
111-
pools.EncodingBuffers.Put(buffer)
112-
}
113-
114101
return err
115102
}
116103

internal/trie/node/hash.go

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -145,21 +145,13 @@ func (n *Node) encodeIfNeeded() (encoding []byte, err error) {
145145
return n.Encoding, nil // no need to copy
146146
}
147147

148-
buffer := pools.EncodingBuffers.Get().(*bytes.Buffer)
149-
buffer.Reset()
150-
defer pools.EncodingBuffers.Put(buffer)
151-
148+
buffer := bytes.NewBuffer(nil)
152149
err = n.Encode(buffer)
153150
if err != nil {
154151
return nil, fmt.Errorf("encoding: %w", err)
155152
}
156153

157-
bufferBytes := buffer.Bytes()
158-
159-
// TODO remove this copying since it defeats the purpose of `buffer`
160-
// and the sync.Pool.
161-
n.Encoding = make([]byte, len(bufferBytes))
162-
copy(n.Encoding, bufferBytes)
154+
n.Encoding = buffer.Bytes()
163155

164156
return n.Encoding, nil // no need to copy
165157
}

internal/trie/pools/pools.go

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -19,15 +19,6 @@ var DigestBuffers = &sync.Pool{
1919
},
2020
}
2121

22-
// EncodingBuffers is a sync pool of buffers of capacity 1.9MB.
23-
var EncodingBuffers = &sync.Pool{
24-
New: func() interface{} {
25-
const initialBufferCapacity = 1900000 // 1.9MB, from checking capacities at runtime
26-
b := make([]byte, 0, initialBufferCapacity)
27-
return bytes.NewBuffer(b)
28-
},
29-
}
30-
3122
// Hashers is a sync pool of blake2b 256 hashers.
3223
var Hashers = &sync.Pool{
3324
New: func() interface{} {

lib/trie/trie.go

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import (
99

1010
"github.com/ChainSafe/gossamer/internal/trie/codec"
1111
"github.com/ChainSafe/gossamer/internal/trie/node"
12-
"github.com/ChainSafe/gossamer/internal/trie/pools"
1312
"github.com/ChainSafe/gossamer/lib/common"
1413
)
1514

@@ -196,10 +195,7 @@ func (t *Trie) MustHash() common.Hash {
196195

197196
// Hash returns the hashed root of the trie.
198197
func (t *Trie) Hash() (rootHash common.Hash, err error) {
199-
buffer := pools.EncodingBuffers.Get().(*bytes.Buffer)
200-
buffer.Reset()
201-
defer pools.EncodingBuffers.Put(buffer)
202-
198+
buffer := bytes.NewBuffer(nil)
203199
err = encodeRoot(t.root, buffer)
204200
if err != nil {
205201
return [32]byte{}, err

0 commit comments

Comments
 (0)