fiber icon indicating copy to clipboard operation
fiber copied to clipboard
verified publisher icon gofiber

🧹 chore: Improve RemoveEscapeChar performance

Open gaby opened this issue 7 months ago • 3 comments

Summary

  • avoid allocations in RemoveEscapeChar.
  • return early if escape character is not found.
  • start loop at first location of escape character.
  • add missing unit-test and benchmarks for RemoveEscapeCharBytes()

Note:

  • Applying the same improvement to RemoveEscapeCharBytes() resulted in slower performance

gaby avatar May 31 '25 16:05 gaby

[!IMPORTANT]

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

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
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need 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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

coderabbitai[bot] avatar May 31 '25 16:05 coderabbitai[bot]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 90.97%. Comparing base (e9e0367) to head (2a1d0a5). Report is 21 commits behind head on main.

Additional details and impacted files 
@@           Coverage Diff           @@
##             main    #3496   +/-   ##
=======================================
  Coverage   90.97%   90.97%           
=======================================
  Files         110      110           
  Lines       11064    11072    +8     
=======================================
+ Hits        10065    10073    +8     
  Misses        745      745           
  Partials      254      254
Flag Coverage Δ
unittests 90.97% <100.00%> (+<0.01%) :arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

codecov[bot] avatar May 31 '25 16:05 codecov[bot]

New vs Old:

ubuntu@ubuntu:~/Desktop/git/v3-fiber/fiber$ go test -benchmem -run=^$ -bench ^Benchmark_Utils_RemoveEscapeChar_Extended$ github.com/gofiber/fiber/v3 -count=4
goos: linux
goarch: amd64
pkg: github.com/gofiber/fiber/v3
cpu: AMD Ryzen 7 7800X3D 8-Core Processor           
Benchmark_Utils_RemoveEscapeChar_Extended/simple-4         	56217482	        20.90 ns/op	      16 B/op	       1 allocs/op
Benchmark_Utils_RemoveEscapeChar_Extended/simple-4         	53135635	        20.92 ns/op	      16 B/op	       1 allocs/op
Benchmark_Utils_RemoveEscapeChar_Extended/simple-4         	57235556	        20.98 ns/op	      16 B/op	       1 allocs/op
Benchmark_Utils_RemoveEscapeChar_Extended/simple-4         	61021663	        20.90 ns/op	      16 B/op	       1 allocs/op
Benchmark_Utils_RemoveEscapeChar_Extended/no_escape-4      	349460665	         3.415 ns/op	       0 B/op	       0 allocs/op
Benchmark_Utils_RemoveEscapeChar_Extended/no_escape-4      	357582379	         3.343 ns/op	       0 B/op	       0 allocs/op
Benchmark_Utils_RemoveEscapeChar_Extended/no_escape-4      	359267046	         3.331 ns/op	       0 B/op	       0 allocs/op
Benchmark_Utils_RemoveEscapeChar_Extended/no_escape-4      	358141676	         3.348 ns/op	       0 B/op	       0 allocs/op
Benchmark_Utils_RemoveEscapeChar_Extended/multiple_escapes-4         	45769770	        23.66 ns/op	      16 B/op	       1 allocs/op
Benchmark_Utils_RemoveEscapeChar_Extended/multiple_escapes-4         	49362645	        23.47 ns/op	      16 B/op	       1 allocs/op
Benchmark_Utils_RemoveEscapeChar_Extended/multiple_escapes-4         	51237378	        23.55 ns/op	      16 B/op	       1 allocs/op
Benchmark_Utils_RemoveEscapeChar_Extended/multiple_escapes-4         	50928654	        23.22 ns/op	      16 B/op	       1 allocs/op
Benchmark_Utils_RemoveEscapeChar_Extended/leading_escape-4           	62444232	        18.88 ns/op	       8 B/op	       1 allocs/op
Benchmark_Utils_RemoveEscapeChar_Extended/leading_escape-4           	57102145	        18.98 ns/op	       8 B/op	       1 allocs/op
Benchmark_Utils_RemoveEscapeChar_Extended/leading_escape-4           	57521332	        19.12 ns/op	       8 B/op	       1 allocs/op
Benchmark_Utils_RemoveEscapeChar_Extended/leading_escape-4           	63135105	        19.02 ns/op	       8 B/op	       1 allocs/op
Benchmark_Utils_RemoveEscapeChar_Extended/trailing_escape-4          	59377788	        19.66 ns/op	      16 B/op	       1 allocs/op
Benchmark_Utils_RemoveEscapeChar_Extended/trailing_escape-4          	58263765	        19.67 ns/op	      16 B/op	       1 allocs/op
Benchmark_Utils_RemoveEscapeChar_Extended/trailing_escape-4          	61408718	        19.86 ns/op	      16 B/op	       1 allocs/op
Benchmark_Utils_RemoveEscapeChar_Extended/trailing_escape-4          	61223427	        19.84 ns/op	      16 B/op	       1 allocs/op
Benchmark_Utils_RemoveEscapeChar_Extended/only_escape_char-4         	168337048	         7.092 ns/op	       0 B/op	       0 allocs/op
Benchmark_Utils_RemoveEscapeChar_Extended/only_escape_char-4         	172812284	         6.985 ns/op	       0 B/op	       0 allocs/op
Benchmark_Utils_RemoveEscapeChar_Extended/only_escape_char-4         	170908411	         6.982 ns/op	       0 B/op	       0 allocs/op
Benchmark_Utils_RemoveEscapeChar_Extended/only_escape_char-4         	171351502	         6.976 ns/op	       0 B/op	       0 allocs/op
PASS
ok  	github.com/gofiber/fiber/v3	28.308s
ubuntu@ubuntu:~/Desktop/git/v3-fiber/fiber$ go test -benchmem -run=^$ -bench ^Benchmark_Utils_RemoveEscapeChar_Extended$ github.com/gofiber/fiber/v3 -count=4
goos: linux
goarch: amd64
pkg: github.com/gofiber/fiber/v3
cpu: AMD Ryzen 7 7800X3D 8-Core Processor           
Benchmark_Utils_RemoveEscapeChar_Extended/simple-4         	51089068	        22.03 ns/op	      16 B/op	       1 allocs/op
Benchmark_Utils_RemoveEscapeChar_Extended/simple-4         	52090060	        22.09 ns/op	      16 B/op	       1 allocs/op
Benchmark_Utils_RemoveEscapeChar_Extended/simple-4         	57271838	        22.04 ns/op	      16 B/op	       1 allocs/op
Benchmark_Utils_RemoveEscapeChar_Extended/simple-4         	56910784	        22.08 ns/op	      16 B/op	       1 allocs/op
Benchmark_Utils_RemoveEscapeChar_Extended/no_escape-4      	51142645	        22.20 ns/op	      16 B/op	       1 allocs/op
Benchmark_Utils_RemoveEscapeChar_Extended/no_escape-4      	50447330	        22.15 ns/op	      16 B/op	       1 allocs/op
Benchmark_Utils_RemoveEscapeChar_Extended/no_escape-4      	54730243	        21.97 ns/op	      16 B/op	       1 allocs/op
Benchmark_Utils_RemoveEscapeChar_Extended/no_escape-4      	56707700	        22.13 ns/op	      16 B/op	       1 allocs/op
Benchmark_Utils_RemoveEscapeChar_Extended/multiple_escapes-4         	49996170	        22.90 ns/op	      16 B/op	       1 allocs/op
Benchmark_Utils_RemoveEscapeChar_Extended/multiple_escapes-4         	49015768	        23.15 ns/op	      16 B/op	       1 allocs/op
Benchmark_Utils_RemoveEscapeChar_Extended/multiple_escapes-4         	55445076	        23.18 ns/op	      16 B/op	       1 allocs/op
Benchmark_Utils_RemoveEscapeChar_Extended/multiple_escapes-4         	50457178	        22.93 ns/op	      16 B/op	       1 allocs/op
Benchmark_Utils_RemoveEscapeChar_Extended/leading_escape-4           	70829822	        17.15 ns/op	       8 B/op	       1 allocs/op
Benchmark_Utils_RemoveEscapeChar_Extended/leading_escape-4           	68588857	        16.97 ns/op	       8 B/op	       1 allocs/op
Benchmark_Utils_RemoveEscapeChar_Extended/leading_escape-4           	71230917	        16.98 ns/op	       8 B/op	       1 allocs/op
Benchmark_Utils_RemoveEscapeChar_Extended/leading_escape-4           	73361605	        17.04 ns/op	       8 B/op	       1 allocs/op
Benchmark_Utils_RemoveEscapeChar_Extended/trailing_escape-4          	55574439	        22.00 ns/op	      16 B/op	       1 allocs/op
Benchmark_Utils_RemoveEscapeChar_Extended/trailing_escape-4          	51498166	        21.95 ns/op	      16 B/op	       1 allocs/op
Benchmark_Utils_RemoveEscapeChar_Extended/trailing_escape-4          	56275039	        21.89 ns/op	      16 B/op	       1 allocs/op
Benchmark_Utils_RemoveEscapeChar_Extended/trailing_escape-4          	53727992	        22.05 ns/op	      16 B/op	       1 allocs/op
Benchmark_Utils_RemoveEscapeChar_Extended/only_escape_char-4         	210210759	         5.713 ns/op	       0 B/op	       0 allocs/op
Benchmark_Utils_RemoveEscapeChar_Extended/only_escape_char-4         	215446750	         5.567 ns/op	       0 B/op	       0 allocs/op
Benchmark_Utils_RemoveEscapeChar_Extended/only_escape_char-4         	215944836	         5.555 ns/op	       0 B/op	       0 allocs/op
Benchmark_Utils_RemoveEscapeChar_Extended/only_escape_char-4         	216903548	         5.549 ns/op	       0 B/op	       0 allocs/op
PASS
ok  	github.com/gofiber/fiber/v3	28.664s

gaby avatar May 31 '25 16:05 gaby

@gaby what happend to "Benchmark_Utils_RemoveEscapeChar_Extended/no_escape" ? its from 3 to 22ns

ReneWerner87 avatar Jun 30 '25 07:06 ReneWerner87

Will address this later. Branch changed are out of sync

gaby avatar Jul 17 '25 12:07 gaby