Skip to content

🧹 chore: add parallel benchmark variants for router and app#4042

Merged
ReneWerner87 merged 8 commits intomainfrom
add-parallel-benchmarks-to-test-files
Feb 1, 2026
Merged

🧹 chore: add parallel benchmark variants for router and app#4042
ReneWerner87 merged 8 commits intomainfrom
add-parallel-benchmarks-to-test-files

Conversation

@gaby
Copy link
Member

@gaby gaby commented Jan 31, 2026

Motivation

  • Improve benchmark coverage by adding concurrent variants that exercise hot paths under parallel load to better measure real-world performance and catch concurrency regressions.
  • Provide small helpers to centralize AcquireCtx plus type assertions for benchmarks to make parallel benchmark bodies safer and clearer.
  • Keep formatting and linting consistent so the new benchmarks integrate cleanly with the existing test suite.

Description

  • Added acquireDefaultCtxForBenchmark to ctx_test.go, acquireDefaultCtxForRouterBenchmark to router_test.go, and acquireDefaultCtxForAppBenchmark to app_test.go to centralize AcquireCtx + *DefaultCtx assertion and failure handling for benchmarks.
  • Appended _Parallel variants that use b.RunParallel() for many existing benchmarks in ctx_test.go, router_test.go, and app_test.go (examples: Benchmark_NewError_Parallel, Benchmark_Communication_Flow_Parallel, Benchmark_Ctx_AcquireReleaseFlow_Parallel, and many Ctx_*_Parallel and router benchmark parallels).
  • Small supporting changes and whitespace/formatting adjustments to ensure the added code conforms with project format and lint rules.

Fixes #3048

@gaby gaby requested a review from a team as a code owner January 31, 2026 20:13
Copilot AI review requested due to automatic review settings January 31, 2026 20:13
@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello @gaby, I'm Gemini Code Assist1! 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 expands the project's benchmarking capabilities by introducing parallel execution for a wide array of tests related to application context, routing, and general application functionality. The primary goal is to gain more accurate performance insights under concurrent conditions, which is crucial for identifying bottlenecks and ensuring stability in high-load scenarios. Additionally, the PR refactors common setup logic for benchmarks into reusable helper functions, promoting cleaner and more maintainable test code.

Highlights

  • Enhanced Benchmark Coverage: Introduced concurrent variants for numerous existing benchmarks across ctx, router, and app components to better assess performance under parallel load and identify concurrency regressions.
  • Benchmark Helper Functions: Added small helper functions (acquireDefaultCtxForBenchmark, acquireDefaultCtxForRouterBenchmark, acquireDefaultCtxForAppBenchmark) to centralize AcquireCtx and type assertions within benchmarks, improving safety and clarity.
  • Code Quality and Consistency: Ensured that all new benchmarks and supporting changes adhere to project formatting and linting standards.

🧠 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.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 31, 2026

Walkthrough

Adds parallel benchmarks and benchmark-only helpers across app_test.go, ctx_test.go, and router_test.go to exercise concurrent behavior for error creation, DefaultCtx acquire/release, and many router code paths. No production code or public API changes.

Changes

Cohort / File(s) Summary
App benchmarks
app_test.go
Adds parallel benchmarks (Benchmark_NewError_Parallel, Benchmark_Communication_Flow_Parallel, Benchmark_Ctx_AcquireReleaseFlow_Parallel) and helper acquireDefaultCtxForAppBenchmark for obtaining *DefaultCtx in benchmarks.
Ctx test adjustments
ctx_test.go
Removes two parallel subtests from Benchmark_Ctx_IsFromLocalhost; non-parallel checks remain. No production/API changes.
Router benchmarks & helper
router_test.go, router_test.go (helper)
Adds helper acquireDefaultCtxForRouterBenchmark and many parallel router benchmarks (e.g., Benchmark_App_RebuildTree_Parallel, Benchmark_Router_Next_Parallel, Benchmark_Route_Match_*_Parallel, handler/case/strict/unescape/compression/startup scenarios, and a GitHub API scenario). Some benchmarks use per-worker App or RequestCtx setups and acquire/release DefaultCtx inside parallel runs.

Sequence Diagram(s)

