perf: use strings.Builder #1571
Conversation
WalkthroughThe changes in this pull request focus on modifications to the Changes
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
bridge-history-api/internal/utils/utils.go (1)
175-182: Consider preallocating builder capacity for large arraysWhile the current implementation is good, for very large arrays, you could further optimize by preallocating the builder's capacity.
Here's a suggested enhancement:
func ConvertBigIntArrayToString(array []*big.Int) string { var sb strings.Builder + // Preallocate capacity (assuming ~20 chars per number plus separator) + sb.Grow(len(array) * 22) for i, num := range array { if i > 0 { sb.WriteString(", ") } sb.WriteString(num.String()) } return sb.String() }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
bridge-history-api/internal/utils/utils.go(1 hunks)
🔇 Additional comments (1)
bridge-history-api/internal/utils/utils.go (1)
175-182: LGTM! Efficient use of strings.Builder
The optimization from string concatenation to strings.Builder is a good improvement that reduces memory allocations and improves performance.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
bridge-history-api/internal/utils/utils.go (1)
175-184: LGTM! Efficient implementation using strings.BuilderThe optimization using
strings.Builderwith preallocated capacity is a good performance improvement over string concatenation. The implementation correctly handles all edge cases and uses the appropriate methods.Consider making the capacity calculation more precise:
- sb.Grow(len(array) * 22) + // Preallocate capacity: 20 chars per number + 2 chars for ", " separator + sb.Grow(len(array) * 20 + (len(array) - 1) * 2)This adjustment:
- Allocates exactly 20 chars per number
- Accounts for separators only between numbers (len-1 times)
- Provides a more accurate capacity estimation
| @@ -172,13 +172,16 @@ func GetBlocksInRange(ctx context.Context, cli *ethclient.Client, start, end uin | |||
|
|
|||
| // ConvertBigIntArrayToString convert the big int array to string | |||
| func ConvertBigIntArrayToString(array []*big.Int) string { | |||
There was a problem hiding this comment.
can you add unit test for your changes?
There was a problem hiding this comment.
I only modified the internal structure of the function and ran the TestConvertBigIntArrayToString test. The result passed successfully.
In the ConvertBigIntArrayToString function, the big.Int is first converted to a string and then concatenated with commas. This can be optimized using strings.Builder, which avoids creating new string objects on each concatenation. The performance advantage is particularly significant when dealing with larger arrays.
In the ConvertBigIntArrayToString function, the big.Int is first converted to a string and then concatenated with commas. This can be optimized using strings.Builder, which avoids creating new string objects on each concatenation. The performance advantage is particularly significant when dealing with larger arrays.
Purpose or design rationale of this PR
Describe your change. Make sure to answer these three questions: What does this PR do? Why does it do it? How does it do it?
PR title
Your PR title must follow conventional commits (as we are doing squash merge for each PR), so it must start with one of the following types:
Deployment tag versioning
Has
tagincommon/version.gobeen updated or have you addedbump-versionlabel to this PR?Breaking change label
Does this PR have the
breaking-changelabel?Summary by CodeRabbit
Bug Fixes
Performance Improvements