🧹 chore: Use sync.Pool for form and multipart binding

[gaby]

Summary

• This pull request significantly optimizes the form binding process by implementing object pooling for internal maps used to store form values and multipart file headers. By reusing these map structures across requests, the change aims to reduce garbage collection pressure and improve overall performance, particularly in high-throughput scenarios.

Related #3767

[coderabbitai]

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Walkthrough

Adds sync.Pool-backed maps and helper functions to reuse form value and multipart file-header maps during form binding; integrates pooled maps into Bind and bindMultipart and adds tests ensuring maps are cleared between consecutive requests.

Changes

Cohort / File(s)	Change Summary
Form Binding Pooling binder/form.go	Adds formMapPool and formFileMapPool (type sync.Pool) plus helpers: acquireFormMap, releaseFormMap, clearFormMap, acquireFileHeaderMap, releaseFileHeaderMap, clearFileHeaderMap. Replaces per-request map allocations in Bind and bindMultipart with pooled maps and releases them after use.
Form Binding Tests binder/form_test.go	Adds tests Test_FormBinder_Bind_MapClearedBetweenRequests and Test_FormBinder_BindMultipart_MapsClearedBetweenRequests to assert pooled maps do not retain state across consecutive binds (standard and multipart).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested labels

🧹 Updates, ⚡️ Performance

Suggested reviewers

• sixcolors
• ReneWerner87
• efectn

Poem

🐰 I found a pool beneath the trees so deep,
Maps rest there snug, no memory to keep.
I bind and hop, then clear each little map,
No leaks, no trails — I nap and then I clap! ✨

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
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.
Description check	⚠️ Warning	The PR description is minimal and lacks most required sections from the template, including detailed changes, benchmarks, documentation updates, type of change, and comprehensive checklist items.	Expand the description to include: detailed list of changes introduced, benchmarks/performance metrics, documentation updates, checkbox for type of change (performance improvement/code consistency), and verification of checklist items (self-review, tests, dependencies, performance optimization, benchmarks).

✅ Passed checks (1 passed)

Check name	Status	Explanation
Title check	✅ Passed	The PR title clearly summarizes the main change: introducing sync.Pool for form and multipart binding to optimize memory allocations.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch add-sync.pool-instances-for-maps-8ip4mi

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[gemini-code-assist]

Summary of Changes

Hello @gaby, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly optimizes the form binding process by implementing object pooling for internal maps used to store form values and multipart file headers. By reusing these map structures across requests, the change aims to reduce garbage collection pressure and improve overall performance, particularly in high-throughput scenarios. The update also includes robust testing to ensure the integrity of data between pooled map uses.

Highlights

• Memory Optimization: Introduced sync.Pool for map[string][]string (form values) and map[string][]*multipart.FileHeader (multipart file headers) within the form binder to reduce memory allocations by reusing map instances.
• Map Clearing Logic: Pooled maps are now explicitly cleared upon acquisition and release, ensuring that data from previous requests does not leak into subsequent ones. This leverages Go's built-in clear function for efficiency.
• Simplified Pool Helpers: The helper functions for managing pooled maps have been streamlined, utilizing the clear built-in and avoiding redundant clearing operations.
• Regression Tests: New regression tests have been added to confirm that both pooled form and multipart maps are correctly cleared between requests, preventing data contamination.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request introduces sync.Pool to reuse maps for form values and multipart file headers, which is a great way to reduce allocations and improve performance. The changes are well-implemented, and the addition of regression tests to ensure maps are cleared between requests is excellent. I have one suggestion to improve consistency and remove a redundant operation in one of the new helper functions.

[codecov]

Codecov Report

❌ Patch coverage is 87.50000% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.65%. Comparing base (032ff5f) to head (de8f338).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
binder/form.go	87.50%	2 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3967      +/-   ##
==========================================
- Coverage   91.67%   91.65%   -0.03%     
==========================================
  Files         119      119              
  Lines       10177    10223      +46     