(omitted — changes are benchmark additions and helpers; no new runtime control-flow feature requiring a sequence diagram)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • sixcolors
  • efectn
  • ReneWerner87

Poem

"I hop through benchmarks with whiskers twitching bright,
goroutines dancing in the soft moonlight.
I fetch each DefaultCtx, swift as a breeze,
routes and errors bowed down on their knees.
carrots for concurrency, munching metrics tonight! 🥕"

🚥 Pre-merge checks | ✅ 3 | ❌ 2
❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 21.21% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ❓ Inconclusive The PR description covers motivation, implementation details, and linked issue reference, but lacks explicit checklist completion as required by the template. Complete the checklist items from the description template to indicate which requirements apply (benchmarks, tests, documentation, etc.).
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title 'chore: add parallel benchmark variants for router and app' clearly and concisely summarizes the main change—adding parallel benchmark variants.
Linked Issues check ✅ Passed The PR successfully implements all objectives from issue #3048: adds parallel benchmarks using b.RunParallel, targets performance-sensitive areas (router, context, app), and centralizes benchmark setup via helper functions.
Out of Scope Changes check ✅ Passed All changes are scoped to adding parallel benchmark variants and helper functions to test files; no unrelated modifications or production code changes detected.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ 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-parallel-benchmarks-to-test-files

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.

@ReneWerner87 ReneWerner87 added this to v3 Jan 31, 2026
@ReneWerner87 ReneWerner87 added this to the v3 milestone Jan 31, 2026
@gaby gaby changed the title 🧹 chore: add parallel benchmark variants for ctx, router and app tests 🧹 chore: add parallel benchmark variants for ctx, router and app Jan 31, 2026
@codecov
Copy link

codecov bot commented Jan 31, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.15%. Comparing base (a62ce1e) to head (9be4804).
⚠️ Report is 13 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4042      +/-   ##
==========================================
- Coverage   91.21%   91.15%   -0.06%     
==========================================
  Files         119      119              
  Lines       11117    11117              
==========================================
- Hits        10140    10134       -6     
- Misses        620      625       +5     
- Partials      357      358       +1     
Flag Coverage Δ
unittests 91.15% <ø> (-0.06%) ⬇️

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

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds parallel benchmark variants to better measure hot paths under concurrent load, plus small helpers to make benchmark ctx acquisition/type assertions more consistent.

Changes:

  • Added per-file benchmark helpers to acquire *DefaultCtx from App.AcquireCtx with a fatal-on-mismatch assertion.
  • Added many new *_Parallel benchmarks using b.RunParallel() in ctx_test.go and router_test.go.
  • Added parallel variants for a couple of app-level benchmarks in app_test.go.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 13 comments.

File Description
router_test.go Adds acquireDefaultCtxForRouterBenchmark and many new router/app parallel benchmarks.
ctx_test.go Adds acquireDefaultCtxForBenchmark and a large set of new ctx method parallel benchmarks.
app_test.go Adds parallel benchmark variants and acquireDefaultCtxForAppBenchmark helper.

Comment on lines +2038 to +2046
appHandler := app.Handler()
c := &fasthttp.RequestCtx{}
c.Request.Header.SetMethod("DELETE")
c.URI().SetPath("/this/is/a/dummy/route/oke")
b.RunParallel(func(pb *testing.PB) {
for pb.Next() {
appHandler(c)
}
})
Copy link

Copilot AI Jan 31, 2026

Choose a reason for hiding this comment

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

This parallel benchmark shares a single *fasthttp.RequestCtx (c) across all goroutines. b.RunParallel executes concurrently, so the handler will read/write the same request/response buffers at the same time, causing data races and invalid results. Allocate/configure a separate RequestCtx per worker (inside the RunParallel closure) and avoid asserting on a concurrently-mutated response object.

Copilot uses AI. Check for mistakes.
router_test.go Outdated
Comment on lines +2188 to +2199
var match bool
var params [maxParams]string
parsed := parseRoute("/user/keys/:id")
route := &Route{use: false, root: false, star: false, routeParser: parsed, Params: parsed.params, path: "/user/keys/:id", Path: "/user/keys/:id", Method: "DELETE"}
route.Handlers = append(route.Handlers, func(_ Ctx) error {
return nil
})
b.RunParallel(func(pb *testing.PB) {
for pb.Next() {
match = route.match("/user/keys/1337", "/user/keys/1337", &params)
}
})
Copy link

