Skip to content

🐛 fix: eliminate data races in parallel router benchmarks#4044

Merged
gaby merged 3 commits intoadd-parallel-benchmarks-to-test-filesfrom
copilot/sub-pr-4042
Feb 1, 2026
Merged

🐛 fix: eliminate data races in parallel router benchmarks#4044
gaby merged 3 commits intoadd-parallel-benchmarks-to-test-filesfrom
copilot/sub-pr-4042

Conversation

Copy link
Contributor

Copilot AI commented Feb 1, 2026 •
edited
Loading

Description

Parallel benchmarks in router_test.go shared mutable state across goroutines, causing data races under -race and non-deterministic test failures. The Benchmark_App_MethodNotAllowed_Parallel benchmark failed CI with race conditions on response headers.

Changes introduced

Fixed data races in four parallel benchmarks:

  • Benchmark_App_RebuildTree_Parallel - Each worker now creates its own App instance instead of mutating shared app.routesRefreshed
  • Benchmark_App_MethodNotAllowed_Parallel - Worker-local RequestCtx per goroutine with post-benchmark verification
  • Benchmark_Router_GitHub_API_Parallel - Eliminated shared match/err variables; worker-local contexts
  • Benchmark_Route_Match_Parallel - Worker-local params arrays with separate verification

Pattern applied:

b.RunParallel(func(pb *testing.PB) {
    // Worker-local state - no sharing
    c := &fasthttp.RequestCtx{}
    c.Request.Header.SetMethod("DELETE")
    c.URI().SetPath("/path")
    for pb.Next() {
        appHandler(c)
    }
})

// Single-threaded verification after benchmark
verifyCtx := &fasthttp.RequestCtx{}
// ... setup ...
appHandler(verifyCtx)
require.Equal(b, 405, verifyCtx.Response.StatusCode())
  • Benchmarks: All four benchmarks pass with -race detector; no performance regression
  • Code consistency: Benchmarks now measure actual parallel performance without race conditions

Type of change

  • Code consistency (non-breaking change which improves code reliability and robustness)

Checklist

  • Conducted a self-review of the code and provided comments for complex or critical parts.
  • Ensured that new and existing unit tests pass locally with the changes.
  • Aimed for optimal performance with minimal allocations in the new code.

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI assigned Copilot and gaby Feb 1, 2026
Copilot AI mentioned this pull request Feb 1, 2026
Merged
@gaby
Fix data races in parallel benchmarks
c037939 
Co-authored-by: gaby <835733+gaby@users.noreply.github.com>
@gaby
Add nolint directives for benchmark error handling
16e596c 
Co-authored-by: gaby <835733+gaby@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Feb 1, 2026

Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • exampleretry.com
    • Triggering command: /tmp/go-build3867482159/b303/client.test /tmp/go-build3867482159/b303/client.test -test.paniconexit0 -test.v=test2json -test.timeout=10m0s -test.count=1 -test.shuffle=on tmain.go ux-amd64/pkg/tool/linux_amd64/asm --gdwarf-5 0.1-go1.25.0.lin-unsafeptr=false mpile ux-amd64/pkg/too/tmp/go-build3867482159/b115/vet.cfg -uns�� uHkQD1rlO mpile ux-amd64/pkg/tool/linux_amd64/vet (dns block)
  • vuln.go.dev
    • Triggering command: /tmp/go-build3794990803/b001/exe/govulncheck /tmp/go-build3794990803/b001/exe/govulncheck ./... mpile l/linux_amd64/vet -o .a mpile ux-amd64/pkg/tool/linux_amd64/vet ux-amd64/src/sli/home/REDACTED/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.25.0.lin/home/REDACTED/go/pkg/mod/golang.org/x/telemetry@v0.0.0-20240522233618-39ace7a40ae7/internal/upload/findwork.go ls/staticcheck/s-unsafeptr=false mpile ux-amd64/pkg/tool/linux_amd64/vet -o JCydZmARv om/golangci/golangci-lint/v2@v2.github.com/gofiber/fiber/v3/middleware/compress ux-amd64/pkg/tool/linux_amd64/vet edeclared.go gci/golangci-lin/tmp/go-build378291692/b293/vet.cfg l/linux_amd64/compile ux-amd64/pkg/tool/linux_amd64/vet (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Copilot AI changed the title [WIP] Add parallel benchmark variants for router and app 🐛 fix: eliminate data races in parallel router benchmarks Feb 1, 2026
Copilot AI requested a review from gaby February 1, 2026 14:37
@codecov
Copy link

codecov bot commented Feb 1, 2026 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.15%. Comparing base (9524cbd) to head (16e596c).
⚠️ Report is 4 commits behind head on add-parallel-benchmarks-to-test-files.

Additional details and impacted files 
@@                            Coverage Diff                            @@
##           add-parallel-benchmarks-to-test-files    #4044      +/-   ##
=========================================================================
- Coverage                                  91.19%   91.15%   -0.04%     
=========================================================================
  Files                                        119      119              
  Lines                                      11117    11117              
=========================================================================
- Hits                                       10138    10134       -4     
- Misses                                       621      625       +4     
  Partials                                     358      358
Flag Coverage Δ
unittests 91.15% <ø> (-0.04%) ⬇️

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.

@gaby gaby marked this pull request as ready for review February 1, 2026 14:55
@gaby gaby requested a review from a team as a code owner February 1, 2026 14:55
@gaby gaby requested review from ReneWerner87, Copilot, efectn and sixcolors and removed request for a team February 1, 2026 14:55
@gaby gaby merged commit 9be4804 into add-parallel-benchmarks-to-test-files Feb 1, 2026
20 of 21 checks passed
@gaby gaby deleted the copilot/sub-pr-4042 branch February 1, 2026 14:55
Copilot AI reviewed Feb 1, 2026
View reviewed changes
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

This PR refactors several parallel router benchmarks to eliminate data races and make the results deterministic under -race, while preserving correctness checks via single-threaded verification passes.

Changes:

  • Benchmark_App_RebuildTree_Parallel now creates a worker-local App and route set in each goroutine instead of mutating shared state (routesRefreshed) across goroutines.
  • Benchmark_App_MethodNotAllowed_Parallel and Benchmark_Route_Match_Parallel now use worker-local state in RunParallel, with correctness verified afterward on a fresh context/params in a single goroutine.
  • Benchmark_Router_GitHub_API_Parallel now uses per-worker RequestCtx and eliminates shared match/err variables, with per-route correctness verified serially after each parallel run.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@sixcolors sixcolors Awaiting requested review from sixcolors sixcolors was automatically assigned from gofiber/maintainers

@ReneWerner87 ReneWerner87 Awaiting requested review from ReneWerner87 ReneWerner87 was automatically assigned from gofiber/maintainers

@efectn efectn Awaiting requested review from efectn efectn was automatically assigned from gofiber/maintainers

+1 more reviewer

@gaby gaby gaby approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@gaby gaby

Copilot code review Copilot

Labels

☢️ Bug ✏️ Feature

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@gaby @ReneWerner87