==========================================
+ Hits         9330     9370      +40     
- Misses        536      539       +3     
- Partials      311      314       +3     

Flag	Coverage Δ
unittests	91.65% <87.50%> (-0.03%)	⬇️

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.

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

binder/form.go (1)

92-104: Consider removing redundant clear on acquire.

acquireFormMap clears the map on line 97, and releaseFormMap also clears before returning to the pool on line 102. This double-clearing is redundant—once on release is sufficient since the pool will only contain cleared maps. Per the PR description's intent to "avoid redundant clearing," removing the clear from acquireFormMap would improve consistency with acquireFileHeaderMap and eliminate unnecessary work.

🔎 Proposed refactor to remove redundant clearing

 func acquireFormMap() map[string][]string {
 	m, ok := formMapPool.Get().(map[string][]string)
 	if !ok {
 		m = make(map[string][]string)
 	}
-	clearFormMap(m)
 	return m
 }

📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 93830f2 and f1d27a0.

📒 Files selected for processing (2)

• binder/form.go
• binder/form_test.go

🧰 Additional context used

📓 Path-based instructions (2)

**/*_test.go

📄 CodeRabbit inference engine (AGENTS.md)

When adding Go tests, always invoke t.Parallel() at the start of each test and subtest to maximize concurrency

Files:

• binder/form_test.go

**/*.go

📄 CodeRabbit inference engine (AGENTS.md)

Prefer github.com/gofiber/utils/v2 helpers (for example, utils.Trim) when performing common operations such as string manipulation, whenever it is practical and appropriate for the surrounding code

Files:

• binder/form_test.go
• binder/form.go

🧠 Learnings (1)

📚 Learning: 2024-12-01T10:28:36.011Z

Learnt from: ReneWerner87
Repo: gofiber/fiber PR: 0
File: :0-0
Timestamp: 2024-12-01T10:28:36.011Z
Learning: Fiber currently supports square bracket notation in form data with 'application/x-www-form-urlencoded' content type, but this feature is not available for multipart form data. The implementation for form-urlencoded is in ctx.go (BodyParser method), while multipart form data handling needs enhancement to support this feature.

Applied to files:

• binder/form.go

🧬 Code graph analysis (1)

binder/form_test.go (2)

binder/form.go (2)

• FormBinding (27-29)
• FormBinding (32-34)

bind.go (1)

• Bind (33-37)

⏰ 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)

• GitHub Check: lint
• GitHub Check: unit (1.25.x, windows-latest)
• GitHub Check: Compare
• GitHub Check: unit (1.25.x, macos-latest)
• GitHub Check: repeated

🔇 Additional comments (8)

binder/form.go (6)

13-24: LGTM! Well-structured sync.Pool declarations.

The pool initialization follows best practices by providing New functions that return the correct map types.

---

43-44: LGTM! Proper pooling integration in Bind.

The acquire/release pattern with defer ensures the map is returned to the pool even if errors occur during binding.

---

64-65: LGTM! Consistent pooling for multipart binding.

Both form values and file headers now use pooled maps with proper defer-based cleanup.

Also applies to: 74-75

---

106-117: LGTM! Efficient clearing strategy for file header maps.

This implementation correctly avoids redundant clearing by only clearing on release, ensuring the pool contains clean maps without unnecessary clear operations on acquisition.

---

119-121: LGTM! Clean use of built-in clear function.

---

123-125: LGTM! Consistent clearing helper.

binder/form_test.go (2)

241-270: LGTM! Effective regression test for form map pooling.

This test properly verifies that pooled form maps are cleared between requests by ensuring fields from the first request (Name) don't bleed into the second binding. The test structure is clean and the assertions are precise.

---

272-322: LGTM! Comprehensive regression test for multipart pooling.

This test validates the critical behavior that both form values and file headers are properly cleared between requests. The verification that Avatar is nil after the second bind ensures the file header map pool is working correctly.

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[Copilot]

Pull request overview

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