Copilot AI Jan 31, 2026

Choose a reason for hiding this comment

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

route.match writes into params and returns match, but both params and match are shared across all parallel workers here. That introduces data races and makes the post-benchmark assertions nondeterministic. Move params/match to worker-local variables inside the RunParallel closure (and only assert using a single-threaded verification run).

Copilot uses AI. Check for mistakes.
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: 6

🤖 Fix all issues with AI agents
In `@ctx_test.go`:
- Around line 8511-8544: Benchmarks like Benchmark_Ctx_Accepts_Parallel and
Benchmark_Ctx_AcceptsCharsets_Parallel currently share a single *DefaultCtx /
*fasthttp.RequestCtx and a shared result variable across goroutines, causing
data races; fix by acquiring a fresh ctx per goroutine (inside the RunParallel
loop or per worker) using app.AcquireCtx or acquireDefaultCtxForBenchmark, keep
result variables (e.g., res) local to the goroutine, call c.Accepts /
c.AcceptsCharsets on that per-goroutine ctx, and release it after use (e.g.,
app.ReleaseCtx(c) or the appropriate teardown) so no shared mutable ctx/state is
accessed concurrently.

In `@router_test.go`:
- Around line 2187-2202: Benchmark_Route_Match_Parallel currently shares the
mutable variables match and params across goroutines causing data races; fix by
making match and params local to each worker inside the b.RunParallel closure
(e.g., declare local vars like matchLocal and paramsLocal inside the pb loop and
call route.match with &paramsLocal), then after the parallel section perform a
single non-parallel invocation to assert correctness using parsed.params (or run
one sequential check using route.match and a fresh params slice) so benchmarks
remain race-free; apply the same change to the other two similar benchmarks that
use params/match.
- Around line 2018-2028: Benchmark_App_RebuildTree_Parallel races because it
mutates shared App state (app.routesRefreshed and app.RebuildTree) concurrently;
either create a fresh App per parallel worker inside b.RunParallel (instantiate
New() and registerDummyRoutes for each goroutine) or serialize access by
protecting the mutation and call to RebuildTree with a sync.Mutex around the
critical section (use a package-local mutex referenced in the benchmark),
ensuring Benchmark_App_RebuildTree_Parallel no longer mutates the same App
concurrently.
- Around line 2149-2168: Benchmark_Router_Next_Parallel currently shares a
single DefaultCtx (c) and result vars across goroutines; move the context
acquisition into the RunParallel closure by calling
acquireDefaultCtxForRouterBenchmark(b, app, request) (or creating a fresh
RequestCtx/Request) inside the b.RunParallel func so each goroutine gets its own
DefaultCtx, make res and err local to the goroutine, and then after
b.RunParallel perform a single non-parallel check (create one DefaultCtx and
call app.next) to assert require.NoError and require.True and verify
c.indexRoute equals 4.
- Around line 2031-2050: The benchmark reuses a single fasthttp.RequestCtx
across goroutines in Benchmark_App_MethodNotAllowed_Parallel (and the other
listed benchmarks), causing data races; change each benchmark to allocate and
initialize a new *fasthttp.RequestCtx inside the b.RunParallel loop (or inside
the pb.Next iteration) rather than reusing the shared c variable — follow the
same per-iteration allocation/initialization pattern used in
Benchmark_Router_Next_Default_Parallel so each parallel worker constructs its
own RequestCtx, sets method and URI, and then calls appHandler(c).
- Around line 2287-2307: Benchmark_Router_GitHub_API_Parallel has a data race
because c, match, and err are shared across goroutines; move the
fasthttp.RequestCtx variable c and the locals match and err inside the
b.RunParallel closure so each goroutine gets its own instances, set the request
method and URI inside the closure before calling
acquireDefaultCtxForRouterBenchmark, call app.next and app.ReleaseCtx with those
goroutine-local vars, and remove the require.NoError and require.True assertions
at the end of the benchmark since parallel benchmark iterations shouldn't
perform correctness checks; refer to symbols
Benchmark_Router_GitHub_API_Parallel, c, match, err,
acquireDefaultCtxForRouterBenchmark, app.next, app.ReleaseCtx, and
routesFixture.TestRoutes to locate where to change.

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
ctx_test.go (1)

8448-8479: ⚠️ Potential issue | 🟠 Major

Resolve unused benchmark helper to clear lint failure.

acquireDefaultCtxForBenchmark is currently unused, and golangci-lint fails because of it. Either remove it or wire it into existing benchmarks. Here’s a minimal wiring that both uses the helper and simplifies the existing type assertion.

🛠️ Suggested fix (use helper in Benchmark_Ctx_OverrideParam)
 func Benchmark_Ctx_OverrideParam(b *testing.B) {
 	app := New()
 
-	ctx := app.AcquireCtx(&fasthttp.RequestCtx{})
-	c, ok := ctx.(*DefaultCtx)
-	if !ok {
-		b.Fatal("AcquireCtx did not return *DefaultCtx")
-	}
-
-	defer app.ReleaseCtx(c)
+	c := acquireDefaultCtxForBenchmark(b, app, &fasthttp.RequestCtx{})
+	defer app.ReleaseCtx(c)
 
 	c.values = [maxParams]string{"original", "12345"}
 	c.route = &Route{Params: []string{"name", "id"}}
 	c.setMatched(true)
@@
 	for b.Loop() {
 		c.OverrideParam("name", "changed")
 	}
 }

@gaby gaby changed the title 🧹 chore: add parallel benchmark variants for ctx, router and app 🧹 chore: add parallel benchmark variants for router and app Feb 1, 2026
Copy link
Member

@ReneWerner87 ReneWerner87 left a comment

Choose a reason for hiding this comment

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

failed bench

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
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: 2

🤖 Fix all issues with AI agents
In `@router_test.go`:
- Around line 2177-2192: The benchmark shares a single fasthttp.RequestCtx
(fctx) across goroutines causing a data race; inside
Benchmark_Router_Next_Default_Immutable_Parallel move creation and setup of the
request context into the b.RunParallel closure (or into the per-goroutine loop)
so each worker uses its own local fasthttp.RequestCtx, e.g. create a local fctx,
set MethodGet and SetRequestURI("/") before calling h(fctx) in the pb.Next loop
to eliminate shared state.
- Around line 2052-2070: Benchmark_Router_NotFound_Parallel is using a single
shared fasthttp.RequestCtx (c) across goroutines causing a data race; move the
RequestCtx creation and request setup inside the b.RunParallel closure so each
goroutine/iteration uses its own local c. Specifically, in
Benchmark_Router_NotFound_Parallel, allocate a new c := &fasthttp.RequestCtx{}
inside the pb loop, set c.Request.Header.SetMethod("DELETE") and
c.URI().SetPath("/this/route/does/not/exist") there, then call appHandler(c);
keep appHandler (from New()/registerDummyRoutes) as-is. Ensure any assertions
that read c.Response are done on per-iteration locals or removed/adjusted so
they don't reference a shared ctx after RunParallel.

@gaby
Copy link
Member Author

gaby commented Feb 1, 2026

@copilot open a new pull request to apply changes based on the comments in this thread

Copy link
Contributor

Copilot AI commented Feb 1, 2026

@gaby I've opened a new pull request, #4044, to work on those changes. Once the pull request is ready, I'll request review from you.

Copilot AI and others added 3 commits February 1, 2026 14:30
Co-authored-by: gaby <835733+gaby@users.noreply.github.com>
Co-authored-by: gaby <835733+gaby@users.noreply.github.com>
🐛 fix: eliminate data races in parallel router benchmarks
@ReneWerner87 ReneWerner87 merged commit 13ac7eb into main Feb 1, 2026
20 of 21 checks passed
@ReneWerner87 ReneWerner87 deleted the add-parallel-benchmarks-to-test-files branch February 1, 2026 16:44
@github-project-automation github-project-automation bot moved this to Done in v3 Feb 1, 2026
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.

🧹 [Maintenance]: Add Parallel Benchmarks

4 participants