pkg/trace/agent: improve NormalizeTag #2951
Conversation
3fffb4a to
c10955b
Compare
Codecov Report
@@ Coverage Diff @@
## master #2951 +/- ##
==========================================
+ Coverage 56.45% 56.51% +0.06%
==========================================
Files 482 482
Lines 34185 34223 +38
==========================================
+ Hits 19298 19340 +42
+ Misses 13728 13724 -4
Partials 1159 1159
|
| continue | ||
| } | ||
|
|
||
| c = unicode.ToLower(c) | ||
| switch { | ||
| // handle always valid cases | ||
| case unicode.IsLetter(c) || c == ':': |
There was a problem hiding this comment.
We can merge the 1st and 3rd case together?
There was a problem hiding this comment.
Actually there is a catch here: switch case statements are evaluated top-to-bottom and the idea here is that you can not go past the second case (length = 0) if the character isn't a letter or a colon. In other words only letters or colons can start the tag name.
If we merge the two, it will alter this rule.
| buf.WriteRune(c) | ||
| lastWasUnderscore = false | ||
| // lower-case | ||
| norm[i] += 'a' - 'A' |
There was a problem hiding this comment.
Nit: we can put +32 directly, saying that it moves a ascii upper-case to ascii lower-case equivalent.
There was a problem hiding this comment.
The compiler should auto compute this expression so we can leave this which I think gives better readability
There was a problem hiding this comment.
I agree with @erichgess here. I liked this way and I even checked how the standard library does it, so I'd rather keep it if that's ok: https://github.com/golang/go/blob/05f8b44d5edc2960eff106e5e780cf83535d0533/src/unicode/letter.go#L267
c10955b to
3c3fe1d
Compare
|
I'd say this is good to review if you'd like to take another look @LotharSee |
|
Has a bug that was fixed in #2957 |
Improves performance of
NormalizeTagInstead of creating a buffer, create a series of tuples representing cuts that need to happen inside the tag string, and apply them later. If no changes need to happen, the same string is returned without any extra allocations.
As can be seen in the benchmarks, the algorithm becomes slower the more cuts need to be made. For common scenarios with no changes or few changes, the improvement is around 400% for memory.