go/stats: improve performance of safeJoinLabels

[mattrobenolt]

safeJoinLabels is in a pretty hot path for builtinbackupengine, every Read/Write file operation requires 2 calls to this function through (*scopedStats).TimedIncrementBytes.

An optimization pass to this function was pretty low hanging fruit compared to trying to materialize/cache the label values correctly.

$ benchstat {before,after}.txt
goos: darwin
goarch: arm64
pkg: vitess.io/vitess/go/stats
cpu: Apple M1 Max
                              │  before.txt  │              after.txt              │
                              │    sec/op    │   sec/op     vs base                │
SafeJoinLabels/no_combined-10   204.40n ± 1%   84.58n ± 0%  -58.62% (p=0.000 n=10)
SafeJoinLabels/combined-10       87.54n ± 9%   30.98n ± 1%  -64.60% (p=0.000 n=10)
geomean                          133.8n        51.19n       -61.73%

                              │ before.txt  │             after.txt              │
                              │    B/op     │    B/op     vs base                │
SafeJoinLabels/no_combined-10   152.00 ± 0%   48.00 ± 0%  -68.42% (p=0.000 n=10)
SafeJoinLabels/combined-10      104.00 ± 0%   24.00 ± 0%  -76.92% (p=0.000 n=10)
geomean                          125.7        33.94       -73.00%

                              │ before.txt │             after.txt              │
                              │ allocs/op  │ allocs/op   vs base                │
SafeJoinLabels/no_combined-10   4.000 ± 0%   1.000 ± 0%  -75.00% (p=0.000 n=10)
SafeJoinLabels/combined-10      2.000 ± 0%   1.000 ± 0%  -50.00% (p=0.000 n=10)
geomean                         2.828        1.000       -64.64%

This showed up as the top offender of allocations in pprofs I was looking at:

Related Issue(s)

Checklist

• "Backport to:" labels have been added if this change should be back-ported to release branches
• If this change is to be back-ported to previous releases, a justification is included in the PR description
• Tests were added or are not required
• Did the new or modified tests pass consistently locally and on CI?
• Documentation was added or is not required

Deployment Notes

None

[vitess-bot]

Review Checklist

Hello reviewers! 👋 Please follow this checklist when reviewing this Pull Request.

General

• Ensure that the Pull Request has a descriptive title.
• Ensure there is a link to an issue (except for internal cleanup and flaky test fixes), new features should have an RFC that documents use cases and test cases.

Tests

• Bug fixes should have at least one unit or end-to-end test, enhancement and new features should have a sufficient number of tests.

Documentation

• Apply the release notes (needs details) label if users need to know about this change.
• New features should be documented.
• There should be some code comments as to why things are implemented the way they are.
• There should be a comment at the top of each new or modified test to explain what the test does.

New flags

• Is this flag really necessary?
• Flag names must be clear and intuitive, use dashes (-), and have a clear help text.

If a workflow is added or modified:

• Each item in Jobs should be named in order to mark it as required.
• If the workflow needs to be marked as required, the maintainer team must be notified.

Backward compatibility

• Protobuf changes should be wire-compatible.
• Changes to _vt tables and RPCs need to be backward compatible.
• RPC changes should be compatible with vitess-operator
• If a flag is removed, then it should also be removed from vitess-operator and arewefastyet, if used there.
• vtctl command output order should be stable and awk-able.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 69.42%. Comparing base (8b37c52) to head (623a744).
Report is 7 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #16953      +/-   ##
==========================================
+ Coverage   69.41%   69.42%   +0.01%     
==========================================
  Files        1570     1570              
  Lines      203929   204000      +71     
==========================================
+ Hits       141551   141622      +71     
  Misses      62378    62378              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[systay]

LGTM

[deepthi]

Nice!

[deepthi]

I think it's ok to merge this without requiring an issue.