perf: avoid string concatenation in loops#7572
Conversation
| copy := "" | ||
|
|
||
| var copySb12 strings.Builder | ||
| for _, c := range s { |
There was a problem hiding this comment.
Maybe a strings.ReplaceAll would be even better
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #7572 +/- ##
==========================================
+ Coverage 55.70% 62.57% +6.87%
==========================================
Files 224 278 +54
Lines 10016 18567 +8551
==========================================
+ Hits 5579 11619 +6040
- Misses 3978 6260 +2282
- Partials 459 688 +229 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@catenacyber The naming of the variables like |
Sure, I did it in a fixup commit. These names come from the automatic tool, that aimes to produce correct code in all cases, even if not the best code ;-) |
|
Can you fix the DCO error? |
acf3fa5 to
8ba6432
Compare
|
Commits squashed and signed off to fix DCO |
Unlikely - mostly sourced from the config file.
Would it be possible to also add |
|
Trying in a new commit on top of that Note that concat-loop is not merged yet : catenacyber/perfsprint#58 |
|
Cool, lets enable it once merged (and if not enabled by default) 👍 @catenacyber can you fix DCO? Otherwise LGTM |
Apply perfpsrint linter Signed-off-by: Philippe Antoine <contact@catenacyber.fr>
Signed-off-by: Philippe Antoine <contact@catenacyber.fr>
ca0446f to
1c41df0
Compare
|
Oops... just fixed up, signed off and force-pushed... @thevilledev nice work on tackling all oss-fuzz issues by the way 👏 |
thevilledev
left a comment
There was a problem hiding this comment.
Thank you @catenacyber! 🥳 It's been fun fixing those
LGTM
1. Why is this pull request needed and what does it do?
This PR is about improving performance
It applies perfpsrint linter to remove string concatenation in loops, which has a quadratic complexity
2. Which issues (if any) are related?
None that I know
Do you have an opinion if the strings looked at here can have a big or unbounded length ? like
rewriteToExpandin plugin/auto/regexp.go ormaybeUnescapein plugin/route53/route53.go3. Which documentation changes (if any) need to be made?
Behavior should remain the same
4. Does this introduce a backward incompatible change or deprecation?
It should not