[Static]: Improve sanitizePath performance #3601
Conversation
before: ``` Benchmark_SanitizePath/nilFS_-_urlencoded_chars Benchmark_SanitizePath/nilFS_-_urlencoded_chars-12 4756809 234.2 ns/op 168 B/op 5 allocs/op Benchmark_SanitizePath/nilFS_-_urlencoded_chars-12 5268462 228.8 ns/op 168 B/op 5 allocs/op Benchmark_SanitizePath/nilFS_-_urlencoded_chars-12 5243587 228.7 ns/op 168 B/op 5 allocs/op Benchmark_SanitizePath/nilFS_-_urlencoded_chars-12 5228282 228.9 ns/op 168 B/op 5 allocs/op Benchmark_SanitizePath/dirFS_-_urlencoded_chars Benchmark_SanitizePath/dirFS_-_urlencoded_chars-12 4437174 269.1 ns/op 192 B/op 6 allocs/op Benchmark_SanitizePath/dirFS_-_urlencoded_chars-12 4466428 270.7 ns/op 192 B/op 6 allocs/op Benchmark_SanitizePath/dirFS_-_urlencoded_chars-12 4496061 268.2 ns/op 192 B/op 6 allocs/op Benchmark_SanitizePath/dirFS_-_urlencoded_chars-12 4477066 268.4 ns/op 192 B/op 6 allocs/op Benchmark_SanitizePath/nilFS_-_slashes Benchmark_SanitizePath/nilFS_-_slashes-12 4411864 272.8 ns/op 192 B/op 6 allocs/op Benchmark_SanitizePath/nilFS_-_slashes-12 4430572 269.4 ns/op 192 B/op 6 allocs/op Benchmark_SanitizePath/nilFS_-_slashes-12 4455325 268.8 ns/op 192 B/op 6 allocs/op Benchmark_SanitizePath/nilFS_-_slashes-12 4445643 268.4 ns/op 192 B/op 6 allocs/op ``` after: ``` Benchmark_SanitizePath/nilFS_-_urlencoded_chars Benchmark_SanitizePath/nilFS_-_urlencoded_chars-12 5381319 202.2 ns/op 120 B/op 4 allocs/op Benchmark_SanitizePath/nilFS_-_urlencoded_chars-12 6024532 201.0 ns/op 120 B/op 4 allocs/op Benchmark_SanitizePath/nilFS_-_urlencoded_chars-12 6059211 198.1 ns/op 120 B/op 4 allocs/op Benchmark_SanitizePath/nilFS_-_urlencoded_chars-12 6072016 197.9 ns/op 120 B/op 4 allocs/op Benchmark_SanitizePath/dirFS_-_urlencoded_chars Benchmark_SanitizePath/dirFS_-_urlencoded_chars-12 4747710 241.6 ns/op 144 B/op 5 allocs/op Benchmark_SanitizePath/dirFS_-_urlencoded_chars-12 5035154 238.5 ns/op 144 B/op 5 allocs/op Benchmark_SanitizePath/dirFS_-_urlencoded_chars-12 4991553 239.1 ns/op 144 B/op 5 allocs/op Benchmark_SanitizePath/dirFS_-_urlencoded_chars-12 5034110 238.4 ns/op 144 B/op 5 allocs/op Benchmark_SanitizePath/nilFS_-_slashes Benchmark_SanitizePath/nilFS_-_slashes-12 5599615 215.4 ns/op 160 B/op 5 allocs/op Benchmark_SanitizePath/nilFS_-_slashes-12 5578185 215.0 ns/op 160 B/op 5 allocs/op Benchmark_SanitizePath/nilFS_-_slashes-12 5578350 216.8 ns/op 160 B/op 5 allocs/op Benchmark_SanitizePath/nilFS_-_slashes-12 5556883 214.9 ns/op 160 B/op 5 allocs/op ```
before: ``` Benchmark_SanitizePath/nilFS_-_urlencoded_chars Benchmark_SanitizePath/nilFS_-_urlencoded_chars-12 4756809 234.2 ns/op 168 B/op 5 allocs/op Benchmark_SanitizePath/nilFS_-_urlencoded_chars-12 5268462 228.8 ns/op 168 B/op 5 allocs/op Benchmark_SanitizePath/nilFS_-_urlencoded_chars-12 5243587 228.7 ns/op 168 B/op 5 allocs/op Benchmark_SanitizePath/nilFS_-_urlencoded_chars-12 5228282 228.9 ns/op 168 B/op 5 allocs/op Benchmark_SanitizePath/dirFS_-_urlencoded_chars Benchmark_SanitizePath/dirFS_-_urlencoded_chars-12 4437174 269.1 ns/op 192 B/op 6 allocs/op Benchmark_SanitizePath/dirFS_-_urlencoded_chars-12 4466428 270.7 ns/op 192 B/op 6 allocs/op Benchmark_SanitizePath/dirFS_-_urlencoded_chars-12 4496061 268.2 ns/op 192 B/op 6 allocs/op Benchmark_SanitizePath/dirFS_-_urlencoded_chars-12 4477066 268.4 ns/op 192 B/op 6 allocs/op Benchmark_SanitizePath/nilFS_-_slashes Benchmark_SanitizePath/nilFS_-_slashes-12 4411864 272.8 ns/op 192 B/op 6 allocs/op Benchmark_SanitizePath/nilFS_-_slashes-12 4430572 269.4 ns/op 192 B/op 6 allocs/op Benchmark_SanitizePath/nilFS_-_slashes-12 4455325 268.8 ns/op 192 B/op 6 allocs/op Benchmark_SanitizePath/nilFS_-_slashes-12 4445643 268.4 ns/op 192 B/op 6 allocs/op ``` after: ``` Benchmark_SanitizePath/nilFS_-_urlencoded_chars Benchmark_SanitizePath/nilFS_-_urlencoded_chars-12 5381319 202.2 ns/op 120 B/op 4 allocs/op Benchmark_SanitizePath/nilFS_-_urlencoded_chars-12 6024532 201.0 ns/op 120 B/op 4 allocs/op Benchmark_SanitizePath/nilFS_-_urlencoded_chars-12 6059211 198.1 ns/op 120 B/op 4 allocs/op Benchmark_SanitizePath/nilFS_-_urlencoded_chars-12 6072016 197.9 ns/op 120 B/op 4 allocs/op Benchmark_SanitizePath/dirFS_-_urlencoded_chars Benchmark_SanitizePath/dirFS_-_urlencoded_chars-12 4747710 241.6 ns/op 144 B/op 5 allocs/op Benchmark_SanitizePath/dirFS_-_urlencoded_chars-12 5035154 238.5 ns/op 144 B/op 5 allocs/op Benchmark_SanitizePath/dirFS_-_urlencoded_chars-12 4991553 239.1 ns/op 144 B/op 5 allocs/op Benchmark_SanitizePath/dirFS_-_urlencoded_chars-12 5034110 238.4 ns/op 144 B/op 5 allocs/op Benchmark_SanitizePath/nilFS_-_slashes Benchmark_SanitizePath/nilFS_-_slashes-12 5599615 215.4 ns/op 160 B/op 5 allocs/op Benchmark_SanitizePath/nilFS_-_slashes-12 5578185 215.0 ns/op 160 B/op 5 allocs/op Benchmark_SanitizePath/nilFS_-_slashes-12 5578350 216.8 ns/op 160 B/op 5 allocs/op Benchmark_SanitizePath/nilFS_-_slashes-12 5556883 214.9 ns/op 160 B/op 5 allocs/op ```
WalkthroughThe changes update the internal implementation of the Changes
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🧰 Additional context used🧠 Learnings (3)📓 Common learningsapp_test.go (7)middleware/static/static_test.go (12)⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
🔇 Additional comments (9)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @ReneWerner87, 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 focuses on a targeted performance improvement for the sanitizePath utility function used in the static file serving middleware. The core changes involve optimizing string and byte slice manipulations to reduce allocations and CPU cycles, resulting in a faster and more efficient path sanitization process. Accompanying these changes are new benchmarks to quantify the performance gains and expanded unit tests to ensure the robustness and correctness of the updated logic.
Highlights
- Performance Optimization: The
sanitizePathfunction within the static file serving middleware has been significantly optimized for performance. This includes reducing memory allocations and improving execution speed by leveraging byte-level operations. - Optimized String/Byte Operations: The logic for handling backslashes and URL-encoded characters in
sanitizePathhas been refactored. Instead ofstrings.ReplaceAllandstrings.Contains, the function now usesbytes.IndexByteand conditional byte manipulation, which avoids unnecessary allocations and improves efficiency, especially when no problematic characters are present. - Enhanced Test Coverage and Benchmarks: New benchmarks (
Benchmark_SanitizePath) have been added to validate the performance improvements, showing notable reductions inns/opandB/op. Comprehensive unit tests (Test_SanitizePath,Test_SanitizePath_Error) were also introduced to ensure the correctness of the path sanitization logic across various scenarios, including edge cases and error conditions.
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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| 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 issue 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 is currently in preview and 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 to provide feedback.
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
-
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. ↩
There was a problem hiding this comment.
Pull Request Overview
This PR optimizes the sanitizePath function performance by replacing string operations with more efficient byte-level operations and function calls. The changes reduce memory allocations and improve execution speed as demonstrated by the benchmark results.
Key changes include:
- Replaced
strings.Contains()andstrings.ReplaceAll()with more efficientstrings.IndexByte()and manual byte manipulation - Optimized backslash-to-slash conversion to avoid unnecessary allocations when no backslashes are present
- Added comprehensive test coverage and benchmarks for the
sanitizePathfunction
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| middleware/static/static.go | Optimized sanitizePath function with efficient byte operations and conditional processing |
| middleware/static/static_test.go | Added benchmark tests and comprehensive unit tests for sanitizePath function |
| app_test.go | Minor struct field reordering (comment-only change) |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3601 +/- ##
=======================================
Coverage 90.92% 90.93%
=======================================
Files 111 111
Lines 11230 11236 +6
=======================================
+ Hits 10211 10217 +6
Misses 764 764
Partials 255 255
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:
|
|
Warning Gemini encountered an error creating the review. You can try again by commenting |
before:
after: