perf: replace uuid.Parse with zero-alloc isGUID format validator in route constraint matching#4392
perf: replace uuid.Parse with zero-alloc isGUID format validator in route constraint matching#4392pageton wants to merge 2 commits into
Conversation
…oute constraint matching
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughRemoves the github.com/google/uuid dependency and adds allocation-free unexported helpers ChangesGUID Validation Refactor
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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 unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" 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. Comment |
![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 replaces the external github.com/google/uuid dependency with a custom, allocation-free isGUID validation function to check GUIDs/UUIDs in various formats. It also adds corresponding unit tests and benchmarks. The review feedback suggests removing the unused orig variable in the isGUID function to clean up the code.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4392 +/- ##
=======================================
Coverage 91.40% 91.40%
=======================================
Files 132 132
Lines 13120 13147 +27
=======================================
+ Hits 11992 12017 +25
- Misses 711 712 +1
- Partials 417 418 +1
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:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
path_test.go (1)
771-806: ⚡ Quick winUse
b.Loop()for consistency with existing benchmarks.These benchmarks use the old-style
for i := 0; i < b.N; i++loop pattern, while existing benchmarks in this file (e.g., line 342) use the modernfor b.Loop()pattern. Update for consistency and to follow current Go benchmarking best practices.♻️ Proposed fix for all four benchmarks
func Benchmark_isGUID(b *testing.B) { val := "f0fa66cc-d22e-445b-866d-1d76e776371d" b.ReportAllocs() b.ResetTimer() - for i := 0; i < b.N; i++ { + for b.Loop() { _ = isGUID(val) } } func Benchmark_isGUID_NoHyphens(b *testing.B) { val := "f0fa66ccd22e445b866d1d76e776371d" b.ReportAllocs() b.ResetTimer() - for i := 0; i < b.N; i++ { + for b.Loop() { _ = isGUID(val) } } func Benchmark_isGUID_Invalid(b *testing.B) { val := "not-a-guid-at-all" b.ReportAllocs() b.ResetTimer() - for i := 0; i < b.N; i++ { + for b.Loop() { _ = isGUID(val) } } func Benchmark_CheckConstraint_GUID(b *testing.B) { b.ReportAllocs() c := &Constraint{ID: guidConstraint} val := "f0fa66cc-d22e-445b-866d-1d76e776371d" b.ResetTimer() - for i := 0; i < b.N; i++ { + for b.Loop() { _ = c.CheckConstraint(val) } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@path_test.go` around lines 771 - 806, Update the four benchmarks Benchmark_isGUID, Benchmark_isGUID_NoHyphens, Benchmark_isGUID_Invalid, and Benchmark_CheckConstraint_GUID to use the modern Go benchmark loop by replacing the manual for i := 0; i < b.N; i++ loop with for b.Loop() { ... }; keep existing setup (val, c := &Constraint{...}, b.ReportAllocs(), b.ResetTimer()) and call isGUID or c.CheckConstraint inside the new for b.Loop() body.path.go (1)
798-820: ⚡ Quick winRemove unused
origvariable.The
origvariable is stored on line 798 but never actually used—line 819's_ = origis a no-op. This appears to be leftover development code and should be removed to eliminate confusion.♻️ Proposed fix
func isGUID(s string) bool { - orig := s if strings.HasPrefix(s, "urn:uuid:") { s = s[9:] } else if len(s) > 2 && s[0] == '{' && s[len(s)-1] == '}' { s = s[1 : len(s)-1] } if len(s) == 36 { return isHexGroup(s, 8) && s[8] == '-' && isHexGroup(s[9:], 4) && s[13] == '-' && isHexGroup(s[14:], 4) && s[18] == '-' && isHexGroup(s[19:], 4) && s[23] == '-' && isHexGroup(s[24:], 12) } if len(s) == 32 { return isHexGroup(s, 32) } - _ = orig return false }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@path.go` around lines 798 - 820, The local variable orig is unused; remove its declaration "orig := s" and the no-op "_ = orig" to clean up dead code in the UUID validation block (the function that inspects s for "urn:uuid:", braces, 36/32 length checks and calls isHexGroup). Ensure you only delete those two occurrences and do not change any other logic or variable names.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@path_test.go`:
- Around line 771-806: Update the four benchmarks Benchmark_isGUID,
Benchmark_isGUID_NoHyphens, Benchmark_isGUID_Invalid, and
Benchmark_CheckConstraint_GUID to use the modern Go benchmark loop by replacing
the manual for i := 0; i < b.N; i++ loop with for b.Loop() { ... }; keep
existing setup (val, c := &Constraint{...}, b.ReportAllocs(), b.ResetTimer())
and call isGUID or c.CheckConstraint inside the new for b.Loop() body.
In `@path.go`:
- Around line 798-820: The local variable orig is unused; remove its declaration
"orig := s" and the no-op "_ = orig" to clean up dead code in the UUID
validation block (the function that inspects s for "urn:uuid:", braces, 36/32
length checks and calls isHexGroup). Ensure you only delete those two
occurrences and do not change any other logic or variable names.
|
@pageton We are not replacing Google UUID. We already neen through this in the past. Please discuss in an issue first, before sending PRs. |
3 participants
Summary
Replaces
uuid.Parse()with a customisGUID()format validator for theguidConstraintroute constraint check, eliminating the dependency ongithub.com/google/uuidinpath.goand reducing per-match overhead.Problem
When routes are registered with a GUID constraint (e.g.,
/users/:id<guid>), theCheckConstraintfunction inpath.gocallsuuid.Parse()on every matching request to validate the parameter:While
uuid.Parseis well-optimized in recent versions ofgoogle/uuid, it performs full parsing into a 16-byte[16]byteUUID struct — work that is unnecessary when the constraint only needs to know whether the format is valid. This also introduces an import ofgithub.com/google/uuidinpath.gosolely for this one call.When is this triggered?
The
guidConstraintcode path is only executed when:<guid>constraint (e.g.,/:id<guid>)This is not a hot path for every request. It only affects apps that use GUID-constrained route parameters. The optimization is narrow in scope but has zero downside.
Solution
Replace
uuid.Parse(param)withisGUID(param)— a pure format validator that checks hex characters and hyphen positions without allocating or parsing into a UUID struct.What changed
path.go:import "github.com/google/uuid"(the package is still used instate.go)isGUID(s string) bool— validates GUID/UUID format without allocationisHexGroup(s string, n int) bool— helper for checking consecutive hex charsguidConstraintcase from_, err = uuid.Parse(param)to early-return withisGUIDpath_test.go:Test_isGUID— 17 test cases covering standard, uppercase, braced, URN, no-hyphen, and invalid formatsBenchmark_isGUID,Benchmark_isGUID_NoHyphens,Benchmark_isGUID_Invalid,Benchmark_CheckConstraint_GUIDSupported formats
isGUIDsupports all formats thatuuid.Parseaccepts for the common URL parameter use cases:f0fa66cc-d22e-445b-866d-1d76e776371dF0FA66CC-D22E-445B-866D-1D76E776371Df0fa66ccd22e445b866d1d76e776371d{f0fa66cc-d22e-445b-866d-1d76e776371d}urn:uuid:f0fa66cc-d22e-445b-866d-1d76e776371dBenchmark Results
Tested on AMD Ryzen 5 7600X, Go 1.26.2, linux/amd64.
Before (
uuid.Parse)After (
isGUID)Comparison
uuid.Parse)isGUID)Checklist