perf: better string concatenation#9705
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9705 +/- ##
=======================================
Coverage 60.97% 60.98%
=======================================
Files 384 384
Lines 36609 36612 +3
=======================================
+ Hits 22322 22327 +5
+ Misses 12678 12677 -1
+ Partials 1609 1608 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
2e2aab6 to
8bf739b
Compare
8bf739b to
5b1094a
Compare
|
@kaovilai could you please take a second look |
fed4c36 to
8e2d4f0
Compare
|
@blackpiglet I have updated please squash when merging |
Signed-off-by: emirot <emirot.nolan@gmail.com> Signed-off-by: nolanemirot <nolan.emirot@broadcom.com> Signed-off-by: emirot <emirot.nolan@gmail.com>
…elero-io#9693) * Enhance backup deletion logic to handle tarball download failures and clean up associated CSI VolumeSnapshotContents Signed-off-by: Priyansh Choudhary <im1706@gmail.com> * added changelog Signed-off-by: Priyansh Choudhary <im1706@gmail.com> * Refactor error handling in backup deletion Signed-off-by: Priyansh Choudhary <im1706@gmail.com> * Refactor backup deletion logic to skip CSI snapshot cleanup on tarball download failure Signed-off-by: Priyansh Choudhary <im1706@gmail.com> * prevent backup deletion when errors occur Signed-off-by: Priyansh Choudhary <im1706@gmail.com> * added logger Signed-off-by: Priyansh Choudhary <im1706@gmail.com> Signed-off-by: emirot <emirot.nolan@gmail.com>
Signed-off-by: emirot <emirot.nolan@gmail.com>
…ion (velero-io#9700) * Add delay to avoid race conditions during VolumeSnapshotContent deletion Signed-off-by: Priyansh Choudhary <im1706@gmail.com> * updated changelog Signed-off-by: Priyansh Choudhary <im1706@gmail.com> * Updated Changelog Signed-off-by: Priyansh Choudhary <im1706@gmail.com> Signed-off-by: emirot <emirot.nolan@gmail.com>
Signed-off-by: Lyndon-Li <lyonghui@vmware.com> Signed-off-by: emirot <emirot.nolan@gmail.com>
Signed-off-by: Lyndon-Li <lyonghui@vmware.com> Signed-off-by: emirot <emirot.nolan@gmail.com>
Signed-off-by: Lyndon-Li <lyonghui@vmware.com> Signed-off-by: emirot <emirot.nolan@gmail.com>
Signed-off-by: Lyndon-Li <lyonghui@vmware.com> Signed-off-by: emirot <emirot.nolan@gmail.com>
Update the community page to add the correct links to community meeting and meeting notes. I also removed the referece of google group as I confirmed the last message was sent 2 years ago. Signed-off-by: Daniel Jiang <daniel.jiang@broadcom.com> Signed-off-by: emirot <emirot.nolan@gmail.com>
Signed-off-by: emirot <emirot.nolan@gmail.com>
ae84800 to
60597b9
Compare
There was a problem hiding this comment.
Pull request overview
Refactors several diagnostic/string-building helpers to avoid inefficient string concatenation patterns that newer golangci-lint/perfsprint versions can flag.
Changes:
- Switch
DiagnosePVC,DiagnosePod, andDiagnoseVSto build output viastrings.Builder+fmt.Fprintf. - Update
GetPodTerminateMessageto usestrings.Builderand add the needed import. - Add an unreleased changelog entry for the performance-focused refactor.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| pkg/util/kube/pvc_pv.go | Refactors PVC diagnostic string construction to use strings.Builder. |
| pkg/util/kube/pod.go | Uses strings.Builder for pod termination/diagnostic message assembly; adds strings import. |
| pkg/util/csi/volume_snapshot.go | Refactors VolumeSnapshot diagnostic string construction to use strings.Builder. |
| changelogs/unreleased/9705-emirot | Adds changelog entry describing the performance improvement. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if containerStatus.State.Terminated != nil { | ||
| if containerStatus.State.Terminated.Message != "" { | ||
| message += containerStatus.State.Terminated.Message + "/" | ||
| message.WriteString(containerStatus.State.Terminated.Message + "/") |
There was a problem hiding this comment.
Inside the loop, this still performs string concatenation (Terminated.Message + "/"), which creates a new temporary string on every iteration and may continue to trigger the concat-loop lint/performance warning. Prefer writing the message and delimiter separately (e.g., write the message then write the "/" delimiter) to avoid per-iteration allocations.
| message.WriteString(containerStatus.State.Terminated.Message + "/") | |
| message.WriteString(containerStatus.State.Terminated.Message) | |
| message.WriteString("/") |
* perf: better string concatenation Signed-off-by: emirot <emirot.nolan@gmail.com> Signed-off-by: nolanemirot <nolan.emirot@broadcom.com> Signed-off-by: emirot <emirot.nolan@gmail.com> * fix: backup deletion silently succeeds when tarball download fails (velero-io#9693) * Enhance backup deletion logic to handle tarball download failures and clean up associated CSI VolumeSnapshotContents Signed-off-by: Priyansh Choudhary <im1706@gmail.com> * added changelog Signed-off-by: Priyansh Choudhary <im1706@gmail.com> * Refactor error handling in backup deletion Signed-off-by: Priyansh Choudhary <im1706@gmail.com> * Refactor backup deletion logic to skip CSI snapshot cleanup on tarball download failure Signed-off-by: Priyansh Choudhary <im1706@gmail.com> * prevent backup deletion when errors occur Signed-off-by: Priyansh Choudhary <im1706@gmail.com> * added logger Signed-off-by: Priyansh Choudhary <im1706@gmail.com> Signed-off-by: emirot <emirot.nolan@gmail.com> * perf: better string concatenation Signed-off-by: emirot <emirot.nolan@gmail.com> * Add delay to avoid race conditions during VolumeSnapshotContent deletion (velero-io#9700) * Add delay to avoid race conditions during VolumeSnapshotContent deletion Signed-off-by: Priyansh Choudhary <im1706@gmail.com> * updated changelog Signed-off-by: Priyansh Choudhary <im1706@gmail.com> * Updated Changelog Signed-off-by: Priyansh Choudhary <im1706@gmail.com> Signed-off-by: emirot <emirot.nolan@gmail.com> * block data mover design Signed-off-by: Lyndon-Li <lyonghui@vmware.com> Signed-off-by: emirot <emirot.nolan@gmail.com> * block data mover design Signed-off-by: Lyndon-Li <lyonghui@vmware.com> Signed-off-by: emirot <emirot.nolan@gmail.com> * irregular volume size Signed-off-by: Lyndon-Li <lyonghui@vmware.com> Signed-off-by: emirot <emirot.nolan@gmail.com> * block data mover design Signed-off-by: Lyndon-Li <lyonghui@vmware.com> Signed-off-by: emirot <emirot.nolan@gmail.com> * Update the "community" page of website (velero-io#9722) Update the community page to add the correct links to community meeting and meeting notes. I also removed the referece of google group as I confirmed the last message was sent 2 years ago. Signed-off-by: Daniel Jiang <daniel.jiang@broadcom.com> Signed-off-by: emirot <emirot.nolan@gmail.com> * perf: better string concatenation Signed-off-by: emirot <emirot.nolan@gmail.com> --------- Signed-off-by: emirot <emirot.nolan@gmail.com> Signed-off-by: nolanemirot <nolan.emirot@broadcom.com> Signed-off-by: Priyansh Choudhary <im1706@gmail.com> Signed-off-by: Lyndon-Li <lyonghui@vmware.com> Signed-off-by: Daniel Jiang <daniel.jiang@broadcom.com> Co-authored-by: Priyansh Choudhary <im1706@gmail.com> Co-authored-by: nolanemirot <nolan.emirot@broadcom.com> Co-authored-by: Lyndon-Li <lyonghui@vmware.com> Co-authored-by: Daniel Jiang <daniel.jiang@broadcom.com>
* perf: better string concatenation Signed-off-by: emirot <emirot.nolan@gmail.com> Signed-off-by: nolanemirot <nolan.emirot@broadcom.com> Signed-off-by: emirot <emirot.nolan@gmail.com> * fix: backup deletion silently succeeds when tarball download fails (velero-io#9693) * Enhance backup deletion logic to handle tarball download failures and clean up associated CSI VolumeSnapshotContents Signed-off-by: Priyansh Choudhary <im1706@gmail.com> * added changelog Signed-off-by: Priyansh Choudhary <im1706@gmail.com> * Refactor error handling in backup deletion Signed-off-by: Priyansh Choudhary <im1706@gmail.com> * Refactor backup deletion logic to skip CSI snapshot cleanup on tarball download failure Signed-off-by: Priyansh Choudhary <im1706@gmail.com> * prevent backup deletion when errors occur Signed-off-by: Priyansh Choudhary <im1706@gmail.com> * added logger Signed-off-by: Priyansh Choudhary <im1706@gmail.com> Signed-off-by: emirot <emirot.nolan@gmail.com> * perf: better string concatenation Signed-off-by: emirot <emirot.nolan@gmail.com> * Add delay to avoid race conditions during VolumeSnapshotContent deletion (velero-io#9700) * Add delay to avoid race conditions during VolumeSnapshotContent deletion Signed-off-by: Priyansh Choudhary <im1706@gmail.com> * updated changelog Signed-off-by: Priyansh Choudhary <im1706@gmail.com> * Updated Changelog Signed-off-by: Priyansh Choudhary <im1706@gmail.com> Signed-off-by: emirot <emirot.nolan@gmail.com> * block data mover design Signed-off-by: Lyndon-Li <lyonghui@vmware.com> Signed-off-by: emirot <emirot.nolan@gmail.com> * block data mover design Signed-off-by: Lyndon-Li <lyonghui@vmware.com> Signed-off-by: emirot <emirot.nolan@gmail.com> * irregular volume size Signed-off-by: Lyndon-Li <lyonghui@vmware.com> Signed-off-by: emirot <emirot.nolan@gmail.com> * block data mover design Signed-off-by: Lyndon-Li <lyonghui@vmware.com> Signed-off-by: emirot <emirot.nolan@gmail.com> * Update the "community" page of website (velero-io#9722) Update the community page to add the correct links to community meeting and meeting notes. I also removed the referece of google group as I confirmed the last message was sent 2 years ago. Signed-off-by: Daniel Jiang <daniel.jiang@broadcom.com> Signed-off-by: emirot <emirot.nolan@gmail.com> * perf: better string concatenation Signed-off-by: emirot <emirot.nolan@gmail.com> --------- Signed-off-by: emirot <emirot.nolan@gmail.com> Signed-off-by: nolanemirot <nolan.emirot@broadcom.com> Signed-off-by: Priyansh Choudhary <im1706@gmail.com> Signed-off-by: Lyndon-Li <lyonghui@vmware.com> Signed-off-by: Daniel Jiang <daniel.jiang@broadcom.com> Co-authored-by: Priyansh Choudhary <im1706@gmail.com> Co-authored-by: nolanemirot <nolan.emirot@broadcom.com> Co-authored-by: Lyndon-Li <lyonghui@vmware.com> Co-authored-by: Daniel Jiang <daniel.jiang@broadcom.com>
Thank you for contributing to Velero!
Please add a summary of your change
Better practice and newer version of golinci-lint will raise an issue
like so
Does your change fix a particular issue?
No
Fixes #(issue)
Please indicate you've done the following:
make new-changelog) or comment/kind changelog-not-requiredon this PR.site/content/docs/main.