chore(metric_extraction): Optimize labels result #15068
Conversation
| if cached, ok := b.resultCache[hash]; ok { | ||
| return cached | ||
| } | ||
|
|
||
| result := NewLabelsResult(b.buf.String(), hash, stream, structuredMetadata, parsed) | ||
| // Now segregate the sorted labels into their categories | ||
| var stream, meta, parsed []labels.Label |
There was a problem hiding this comment.
Should you push it and re-use those slice from the same labels builders ?
| // Check which category this label belongs to | ||
| if labelsContain(b.add[ParsedLabel], l.Name) { | ||
| parsed = append(parsed, l) | ||
| } else if labelsContain(b.add[StructuredMetadataLabel], l.Name) { |
There was a problem hiding this comment.
Might be wise to do this test first.
There was a problem hiding this comment.
LGTM
Some suggestions left.
Do you think we should also cache that call
func (l labelsResult) Labels() labels.Labels {
return flattenLabels(nil, l.stream, l.structuredMetadata, l.parsed)
}
Seems like we could cache if it's being run multiple time on the same object since it's immutable.
|
Thanks! Will address the comments separately in a separate PR. |
|
To address the comments here, re-using the slices as expected seem to increase in-use memory as opposed to initializing the slices for parsed, SM and stream labels. Also, the call |
What this PR does / why we need it:
The current implementation of
LabelsResultmethod used in critical flows within metrics_generation, pipeline, etc fetched UnsortedLabels in the buffer for each category (ParsedLabel, structured metadata, StreamLabel) and individually sorts them. The sorted results are cached in memory. Majority of resource utilisation here was on sorting labels of each of the categories and creating a copy from buffer.The new implementation fetches all Unsorted Labels and sorts them collectively and caches the result first. Individual categories are segregated after caching.
(notice the labels.Copy is gone in the newer implementation in memory profile)
Results:
BenchmarkStreamLineSampleExtractor_Process
Cpu before:
Cpu after
Mem before:
Cpu after:
BenchmarkReadWithStructuredMetadata: create a memchunk and iterate on it
cpu and memory
benchstat result -
Overall Summary from the results:
Excluding some variability in the measurements, The new implementation is at least 28% faster than the older one with a dramatic 89% improvement in memory usage. Each run also took 79.8% fewer allocs/op than the old implementation.
Which issue(s) this PR fixes:
Special notes for your reviewer:
Checklist
CONTRIBUTING.mdguide (required)featPRs are unlikely to be accepted unless a case can be made for the feature actually being a bug fix to existing behavior.docs/sources/setup/upgrade/_index.mddeprecated-config.yamlanddeleted-config.yamlfiles respectively in thetools/deprecated-config-checkerdirectory. Example PR