perf: preallocate slice size in Client::Param()#3782
perf: preallocate slice size in Client::Param()#3782gaby merged 3 commits intogofiber:mainfrom arturmelanchyk:slice-prealloc
Conversation
WalkthroughPreallocate result slices in three client functions to set capacity from the number of retrieved values before appending, without changing return values or control flow. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3782 +/- ##
==========================================
- Coverage 91.67% 91.62% -0.06%
==========================================
Files 113 113
Lines 11959 11959
==========================================
- Hits 10964 10958 -6
- Misses 731 736 +5
- Partials 264 265 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Signed-off-by: Artur Melanchyk <13834276+arturmelanchyk@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
client/request.go (2)
211-217: Consider applying the same preallocation pattern for consistency.The
Parammethod uses the same pattern asFormDatabut doesn't preallocate the result slice. For consistency with the optimization applied in this PR, consider applying the same change here:func (r *Request) Param(key string) []string { - var res []string tmp := r.params.PeekMulti(key) + res := make([]string, 0, len(tmp)) for _, v := range tmp { res = append(res, utils.UnsafeString(v)) } return res }
710-719: Optional: Consider preallocating with estimated capacity.While
PeekMultiplecannot preallocate the exact capacity (since it filters by key), you could provide an initial capacity hint to reduce early reallocations:func (h *Header) PeekMultiple(key string) []string { - var res []string + res := make([]string, 0, 4) // reasonable initial capacity byteKey := []byte(key) for k, value := range h.RequestHeader.All() { if bytes.EqualFold(k, byteKey) { res = append(res, utils.UnsafeString(value)) } } return res }Note: This is a minor optimization since the exact count is unknown upfront.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
client/request.go(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*.go: Format Go code using gofumpt (enforced viamake format)
Ensure code passes golangci-lint checks (enforced viamake lint)
Optimize struct field alignment using betteralign (enforced viamake betteralign)
Modernize Go code using gopls modernize (enforced viamake modernize)
Files:
client/request.go
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Compare
- GitHub Check: unit (1.25.x, windows-latest)
- GitHub Check: repeated
🔇 Additional comments (1)
client/request.go (1)
449-456: LGTM! Good optimization.The preallocation correctly sizes the result slice capacity to match the number of values, avoiding reallocation during the append loop. This is a clean performance improvement with no behavior change.
Signed-off-by: Artur Melanchyk <13834276+arturmelanchyk@users.noreply.github.com>
There was a problem hiding this comment.
Pull Request Overview
This PR improves performance by preallocating slice capacity in three methods that return query parameter and form data values. The optimization prevents multiple memory reallocations during slice growth by setting the initial capacity to match the expected number of elements.
- Preallocated slice capacity in
Client.Param()method - Preallocated slice capacity in
Request.Param()method - Preallocated slice capacity in
Request.FormData()method
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| client/client.go | Updated Client.Param() to preallocate slice with capacity based on tmp length |
| client/request.go | Updated Request.Param() and Request.FormData() methods to preallocate slices with capacity based on tmp length |
Description
Tiny improvement
Changes introduced
List the new features or adjustments introduced in this pull request. Provide details on benchmarks, documentation updates, changelog entries, and if applicable, the migration guide.
Type of change
Please delete options that are not relevant.
Checklist
Before you submit your pull request, please make sure you meet these requirements:
/docs/directory for Fiber's documentation.Commit formatting
Please use emojis in commit messages for an easy way to identify the purpose or intention of a commit. Check out the emoji cheatsheet here: CONTRIBUTING.md