♻️ Refactor: reduce the memory usage of RoutePatternMatch#3335
♻️ Refactor: reduce the memory usage of RoutePatternMatch#3335ReneWerner87 merged 4 commits intogofiber:mainfrom
Conversation
```
goos: linux
goarch: amd64
pkg: github.com/gofiber/fiber/v3
cpu: AMD EPYC 7763 64-Core Processor
│ route_pattern_match_old.txt │ route_pattern_match_new.txt │
│ sec/op │ sec/op vs base │
_RoutePatternMatch//api/v1/const_|_match_|_/api/v1/const-4 263.4n ± 2% 249.0n ± 4% -5.47% (p=0.001 n=10)
_RoutePatternMatch//api/v1/const_|_not_match_|_/api/v1-4 258.7n ± 4% 244.7n ± 2% -5.43% (p=0.000 n=10)
_RoutePatternMatch//api/v1/const_|_not_match_|_/api/v1/-4 254.6n ± 4% 246.3n ± 2% -3.26% (p=0.000 n=10)
_RoutePatternMatch//api/v1/const_|_not_match_|_/api/v1/something-4 265.1n ± 4% 255.6n ± 3% -3.60% (p=0.001 n=10)
_RoutePatternMatch//api/:param/fixedEnd_|_match_|_/api/abc/fixedEnd-4 775.9n ± 3% 775.6n ± 2% ~ (p=0.424 n=10)
_RoutePatternMatch//api/:param/fixedEnd_|_not_match_|_/api/abc/def/fixedEnd-4 796.7n ± 3% 767.1n ± 2% -3.72% (p=0.001 n=10)
_RoutePatternMatch//api/v1/:param/*_|_match_|_/api/v1/entity-4 916.2n ± 1% 904.8n ± 3% ~ (p=0.052 n=10)
_RoutePatternMatch//api/v1/:param/*_|_match_|_/api/v1/entity/-4 913.8n ± 4% 909.1n ± 3% ~ (p=0.393 n=10)
_RoutePatternMatch//api/v1/:param/*_|_match_|_/api/v1/entity/1-4 915.0n ± 3% 907.2n ± 2% ~ (p=0.165 n=10)
_RoutePatternMatch//api/v1/:param/*_|_not_match_|_/api/v-4 917.5n ± 2% 876.7n ± 2% -4.46% (p=0.000 n=10)
_RoutePatternMatch//api/v1/:param/*_|_not_match_|_/api/v2-4 918.5n ± 2% 886.8n ± 2% -3.45% (p=0.000 n=10)
_RoutePatternMatch//api/v1/:param/*_|_not_match_|_/api/v1/-4 935.6n ± 2% 901.9n ± 2% -3.60% (p=0.000 n=10)
geomean 588.3n 570.7n -2.99%
│ route_pattern_match_old.txt │ route_pattern_match_new.txt │
│ B/op │ B/op vs base │
_RoutePatternMatch//api/v1/const_|_match_|_/api/v1/const-4 168.0 ± 0% 152.0 ± 0% -9.52% (p=0.000 n=10)
_RoutePatternMatch//api/v1/const_|_not_match_|_/api/v1-4 160.0 ± 0% 144.0 ± 0% -10.00% (p=0.000 n=10)
_RoutePatternMatch//api/v1/const_|_not_match_|_/api/v1/-4 160.0 ± 0% 144.0 ± 0% -10.00% (p=0.000 n=10)
_RoutePatternMatch//api/v1/const_|_not_match_|_/api/v1/something-4 176.0 ± 0% 160.0 ± 0% -9.09% (p=0.000 n=10)
_RoutePatternMatch//api/:param/fixedEnd_|_match_|_/api/abc/fixedEnd-4 440.0 ± 0% 440.0 ± 0% ~ (p=1.000 n=10) ¹
_RoutePatternMatch//api/:param/fixedEnd_|_not_match_|_/api/abc/def/fixedEnd-4 464.0 ± 0% 440.0 ± 0% -5.17% (p=0.000 n=10)
_RoutePatternMatch//api/v1/:param/*_|_match_|_/api/v1/entity-4 536.0 ± 0% 536.0 ± 0% ~ (p=1.000 n=10) ¹
_RoutePatternMatch//api/v1/:param/*_|_match_|_/api/v1/entity/-4 536.0 ± 0% 536.0 ± 0% ~ (p=1.000 n=10) ¹
_RoutePatternMatch//api/v1/:param/*_|_match_|_/api/v1/entity/1-4 536.0 ± 0% 536.0 ± 0% ~ (p=1.000 n=10) ¹
_RoutePatternMatch//api/v1/:param/*_|_not_match_|_/api/v-4 544.0 ± 0% 528.0 ± 0% -2.94% (p=0.000 n=10)
_RoutePatternMatch//api/v1/:param/*_|_not_match_|_/api/v2-4 544.0 ± 0% 528.0 ± 0% -2.94% (p=0.000 n=10)
_RoutePatternMatch//api/v1/:param/*_|_not_match_|_/api/v1/-4 544.0 ± 0% 528.0 ± 0% -2.94% (p=0.000 n=10)
geomean 353.7 337.9 -4.47%
¹ all samples are equal
│ route_pattern_match_old.txt │ route_pattern_match_new.txt │
│ allocs/op │ allocs/op vs base │
_RoutePatternMatch//api/v1/const_|_match_|_/api/v1/const-4 6.000 ± 0% 5.000 ± 0% -16.67% (p=0.000 n=10)
_RoutePatternMatch//api/v1/const_|_not_match_|_/api/v1-4 6.000 ± 0% 5.000 ± 0% -16.67% (p=0.000 n=10)
_RoutePatternMatch//api/v1/const_|_not_match_|_/api/v1/-4 6.000 ± 0% 5.000 ± 0% -16.67% (p=0.000 n=10)
_RoutePatternMatch//api/v1/const_|_not_match_|_/api/v1/something-4 6.000 ± 0% 5.000 ± 0% -16.67% (p=0.000 n=10)
_RoutePatternMatch//api/:param/fixedEnd_|_match_|_/api/abc/fixedEnd-4 13.00 ± 0% 13.00 ± 0% ~ (p=1.000 n=10) ¹
_RoutePatternMatch//api/:param/fixedEnd_|_not_match_|_/api/abc/def/fixedEnd-4 14.00 ± 0% 13.00 ± 0% -7.14% (p=0.000 n=10)
_RoutePatternMatch//api/v1/:param/*_|_match_|_/api/v1/entity-4 14.00 ± 0% 14.00 ± 0% ~ (p=1.000 n=10) ¹
_RoutePatternMatch//api/v1/:param/*_|_match_|_/api/v1/entity/-4 14.00 ± 0% 14.00 ± 0% ~ (p=1.000 n=10) ¹
_RoutePatternMatch//api/v1/:param/*_|_match_|_/api/v1/entity/1-4 14.00 ± 0% 14.00 ± 0% ~ (p=1.000 n=10) ¹
_RoutePatternMatch//api/v1/:param/*_|_not_match_|_/api/v-4 15.00 ± 0% 14.00 ± 0% -6.67% (p=0.000 n=10)
_RoutePatternMatch//api/v1/:param/*_|_not_match_|_/api/v2-4 15.00 ± 0% 14.00 ± 0% -6.67% (p=0.000 n=10)
_RoutePatternMatch//api/v1/:param/*_|_not_match_|_/api/v1/-4 15.00 ± 0% 14.00 ± 0% -6.67% (p=0.000 n=10)
geomean 10.67 9.811 -8.08%
¹ all samples are equal
```
…tPart
```
goos: linux
goarch: amd64
pkg: github.com/gofiber/fiber/v3
cpu: AMD EPYC 7763 64-Core Processor
│ route_pattern_match_old.txt │ route_pattern_match_new3.txt │
│ sec/op │ sec/op vs base │
_RoutePatternMatch//api/v1/const_|_match_|_/api/v1/const-4 264.3n ± 2% 253.8n ± 2% -3.95% (p=0.001 n=10)
_RoutePatternMatch//api/v1/const_|_not_match_|_/api/v1-4 258.5n ± 1% 247.6n ± 2% -4.24% (p=0.000 n=10)
_RoutePatternMatch//api/v1/const_|_not_match_|_/api/v1/-4 260.8n ± 3% 249.7n ± 4% -4.26% (p=0.003 n=10)
_RoutePatternMatch//api/v1/const_|_not_match_|_/api/v1/something-4 265.4n ± 2% 256.1n ± 2% -3.49% (p=0.000 n=10)
_RoutePatternMatch//api/:param/fixedEnd_|_match_|_/api/abc/fixedEnd-4 783.8n ± 2% 777.5n ± 3% ~ (p=0.218 n=10)
_RoutePatternMatch//api/:param/fixedEnd_|_not_match_|_/api/abc/def/fixedEnd-4 797.8n ± 1% 773.6n ± 3% -3.03% (p=0.001 n=10)
_RoutePatternMatch//api/v1/:param/*_|_match_|_/api/v1/entity-4 920.3n ± 2% 926.0n ± 3% ~ (p=0.896 n=10)
_RoutePatternMatch//api/v1/:param/*_|_match_|_/api/v1/entity/-4 920.4n ± 4% 908.2n ± 2% ~ (p=0.063 n=10)
_RoutePatternMatch//api/v1/:param/*_|_match_|_/api/v1/entity/1-4 927.9n ± 2% 919.0n ± 3% ~ (p=0.579 n=10)
_RoutePatternMatch//api/v1/:param/*_|_not_match_|_/api/v-4 920.4n ± 3% 889.5n ± 3% -3.36% (p=0.007 n=10)
_RoutePatternMatch//api/v1/:param/*_|_not_match_|_/api/v2-4 916.9n ± 2% 891.9n ± 2% -2.73% (p=0.000 n=10)
_RoutePatternMatch//api/v1/:param/*_|_not_match_|_/api/v1/-4 938.8n ± 5% 891.2n ± 2% -5.07% (p=0.000 n=10)
geomean 591.7n 575.5n -2.73%
│ route_pattern_match_old.txt │ route_pattern_match_new3.txt │
│ B/op │ B/op vs base │
_RoutePatternMatch//api/v1/const_|_match_|_/api/v1/const-4 168.0 ± 0% 152.0 ± 0% -9.52% (p=0.000 n=10)
_RoutePatternMatch//api/v1/const_|_not_match_|_/api/v1-4 160.0 ± 0% 144.0 ± 0% -10.00% (p=0.000 n=10)
_RoutePatternMatch//api/v1/const_|_not_match_|_/api/v1/-4 160.0 ± 0% 144.0 ± 0% -10.00% (p=0.000 n=10)
_RoutePatternMatch//api/v1/const_|_not_match_|_/api/v1/something-4 176.0 ± 0% 160.0 ± 0% -9.09% (p=0.000 n=10)
_RoutePatternMatch//api/:param/fixedEnd_|_match_|_/api/abc/fixedEnd-4 440.0 ± 0% 440.0 ± 0% ~ (p=1.000 n=10) ¹
_RoutePatternMatch//api/:param/fixedEnd_|_not_match_|_/api/abc/def/fixedEnd-4 464.0 ± 0% 440.0 ± 0% -5.17% (p=0.000 n=10)
_RoutePatternMatch//api/v1/:param/*_|_match_|_/api/v1/entity-4 536.0 ± 0% 536.0 ± 0% ~ (p=1.000 n=10) ¹
_RoutePatternMatch//api/v1/:param/*_|_match_|_/api/v1/entity/-4 536.0 ± 0% 536.0 ± 0% ~ (p=1.000 n=10) ¹
_RoutePatternMatch//api/v1/:param/*_|_match_|_/api/v1/entity/1-4 536.0 ± 0% 536.0 ± 0% ~ (p=1.000 n=10) ¹
_RoutePatternMatch//api/v1/:param/*_|_not_match_|_/api/v-4 544.0 ± 0% 528.0 ± 0% -2.94% (p=0.000 n=10)
_RoutePatternMatch//api/v1/:param/*_|_not_match_|_/api/v2-4 544.0 ± 0% 528.0 ± 0% -2.94% (p=0.000 n=10)
_RoutePatternMatch//api/v1/:param/*_|_not_match_|_/api/v1/-4 544.0 ± 0% 528.0 ± 0% -2.94% (p=0.000 n=10)
geomean 353.7 337.9 -4.47%
¹ all samples are equal
│ route_pattern_match_old.txt │ route_pattern_match_new3.txt │
│ allocs/op │ allocs/op vs base │
_RoutePatternMatch//api/v1/const_|_match_|_/api/v1/const-4 6.000 ± 0% 5.000 ± 0% -16.67% (p=0.000 n=10)
_RoutePatternMatch//api/v1/const_|_not_match_|_/api/v1-4 6.000 ± 0% 5.000 ± 0% -16.67% (p=0.000 n=10)
_RoutePatternMatch//api/v1/const_|_not_match_|_/api/v1/-4 6.000 ± 0% 5.000 ± 0% -16.67% (p=0.000 n=10)
_RoutePatternMatch//api/v1/const_|_not_match_|_/api/v1/something-4 6.000 ± 0% 5.000 ± 0% -16.67% (p=0.000 n=10)
_RoutePatternMatch//api/:param/fixedEnd_|_match_|_/api/abc/fixedEnd-4 13.00 ± 0% 13.00 ± 0% ~ (p=1.000 n=10) ¹
_RoutePatternMatch//api/:param/fixedEnd_|_not_match_|_/api/abc/def/fixedEnd-4 14.00 ± 0% 13.00 ± 0% -7.14% (p=0.000 n=10)
_RoutePatternMatch//api/v1/:param/*_|_match_|_/api/v1/entity-4 14.00 ± 0% 14.00 ± 0% ~ (p=1.000 n=10) ¹
_RoutePatternMatch//api/v1/:param/*_|_match_|_/api/v1/entity/-4 14.00 ± 0% 14.00 ± 0% ~ (p=1.000 n=10) ¹
_RoutePatternMatch//api/v1/:param/*_|_match_|_/api/v1/entity/1-4 14.00 ± 0% 14.00 ± 0% ~ (p=1.000 n=10) ¹
_RoutePatternMatch//api/v1/:param/*_|_not_match_|_/api/v-4 15.00 ± 0% 14.00 ± 0% -6.67% (p=0.000 n=10)
_RoutePatternMatch//api/v1/:param/*_|_not_match_|_/api/v2-4 15.00 ± 0% 14.00 ± 0% -6.67% (p=0.000 n=10)
_RoutePatternMatch//api/v1/:param/*_|_not_match_|_/api/v1/-4 15.00 ± 0% 14.00 ± 0% -6.67% (p=0.000 n=10)
geomean 10.67 9.811 -8.08%
¹ all samples are equal
```
WalkthroughThe pull request refactors the routing logic in Changes
Sequence Diagram(s)sequenceDiagram
participant R as RoutePatternMatch
participant U as utils.ToLowerBytes
participant A as analyseParameterPart/ConstantPart
participant E as RemoveEscapeCharBytes
R->>U: Convert pattern to lowercase bytes
U-->>R: Return lowercase bytes
R->>A: Analyze pattern parts (return integer length)
A-->>R: Return processed length and segment
R->>E: Remove escape characters from byte slice
E-->>R: Return cleaned bytes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3335 +/- ##
==========================================
- Coverage 84.29% 84.23% -0.07%
==========================================
Files 116 116
Lines 11572 11578 +6
==========================================
- Hits 9755 9753 -2
- Misses 1389 1395 +6
- Partials 428 430 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
path.go (2)
155-185: Consider consistent usage of byte slices or strings to reduce repeated conversions.You've successfully switched to byte slices for
patternPrettybut frequently convert it back to a string (lines 167, 169, 172, 185). This repeated conversion could introduce unnecessary overhead. As a good-to-have improvement, consider either consistently using byte slices throughout the matching logic or, if strings are ultimately needed, maintaining a single string variable for comparisons.
629-639: In-place modification inRemoveEscapeCharBytesmay impact external usage of the slice.
RemoveEscapeCharBytesperforms in-place modifications on the provided slice and then slices it down. While this is efficient, be cautious if the slice is used elsewhere, as the original contents will be altered.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
path.go(7 hunks)
🔇 Additional comments (3)
path.go (3)
192-204: Initialization of 'n' and 'seg' for route parsing looks good.Declaring and assigning
nandsegbefore entering the conditional branches simplifies readability. The logic for appending segments and parameters also appears consistent with the new return signatures.
279-291: Returning length instead of substring is cleanly implemented.Changing
analyseConstantPartto return just the processed length boosts performance by avoiding substring allocations. The method handles edge cases such asnextParamPositionbeing-1gracefully.
294-399:Details
❓ Verification inconclusive
Parameter parsing refactor looks coherent, but verify no old function calls remain.
Shifting
analyseParameterPartto return an integer for processed length aligns with the PR’s goal of reducing allocations. Please ensure no references to the old(string, *routeSegment)signature linger in the codebase or tests.
🏁 Script executed:
#!/bin/bash # Verify that no references to the old return signatures exist in the repository: ast-grep --pattern $'func (routeParser $_) analyseParameterPart($_) (string, $_) {\n $$$ \n}' ast-grep --pattern $'func (*routeParser) analyseConstantPart($_) (string, $_) {\n $$$ \n}'Length of output: 182
Action: Confirm No Legacy Signatures Remain
The parameter parsing refactor appears correct with the updated signature returning an integer and a pointer to a route segment. However, automated searches using ast-grep did not generate output, which means the absence of matches is inconclusive. Please manually verify that no lingering references to the old
(string, *routeSegment)signature exist in the codebase or in tests—ensure that all call sites have been updated accordingly.
- Files to check:
path.go(lines 294–399) and any test files that invokeanalyseParameterPart.- Verification: Confirm that no assignments expect a
(string, *routeSegment)return type from this function.
In lines 169, 172, and 185, the program does not allocate memory when converting between strings and byte slices. In line 167, I am still working on reducing the memory usage of |
|
@ksw2000 I actually ran into the same problem with |
I tried for the whole day, and I think this is a big issue that cannot be solved in one go. It needs to be optimized in multiple steps. Some functions can be refactored to be more concise. For example, |
Description
The string variable
patternPrettyinRoutePatternMatchcan be changed into[]byte. The byte slice provides greater flexibility for modifying its bytes. Thus, in the benchmark test, we can observe a reduction in memory usage and allocation.RemoveEscapeCharBytesanalyseConstantPartandanalyseParameterPart. Because we only use the length of the returned string, we can just return the number.I'm still working on reducing memory usage in
path.go. However, it does not always lead to a reduction in memory usage. Therefore, I am submitting this pull request first. If I succeed in further reducing memory usage and allocation times, I will submit additional pull requests.Type of change
Please delete options that are not relevant.
Checklist
Before you submit your pull request, please make sure you meet these requirements:
/docs/directory for Fiber's documentation.