New nodes map with capacity

[mask-pp]

• change before:
  BenchmarkMerge/1K-12 4980 224241 ns/op
  BenchmarkMerge/10K-12 470 2482620 ns/op

• change after:
  BenchmarkMerge/1K-12 6000 186740 ns/op
  BenchmarkMerge/10K-12 574 2000180 ns/op

[karalabe]

I’n not following, the before values are *faster* than the after values.

…

On Sat, 4 May 2024 at 04:19, maskpp ***@***.***> wrote: @mask-pp <https://github.com/mask-pp> requested your review on: #29704 <#29704> New nodes map with capacity as a code owner. — Reply to this email directly, view it on GitHub <#29704 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAA7UGLCYCRPDXE4PZJ4YZTZARAUVAVCNFSM6AAAAABHGNS7UKVHI2DSMVQWIX3LMV45UABCJFZXG5LFIV3GK3TUJZXXI2LGNFRWC5DJN5XDWMJSG4YDEOBSHEYTCOA> . You are receiving this because your review was requested.Message ID: ***@***.***>

[mask-pp]

I’n not following, the before values are faster than the after values.
…
On Sat, 4 May 2024 at 04:19, maskpp @.> wrote: @mask-pp https://github.com/mask-pp requested your review on: #29704 <#29704> New nodes map with capacity as a code owner. — Reply to this email directly, view it on GitHub <#29704 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAA7UGLCYCRPDXE4PZJ4YZTZARAUVAVCNFSM6AAAAABHGNS7UKVHI2DSMVQWIX3LMV45UABCJFZXG5LFIV3GK3TUJZXXI2LGNFRWC5DJN5XDWMJSG4YDEOBSHEYTCOA . You are receiving this because your review was requested.Message ID: @.>

@karalabe Sorry, it's a mistake. The conversation content is changed.

[rjl493456442]

I don't think this change is worthwhile.

Technically, it's recommended to pre-allocate a slice with a capacity if the slice size can be known in advance. However, the number of nodes for committing is unknown in merkle tree unless we introduce a counter in Trie.

Besides, the performance difference is actually very tiny.

func BenchmarkAddNode(b *testing.B) {
	b.Run("1K-no-alloc", func(b *testing.B) {
		benchmarkAddNode(b, 1000, false)
	})
	b.Run("10K-no-alloc", func(b *testing.B) {
		benchmarkAddNode(b, 10_000, false)
	})
	b.Run("1K-alloc", func(b *testing.B) {
		benchmarkAddNode(b, 1000, true)
	})
	b.Run("10K-alloc", func(b *testing.B) {
		benchmarkAddNode(b, 10_000, true)
	})
}

func benchmarkAddNode(b *testing.B, count int, alloc bool) {
	var (
		paths [][]byte
		node  = New(crypto.Keccak256Hash([]byte("deadbeef")), []byte("deadbeef"))
	)
	for i := 0; i < count; i++ {
		path := make([]byte, 4)
		rand.Read(path)
		paths = append(paths, path)
	}
	b.ResetTimer()

	for i := 0; i < b.N; i++ {
		var set *NodeSet
		if alloc {
			set = NewNodeSetWithCap(common.Hash{}, count)
		} else {
			set = NewNodeSet(common.Hash{})
		}
		for j := 0; j < count; j++ {
			set.AddNode(paths[j], node)
		}
	}
}

goos: darwin
goarch: arm64
pkg: github.com/ethereum/go-ethereum/trie/trienode
BenchmarkAddNode
BenchmarkAddNode/1K-no-alloc
BenchmarkAddNode/1K-no-alloc-8         	   16284	     71414 ns/op
BenchmarkAddNode/10K-no-alloc
BenchmarkAddNode/10K-no-alloc-8        	    1634	    720144 ns/op
BenchmarkAddNode/1K-alloc
BenchmarkAddNode/1K-alloc-8            	   32475	     37056 ns/op
BenchmarkAddNode/10K-alloc
BenchmarkAddNode/10K-alloc-8           	    2940	    408725 ns/op

For sure, the node insertion is about 2x faster. Given that we do trie commit in parallel and the slowest one should be account trie. Let's assume 5K dirty nodes in account trie, then the performance gain is less than 0.3ms.

All in all, the idea is good, but the complexity(maintaining a counter in trie) is not worthwhile compare with the little performance gain.