refactor: streamline request handler selection and context management for improved performance#4233
Conversation
… for improved performance
WalkthroughA centralized request-handler selection (new selectRequestHandler) picks between defaultRequestHandler and customRequestHandler based on app.hasCustomCtx. Context acquisition/release is refactored with pooled DefaultCtx helpers and a userContextSet flag. Path lowercasing moved to appendLowerBytes and route matching reuses cached path strings. ChangesRequest Handler Selection & Context Management Refactor
Sequence Diagram(s)sequenceDiagram
participant Client
participant fasthttp
participant App
participant Router
participant DefaultCtxPool
Client->>fasthttp: HTTP request
fasthttp->>App: invoke Handler (selectRequestHandler)
App->>App: selectRequestHandler() -> default/custom handler
App->>DefaultCtxPool: acquireDefaultCtx (if default)
DefaultCtxPool-->>App: *DefaultCtx
App->>Router: next / nextCustom with context
Router->>Router: route.match(detectionPath, path)
Router-->>App: handler result / error
App->>DefaultCtxPool: releaseDefaultCtx (if default)
App->>fasthttp: write response
fasthttp->>Client: HTTP response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4233 +/- ##
==========================================
- Coverage 91.25% 91.11% -0.15%
==========================================
Files 127 127
Lines 12556 12605 +49
==========================================
+ Hits 11458 11485 +27
- Misses 687 702 +15
- Partials 411 418 +7
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:
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request optimizes the request handling path by introducing specialized handlers (defaultRequestHandler and customRequestHandler) to avoid interface assertions and unnecessary defer calls. It refactors context management, including a more efficient way to acquire and release default contexts, and optimizes path handling and flash cookie detection. Additionally, it updates documentation and removes redundant release methods from the request and response interfaces. I have no feedback to provide.
There was a problem hiding this comment.
Pull request overview
This PR refactors Fiber’s request handling path to reduce per-request overhead, primarily by splitting default vs custom context request handlers and tightening hot-path operations in routing and context management.
Changes:
- Introduces separate
defaultRequestHandlerandcustomRequestHandler, and selects between them viaselectRequestHandler(). - Reduces allocations/overhead in routing and path handling (precomputes unsafe string conversions; replaces lowercasing helper with a local fast-path).
- Adjusts context lifecycle cleanup (tracks whether a user context was stored; simplifies req/res release mechanics; updates related interface docs).
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| router.go | Precomputes route match strings; splits request handlers; inlines some hot-path checks. |
| app.go | Routes fasthttp handler setup through selectRequestHandler() for default/custom ctx. |
| ctx_interface.go | Adds acquireDefaultCtx to avoid interface/type assertion overhead in default path. |
| ctx.go | Optimizes detection-path lowercasing; refines user-context storage/cleanup; removes req/res release calls. |
| errors_internal.go | Adds a dedicated panic error for default ctx pool type assertion failures. |
| req_interface_gen.go | Doc clarifications around multipart parsing respecting BodyLimit; removes release() from interface. |
| req.go | Removes DefaultReq.release() implementation. |
| res_interface_gen.go | Doc tweaks; clarifies getLocationFromRoute semantics; removes release() from interface. |
| res.go | Removes DefaultRes.release() implementation. |
| ctx_interface_gen.go | Mirrors doc clarifications for multipart + AutoFormat and getLocationFromRoute. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
ctx.go (1)
669-678: Consider expressingappendLowerBytesviautils/v2.
utils.ToLowerBytesalready provides a lookup-table-based ASCII lower-case ingithub.com/gofiber/utils/v2. The current single call site can be written as:♻️ Optional refactor
- if !c.app.config.CaseSensitive { - c.detectionPath = appendLowerBytes(c.detectionPath, c.path) - } else { - c.detectionPath = append(c.detectionPath[:0], c.path...) - } + c.detectionPath = append(c.detectionPath[:0], c.path...) + if !c.app.config.CaseSensitive { + c.detectionPath = utils.ToLowerBytes(c.detectionPath) + }…and
appendLowerBytescan be removed.This trades the current single-pass copy+lower for a copy followed by an in-place lookup-table lower. Benchmarks should validate which is faster on the targeted hot path; if the single-pass branch wins, ignore. Otherwise, leaning on the shared helper avoids hand-rolling a primitive.
As per coding guidelines: "Prefer
github.com/gofiber/utils/v2helpers (for example,utils.Trim) when performing common operations such as string manipulation, whenever it is practical and appropriate for the surrounding code".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ctx.go` around lines 669 - 678, The function appendLowerBytes implements ASCII lowercasing manually; replace its single call site to use utils.ToLowerBytes from github.com/gofiber/utils/v2 and remove appendLowerBytes; update imports to add utils (or reuse existing utils import), ensure the call site expects the same behavior (in-place/lowered copy) and run benchmarks if you want to validate performance before deleting appendLowerBytes.router.go (1)
442-446: Consolidate flash-cookie checks by usinghasFlashCookiehelper from both handlers.
defaultRequestHandlerduplicates the flash-cookie optimization inline (raw-header scan followed byCookie(FlashCookieName) != nil), whilecustomRequestHandlercalls the existinghasFlashCookiehelper that performs the identical checks. SincehasFlashCookiealready encodes the optimized fast path (lines 50–61 in redirect.go), replace the inline check indefaultRequestHandlerwith a call tohasFlashCookiefor consistency. This prevents future flash-cookie semantics changes from requiring updates at both call sites.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@router.go` around lines 442 - 446, The inline raw-header + Cookie(FlashCookieName) check in defaultRequestHandler duplicates logic already implemented in hasFlashCookie; replace that conditional with a call to hasFlashCookie so defaultRequestHandler uses the same optimized fast-path helper, and then call ctx.Redirect().parseAndClearFlashMessages() as before (i.e., remove the rawHeaders/bytes.Contains branch and invoke hasFlashCookie to decide whether to parse and clear flash messages).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ctx_interface.go`:
- Around line 84-93: acquireDefaultCtx currently panics if app.pool.Get() isn't
a *DefaultCtx, which can occur during a race if setCtxFunc is called after the
server starts (app.newCtxFunc non-nil → pool.New returns CustomCtx) while
app.server.Handler still points to defaultRequestHandler; to fix, either assert
in setCtxFunc that it must be called before Listen() (e.g., check app.server ==
nil and return or panic with a clear message) or make acquireDefaultCtx
resilient by detecting a non-*DefaultCtx from app.pool.Get() and delegating to
the generic AcquireCtx / customRequestHandler path (release the non-matching
object if needed and obtain a proper ctx via AcquireCtx) so in-flight requests
never panic—update setCtxFunc, acquireDefaultCtx, and any pool.New logic to
implement one of these two approaches.
In `@router.go`:
- Around line 428-463: Add a single deferred cleanup at the start of
defaultRequestHandler to match customRequestHandler (e.g., defer
app.ReleaseCtx(ctx)) so the ctx is always returned to the pool even if
app.next(ctx) or SendStatus panics, then remove the three duplicated blocks that
manually check ctx.abandoned.Load() and call ctx.release(); app.pool.Put(ctx)
(and the early returns that only exist to release the ctx), leaving logic for
methodInt == -1 and error handling intact but relying on ReleaseCtx to do the
actual release and abandonment check.
---
Nitpick comments:
In `@ctx.go`:
- Around line 669-678: The function appendLowerBytes implements ASCII
lowercasing manually; replace its single call site to use utils.ToLowerBytes
from github.com/gofiber/utils/v2 and remove appendLowerBytes; update imports to
add utils (or reuse existing utils import), ensure the call site expects the
same behavior (in-place/lowered copy) and run benchmarks if you want to validate
performance before deleting appendLowerBytes.
In `@router.go`:
- Around line 442-446: The inline raw-header + Cookie(FlashCookieName) check in
defaultRequestHandler duplicates logic already implemented in hasFlashCookie;
replace that conditional with a call to hasFlashCookie so defaultRequestHandler
uses the same optimized fast-path helper, and then call
ctx.Redirect().parseAndClearFlashMessages() as before (i.e., remove the
rawHeaders/bytes.Contains branch and invoke hasFlashCookie to decide whether to
parse and clear flash messages).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: cf125ae5-1365-42d9-b24a-bb88f6e37a51
📒 Files selected for processing (10)
app.goctx.goctx_interface.goctx_interface_gen.goerrors_internal.goreq.goreq_interface_gen.gores.gores_interface_gen.gorouter.go
💤 Files with no reviewable changes (2)
- req.go
- res.go
|
/gemini review |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Code Review
This pull request refactors Fiber's request handling logic by introducing specialized handlers for default and custom contexts, which improves performance and type safety. Key updates include the addition of a userContextSet flag to DefaultCtx for optimized cleanup and a new appendLowerBytes utility for path normalization. Review feedback suggests optimizing appendLowerBytes by pre-allocating slice capacity to avoid reallocations in hot paths and adding a defensive nil check for c.fasthttp in the release method to prevent potential panics.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ctx.go`:
- Around line 649-652: When CaseSensitive is false the code uses
appendLowerBytes to lowercase only ASCII into c.detectionPath, but registered
routes in router.go are normalized using Unicode-aware lowercasing, causing
non-ASCII mismatches (e.g., /Ä vs /ä). Fix by restoring Unicode-safe lowercasing
when c.app.config.CaseSensitive is false: replace the ASCII-only
appendLowerBytes path with the same Unicode-aware normalization used for route
registration (or run a non-ASCII-aware fallback before matching), ensuring
c.detectionPath is produced with the same Unicode lowercasing logic as router.go
so comparisons succeed for non-ASCII characters.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 1b12665a-12d9-4c61-a1bd-423c5a23d3aa
📒 Files selected for processing (6)
app.goctx.goctx_interface.goctx_test.gopath.gorouter.go
✅ Files skipped from review due to trivial changes (1)
- path.go
There was a problem hiding this comment.
♻️ Duplicate comments (1)
ctx.go (1)
649-652:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRestore Unicode-consistent lowercasing for detection path
Line 650 lowercases request paths with ASCII-only
appendLowerBytes, but route normalization still uses Unicode-aware lowercasing (utilsstrings.ToLower) in the same PR branch. WithCaseSensitive=false, non-ASCII paths can stop matching (e.g.,/Ävs/ä).💡 Suggested fix
import ( "context" "crypto/tls" "fmt" "io" "maps" "mime/multipart" "strings" "sync/atomic" "time" "github.com/gofiber/utils/v2" + utilsbytes "github.com/gofiber/utils/v2/bytes" "github.com/valyala/bytebufferpool" "github.com/valyala/fasthttp" ) @@ if !c.app.config.CaseSensitive { - c.detectionPath = appendLowerBytes(c.detectionPath, c.path) + c.detectionPath = append(c.detectionPath[:0], c.path...) + c.detectionPath = utilsbytes.UnsafeToLower(c.detectionPath) } else { c.detectionPath = append(c.detectionPath[:0], c.path...) }Reference checked:
- https://raw.githubusercontent.com/gofiber/fiber/performance-3/router.go
- https://raw.githubusercontent.com/gofiber/fiber/performance-3/path.go
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ctx.go` around lines 649 - 652, The detection path lowercasing uses ASCII-only appendLowerBytes when CaseSensitive is false, which breaks Unicode-aware normalization; change the branch that sets c.detectionPath so it uses the project's Unicode-aware lowercasing (e.g., utilsstrings.ToLower) on c.path before appending instead of appendLowerBytes. Update the code around appendLowerBytes / append(c.detectionPath[:0], c.path...) so that when c.app.config.CaseSensitive is false you lowercased c.path with utilsstrings.ToLower (then append the resulting bytes into c.detectionPath) while keeping the existing non-lowercasing branch unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@ctx.go`:
- Around line 649-652: The detection path lowercasing uses ASCII-only
appendLowerBytes when CaseSensitive is false, which breaks Unicode-aware
normalization; change the branch that sets c.detectionPath so it uses the
project's Unicode-aware lowercasing (e.g., utilsstrings.ToLower) on c.path
before appending instead of appendLowerBytes. Update the code around
appendLowerBytes / append(c.detectionPath[:0], c.path...) so that when
c.app.config.CaseSensitive is false you lowercased c.path with
utilsstrings.ToLower (then append the resulting bytes into c.detectionPath)
while keeping the existing non-lowercasing branch unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: d296d3ad-89f7-4bf4-8f72-ef2f4f2d852b
📒 Files selected for processing (4)
ctx.goctx_interface.goctx_test.gopath.go
✅ Files skipped from review due to trivial changes (1)
- ctx_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- path.go
Optimizes Fiber’s default request hot path while preserving custom context behavior and panic-safe context cleanup.
Changes:
*DefaultCtxpath avoids unnecessary interface/type dispatchhasFlashCookieviewBindMapBenchmarks:
Benchmark_Communication_Flow:39.88 ns/op->35.71 ns/opBenchmark_Router_Next_Default:33.23 ns/op->30.23 ns/opBenchmark_Router_Handler:77.84 ns/op->75.97 ns/opBenchmark_Router_Chain: unchanged in final review-safe versionNo allocation regressions.