Skip to content

⚡ perf: Improve allocations for Request Params()#3766

Merged
gaby merged 5 commits intogofiber:mainfrom
arturmelanchyk:prealloc
Sep 27, 2025
Merged

⚡ perf: Improve allocations for Request Params()#3766
gaby merged 5 commits intogofiber:mainfrom
arturmelanchyk:prealloc

Conversation

@arturmelanchyk
Copy link
Contributor

Description

  1. Saves ~8% time on allocations in Request::Params()
benchstat old-params.txt new-params.txt                                                       
goos: darwin
goarch: arm64
pkg: github.com/gofiber/fiber/v3/client
cpu: Apple M3 Max
                │ old-params.txt │           new-params.txt            │
                │     sec/op     │    sec/op     vs base               │
_Request_Params      151.6n ± 0%   139.3n ± 11%  -8.11% (p=0.000 n=20)

                │ old-params.txt │         new-params.txt         │
                │      B/op      │    B/op     vs base            │
_Request_Params       184.0 ± 0%   184.0 ± 0%  ~ (p=1.000 n=20) ¹
¹ all samples are equal

                │ old-params.txt │           new-params.txt           │
                │   allocs/op    │ allocs/op   vs base                │
_Request_Params       6.000 ± 0%   5.000 ± 0%  -16.67% (p=0.000 n=20)
  1. Saves ~6% time on allocations in Request::AllFormData()
benchstat old-allformdata.txt new-allformdata.txt                                                       
goos: darwin
goarch: arm64
pkg: github.com/gofiber/fiber/v3/client
cpu: Apple M3 Max
                     │ old-allformdata.txt │        new-allformdata.txt         │
                     │       sec/op        │   sec/op     vs base               │
_Request_AllFormData           151.2n ± 1%   142.4n ± 2%  -5.79% (p=0.000 n=20)

                     │ old-allformdata.txt │      new-allformdata.txt       │
                     │        B/op         │    B/op     vs base            │
_Request_AllFormData            184.0 ± 0%   184.0 ± 0%  ~ (p=1.000 n=20) ¹
¹ all samples are equal

                     │ old-allformdata.txt │        new-allformdata.txt         │
                     │      allocs/op      │ allocs/op   vs base                │
_Request_AllFormData            6.000 ± 0%   5.000 ± 0%  -16.67% (p=0.000 n=20)

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.

  • Benchmarks: Describe any performance benchmarks and improvements related to the changes.
  • Documentation Update: Detail the updates made to the documentation and links to the changed files.
  • Changelog/What's New: Include a summary of the additions for the upcoming release notes.
  • Migration Guide: If necessary, provide a guide or steps for users to migrate their existing code to accommodate these changes.
  • API Alignment with Express: Explain how the changes align with the Express API.
  • API Longevity: Discuss the steps taken to ensure that the new or updated APIs are consistent and not prone to breaking changes.
  • Examples: Provide examples demonstrating the new features or changes in action.

Type of change

Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)
  • Enhancement (improvement to existing features and functionality)
  • Documentation update (changes to documentation)
  • Performance improvement (non-breaking change which improves efficiency)
  • Code consistency (non-breaking change which improves code reliability and robustness)

Checklist

Before you submit your pull request, please make sure you meet these requirements:

  • Followed the inspiration of the Express.js framework for new functionalities, making them similar in usage.
  • Conducted a self-review of the code and provided comments for complex or critical parts.
  • Updated the documentation in the /docs/ directory for Fiber's documentation.
  • Added or updated unit tests to validate the effectiveness of the changes or new features.
  • Ensured that new and existing unit tests pass locally with the changes.
  • Verified that any new dependencies are essential and have been agreed upon by the maintainers/community.
  • Aimed for optimal performance with minimal allocations in the new code.
  • Provided benchmarks for the new code to analyze and improve upon.

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

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 26, 2025

Walkthrough

Adds an early-return fast-path when there are no values in Request.Params() and Request.AllFormData(); otherwise both iterators allocate a single backing slice sized 2×vals and split it into k and v subslices to reduce temporary allocations. Tests refactored to exercise iterator-style callbacks for empty and populated cases.

Changes

Cohort / File(s) Summary of changes
Request iteration preallocation & fast-path
client/request.go
Adds an early return when iterators have zero values; uses one preallocated backing slice (make([]string, 2*vals)) and splits it into k and v subslices (prealloc[:0:vals] and prealloc[vals:vals:2*vals]) to avoid separate allocations while preserving behavior.
Iterator-style tests (empty & populated cases)
client/request_test.go
Refactors tests to use iterator callbacks for Params() and AllFormData(), adds parallel subtests for "empty iterator" and "populated iterator", verifies no callback calls for empty cases, and asserts collected maps for populated cases; includes per-test request lifecycle and cleanup.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant C as Caller
    participant R as Request.Params()/AllFormData()

    C->>R: call iterator with callback
    R->>R: compute vals
    alt vals == 0
        R-->>C: fast-path — do not invoke callback (return)
    else vals > 0
        R->>R: prealloc := make([]string, 2*vals)
        R->>R: k := prealloc[:0:vals]
        R->>R: v := prealloc[vals:vals:2*vals]
        loop for i in 0..vals-1
            R->>C: invoke callback(k[i], v[i])
        end
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • efectn
  • gaby
  • sixcolors

Poem

