Calculate common tags only once. #250
Conversation
This PR extracts commonTags outside of loop. They are not changed inside the loop so there is no need to recalculate them every time. Ta reduce allocations tags slice i create with initial capacity equal to the lenght of application labels. ``` benchmark old ns/op new ns/op delta BenchmarkApp_RegistrationIntents-8 438 415 -5.25% BenchmarkApp_RegistrationIntents-8 436 419 -3.90% BenchmarkApp_RegistrationIntents-8 432 415 -3.94% benchmark old allocs new allocs delta BenchmarkApp_RegistrationIntents-8 4 3 -25.00% BenchmarkApp_RegistrationIntents-8 4 3 -25.00% BenchmarkApp_RegistrationIntents-8 4 3 -25.00% benchmark old bytes new bytes delta BenchmarkApp_RegistrationIntents-8 128 208 +62.50% BenchmarkApp_RegistrationIntents-8 128 208 +62.50% BenchmarkApp_RegistrationIntents-8 128 208 +62.50% ```
| @@ -135,7 +136,7 @@ func marathonAppNameToServiceName(name string, nameSeparator string) string { | |||
| } | |||
|
|
|||
| func labelsToTags(labels map[string]string) []string { | |||
| tags := []string{} | |||
| tags := make([]string, 0, len(labels)) | |||
There was a problem hiding this comment.
len(labels) is ineffective here, the first thing that will happen to it is append, which will allocate new slice to fit tags and commonTags.
There was a problem hiding this comment.
No, this capacity is used to prepare a space for tags appended in for loop below. I'm not trying to optimize allocations of the caller of labelsToTags.
There was a problem hiding this comment.
so why are you benchmarking BenchmarkApp_RegistrationIntents when you not trying to optimize it? ;)
There was a problem hiding this comment.
I'm prepare the benchmark to see the whole change in performance of RegistrationIntents not just labelsToTags.
In this PR I removed one allocation in creation of tags from labels. Instead of starting with empty slice and grow it when needed I start with capacity that will be sufficient for tags. This reduces the allocation of the caller.
I do not agree this is ineffective but I agree it's a micro optimization. labelsToTags is called twice and this change improves the first call. There are cases when the second call will also be improved (port labels count equal size of common tags but none of this label will be a tag) but I'm only interested in most common case in our installation.
If you think you can improve this part of code feel free to use the benchmark and show your results. I'll be happy to accept a pull request.
This PR extracts commonTags outside of loop.
They are not changed inside the loop so there is no
need to recalculate them every time.
Ta reduce allocations tags slice i create with
initial capacity equal to the lenght of application labels.