Skip to content

Conversation

@zhetaicheleba
Copy link
Contributor

strings.Builder has fewer memory allocations and better performance.
More info: golang/go#75190

Inspired by #17696 and replace all the case.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors string concatenation operations to use strings.Builder for improved performance and reduced memory allocations. The change replaces direct string concatenation (using +=) with strings.Builder methods across test code, compilation utilities, and runtime functions.

Key changes:

  • Replaced string concatenation with strings.Builder in loops and multiple append operations
  • Added strings import where needed
  • Changed return statements to use Builder.String() method

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
execution/vm/evm_test.go Converted two string concatenation patterns in test code to use strings.Builder
execution/vm/asm/compiler.go Refactored hex encoding loop to build binary string using strings.Builder
execution/protocol/block_exec.go Updated gas logging to use strings.Builder for transaction gas list formatting
execution/commitment/trie/debug.go Converted debug string formatting in fstring method to use strings.Builder
execution/abi/type.go Refactored tuple expression building to use strings.Builder for canonical parameter construction

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

for i, node := range &n.Children {
if node == nil {
resp += indices[i] + ": <nil> "
resp.WriteString(indices[i] + ": <nil> ")
Copy link

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

String concatenation (indices[i] + \": <nil> \") defeats the purpose of using strings.Builder. Use resp.WriteString(indices[i]) followed by resp.WriteString(\": <nil> \") to avoid intermediate string allocation.

Suggested change
resp.WriteString(indices[i] + ": <nil> ")
resp.WriteString(indices[i])
resp.WriteString(": <nil> ")

Copilot uses AI. Check for mistakes.
resp.WriteString(indices[i] + ": <nil> ")
} else {
resp += indices[i] + ": " + node.fstring(ind+" ")
resp.WriteString(indices[i] + ": " + node.fstring(ind+" "))
Copy link

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Multiple string concatenations (indices[i] + \": \" + node.fstring(...) and ind+\" \") create intermediate allocations. Split into separate WriteString calls: resp.WriteString(indices[i]), resp.WriteString(\": \"), resp.WriteString(node.fstring(ind+\" \")). Consider also passing a strings.Builder to fstring to avoid the inner concatenation ind+\" \".

Copilot uses AI. Check for mistakes.
@AskAlexSharov AskAlexSharov merged commit 7ab7c44 into erigontech:main Dec 11, 2025
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants