[chore] preallocate slice #24956
[chore] preallocate slice #24956
Conversation
atoulme
force-pushed
the
k8sattrs_prealloc
branch
from
4f27357
to
6ecd931
Compare
| @@ -200,8 +200,8 @@ func withExtractAnnotations(annotations ...FieldExtractConfig) option { | |||
| } | |||
|
|
|||
| func extractFieldRules(fieldType string, fields ...FieldExtractConfig) ([]kube.FieldExtractionRule, error) { | |||
| var rules []kube.FieldExtractionRule | |||
| for _, a := range fields { | |||
| rules := make([]kube.FieldExtractionRule, len(fields)) | |||
There was a problem hiding this comment.
| rules := make([]kube.FieldExtractionRule, len(fields)) | |
| rules := make([]kube.FieldExtractionRule, 0, len(fields)) |
There was a problem hiding this comment.
After merging a few PRs like this, I'm not 100% sure about introducing this linter. In some cases (like this one), where the list is usually small and the iteration is often not triggered due to an empty (or nil) list after range, it's beneficial and more performant to not pre-allocate the slice and keep it nil.
I'd personally vote for not having this linter, but I'm ok with introducing it if @open-telemetry/collector-contrib-approvers think otherwise
There was a problem hiding this comment.
I'd also vote not to use it. Some of the PRs I reviewed seemed to decrease readability slightly, which I'd only find worthwhile if the code is in a hot path. I think it's a simple enough thing to decide and/or change case by case.
There was a problem hiding this comment.
I agree on letting this be a per-component choice. Hopefully we can use benchmarks/profiling to find areas where this is truly useful.
|
Closing this PR as this particular optimization is rejected since it ends up creating additional objects when nil may be returned. |
No description provided.