I nibble bytes and stash them neat,
One backing slice makes pairs complete.
If none are due, I skip the race,
Light paws, fewer steps, a tidy place.
Hop on, code — a rabbit's quiet feat. 🐇

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Description Check ⚠️ Warning The pull request description provides comprehensive benchmarking data but retains the template placeholders in the Changes introduced section without actual summaries of documentation updates, changelog entries, or migration guidance, and it omits the required Fixes # reference to link the change to an issue, resulting in an incompletely filled template. Please replace the placeholder checklist under Changes introduced with actual entries summarizing benchmarks, documentation updates, changelog additions, and migration steps if needed, include a Fixes # reference if applicable, and update the checklist to reflect completed tasks to fully meet the template requirements.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (1 passed)
Check name Status Explanation
Title Check ✅ Passed The title concisely indicates a performance optimization by improving allocations in the Request Params method, reflecting one of the core enhancements introduced in this PR while using clear and direct language.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 641c161 and ac31023.

📒 Files selected for processing (1)
  • client/request_test.go (2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.go

📄 CodeRabbit inference engine (AGENTS.md)

**/*.go: Format Go code using gofumpt (enforced via make format)
Ensure code passes golangci-lint checks (enforced via make lint)
Optimize struct field alignment using betteralign (enforced via make betteralign)
Modernize Go code using gopls modernize (enforced via make modernize)

Files:

  • client/request_test.go
🧬 Code graph analysis (1)
client/request_test.go (1)
client/request.go (2)
  • AcquireRequest (961-967)
  • ReleaseRequest (971-974)
⏰ 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: unit (1.25.x, windows-latest)
  • GitHub Check: repeated
  • GitHub Check: Compare
🔇 Additional comments (2)
client/request_test.go (2)

325-362: Iterator coverage looks solid

Exercising both the empty and populated cases via iterator callbacks (plus ensuring pooled requests get released with t.Cleanup) matches the optimized Params semantics and guards against regressions. Nicely done.


1401-1438: Form-data iterator tests align well

Mirroring the iterator-based assertions for AllFormData() gives confidence that the shared preallocation path still yields distinct slices per key while skipping work for empty payloads. Looks good to me.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@arturmelanchyk arturmelanchyk marked this pull request as ready for review September 26, 2025 19:05
@arturmelanchyk arturmelanchyk requested a review from a team as a code owner September 26, 2025 19:05
@codecov
Copy link

codecov bot commented Sep 26, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.38%. Comparing base (0224bae) to head (ac31023).
⚠️ Report is 6 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3766   +/-   ##
=======================================
  Coverage   91.37%   91.38%           
=======================================
  Files         113      113           
  Lines       11874    11882    +8     
=======================================
+ Hits        10850    10858    +8     
  Misses        755      755           
  Partials      269      269           
Flag Coverage Δ
unittests 91.38% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (2)
client/request.go (2)

219-225: Skip work when there are no params

Micro-optimization to avoid allocation and sort on empty input.

Apply this diff:

 func (r *Request) Params() iter.Seq2[string, []string] {
 	return func(yield func(string, []string) bool) {
 		vals := r.params.Len()
+		if vals == 0 {
+			return
+		}
 		prealloc := make([]string, 2*vals)
 		p := pair{
 			k: prealloc[:0:vals],
 			v: prealloc[vals : vals : 2*vals],
 		}

456-461: Skip work when there is no form data

Same micro-optimization as Params.

Apply this diff:

 func (r *Request) AllFormData() iter.Seq2[string, []string] {
 	return func(yield func(string, []string) bool) {
 		vals := r.formData.Len()
+		if vals == 0 {
+			return
+		}
 		prealloc := make([]string, 2*vals)
 		p := pair{
 			k: prealloc[:0:vals],
 			v: prealloc[vals : vals : 2*vals],
 		}
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0b0a4a4 and e2e1d7d.

📒 Files selected for processing (1)
  • client/request.go (2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.go

📄 CodeRabbit inference engine (AGENTS.md)

**/*.go: Format Go code using gofumpt (enforced via make format)
Ensure code passes golangci-lint checks (enforced via make lint)
Optimize struct field alignment using betteralign (enforced via make betteralign)
Modernize Go code using gopls modernize (enforced via make 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). (4)
  • GitHub Check: unit (1.25.x, macos-13)
  • GitHub Check: repeated
  • GitHub Check: unit (1.25.x, windows-latest)
  • GitHub Check: Compare
🔇 Additional comments (4)
client/request.go (4)

221-225: Prealloc for paired slices is correct and beneficial

Using a single backing array and three-index slicing for k and v reduces allocations while keeping capacities aligned. Nice win.


457-461: Same solid prealloc pattern for form data

Mirrors the params optimization; good use of shared backing array and capacities.


221-225: No action needed: Args.Len() equals the number of pairs yielded by All(), so the prealloc optimization remains valid


233-240: Ensure minimum Go version ≥1.21 for integer range loops
for i := range vals relies on range-over-int added in Go 1.21; update your go.mod or CI to target Go 1.21+ to avoid build failures.

Copy link
Member

@renanbastos93 renanbastos93 left a comment

Choose a reason for hiding this comment

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

LGTM

@gaby gaby added this to v3 Sep 27, 2025
@gaby gaby added this to the v3 milestone Sep 27, 2025
@gaby gaby moved this to In Progress in v3 Sep 27, 2025
@gaby gaby changed the title Save 6-8% time on various Request functions ⚡ perf: Improve allocations for Request Params() Sep 27, 2025
@gaby gaby merged commit a3b236f into gofiber:main Sep 27, 2025
15 checks passed
@github-project-automation github-project-automation bot moved this from In Progress to Done in v3 Sep 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants