perf: Reduce allocations for JSON and logfmt parser

[jeschkies]

What this PR does / why we need it:
The JSON and logfmt parsers suffer from a high number of allocations when the parsed labels have a high cardinality.

This change reduces the allocations quite a bit by copying the labels bytes to a byte buffer.

go test -bench Benchmark.+HighCardinality -count=10 -benchtime=10s -memprofile=mem.prof -run=^$
                               │ base_no_validation.log │       new_no_validation.log        │
                               │         sec/op         │   sec/op     vs base               │
JSONParserHighCardinality-16                245.1m ± 1%   227.9m ± 4%  -7.02% (p=0.000 n=10)
LogfmtParserHighCardinality-16              447.7m ± 2%   415.7m ± 3%  -7.15% (p=0.000 n=10)
geomean                                     331.3m        307.8m       -7.08%

                               │ base_no_validation.log │        new_no_validation.log         │
                               │          B/op          │     B/op      vs base                │
JSONParserHighCardinality-16               48.81Mi ± 0%   37.33Mi ± 0%  -23.51% (p=0.000 n=10)
LogfmtParserHighCardinality-16             69.48Mi ± 1%   46.39Mi ± 0%  -33.24% (p=0.000 n=10)
geomean                                    58.23Mi        41.61Mi       -28.54%

                               │ base_no_validation.log │        new_no_validation.log        │
                               │       allocs/op        │  allocs/op   vs base                │
JSONParserHighCardinality-16               1323.9k ± 0%   608.2k ± 0%  -54.06% (p=0.000 n=10)
LogfmtParserHighCardinality-16             1848.0k ± 0%   215.4k ± 0%  -88.34% (p=0.000 n=10)
geomean                                     1.564M        361.9k       -76.86%

Special notes for your reviewer:

Checklist

• Reviewed the CONTRIBUTING.md guide (required)
• Documentation added
• Tests updated
• Title matches the required conventional commits format, see here
  • Note that Promtail is considered to be feature complete, and future development for logs collection will be in Grafana Alloy. As such, feat PRs are unlikely to be accepted unless a case can be made for the feature actually being a bug fix to existing behavior.
• Changes that require user attention or interaction to upgrade are documented in docs/sources/setup/upgrade/_index.md
• If the change is deprecating or removing a configuration option, update the deprecated-config.yaml and deleted-config.yaml files respectively in the tools/deprecated-config-checker directory. Example PR

[jeschkies]

This method should ideally go. It's used only once AFAICT

[chaudum]

Why initialize with capacity 0? NewBaseLabelsBuilderWithGrouping initializes with capacity 16.

[chaudum]

I think a comment could help to understand faster how the struct is supposed to work.

[chaudum]

I remember having a log line corruption bug in this area of the JSON parser. We need to be super careful about changes. The tests however assert that the original log line is not modified.

[jeschkies]

Reset is not resetting BaseLabelsBuilder.jsonPath. So I'd say we cannot use unsafe here. The implementation for JSON path would have to change. Btw it's the biggest impact on resources.

buildJSONPathFromPrefixBuffer uses and unsafe string
I was wrong. buildJSONPathFromPrefixBuffer has an append which copies jsonPath = append(jsonPath, partStr)

I'll follow up once this lands.

[chaudum]

Can be removed?

[chaudum]

Should the assertion be removed? Or did you want to uncomment it again?

[jeschkies]

It's enabled. I removed it originally because the original resLbs.Labels() was messing with the benchmark results.

[chaudum]

👍

[chaudum]

Thanks!

[slim-bean]

@claude hello?

[claude]

Claude finished @slim-bean's task —— View job

---

Hello @slim-bean! 👋

I'm here and ready to help with any Loki development tasks you need assistance with. This PR looks like it had some great performance improvements for the JSON and logfmt parsers - nice work by @jeschkies on reducing those allocations!

Let me know if you need any code reviews, analysis, or implementation work.

[slim-bean]

@claude can you look at this code for potential off-by-one type or similar errors please

[claude]

Claude finished @slim-bean's task —— View job

---

Code Review: Potential Off-by-One and Similar Errors

I've analyzed the key files changed in this performance optimization PR. Here are my findings:

✅ No Critical Off-by-One Errors Found

The code appears well-implemented with proper bounds checking. Here's what I examined:

pkg/logql/log/labels_columnar.go - New columnar storage implementation

• stringColumn.get() (lines 58-67): ✅ Correctly handles bounds with index+1 >= len(s.offsets) check
• stringColumn.index() (lines 70-95): ✅ Proper length validation ensures exact matches
• stringColumn.del() (lines 47-49): ✅ Standard slice deletion pattern s.indices[:i], s.indices[i+1:]...

pkg/logql/log/parser.go - Buffer management

• buildSanitizedPrefixFromBuffer() (lines 213-228): ✅ Safe iteration over j.prefixBuffer
• Buffer reuse patterns: ✅ Proper reset with buf = buf[:0] before reuse
• Array indexing: ✅ All slice operations use standard Go patterns

pkg/logql/log/labels.go - Label handling

• Range operations: ✅ Uses Range() methods instead of manual indexing
• Slice operations: ✅ Standard patterns like b.del = b.del[:0] for clearing

⚠️ Minor Observations (not errors)

1. Potential enhancement in labels_columnar.go:142: Uses linear search for ≤50 labels, which is appropriate for this threshold.

2. Review comment addressed: The concern about unsafe operations in buildJSONPathFromPrefixBuffer() (parser.go:241) is actually safe - the code creates a proper string copy via append(jsonPath, partStr).

3. Buffer capacity management: The code uses reasonable initial capacities (64 bytes for value buffers, 16 for labels) and grows as needed.

Test Coverage Analysis

The new columnar implementation has comprehensive tests covering:

• Basic add/get operations
• Deletion scenarios
• Edge cases (empty values, non-existent keys)
• Index boundary conditions

Conclusion

This performance optimization is well-implemented with no off-by-one errors or buffer overflow risks. The columnar storage approach with proper bounds checking and the comprehensive test suite give me confidence in the implementation's correctness.

The significant allocation reductions (23-88% fewer allocations) come from smart buffer reuse rather than unsafe optimizations, which is the right approach.

---

[slim-bean]

@claude can you review this PR for use of global variables and any potential concerns

[claude]

Claude finished @slim-bean's task —— View job

---

Global Variable Analysis for PR #18637

I've completed a thorough analysis of this performance optimization PR for global variable usage and potential concerns.

Todo List:

• Read and analyze key changed files for global variables
• Examine labels.go for global state usage
• Review labels_columnar.go implementation
• Analyze parser.go changes
• Check other changed files for global variable patterns
• Identify potential concurrency or initialization concerns
• Provide comprehensive review findings

✅ Global Variable Review Summary

Global Variables Found:

pkg/logql/log/labels.go:

• EmptyLabelsResult (line 14) - A pre-computed empty labels result for performance
• allCategories (lines 97-101) - Static slice of valid label categories
• smp (line 558) - Global stringMapPool for string map pooling

pkg/logql/log/parser.go:

• trueBytes and falseBytes (lines 38-39) - Static byte slices for boolean values
• Error variables (lines 41-44) - Pre-defined error constants
• removeInvalidUtf (lines 47-52) - Static function for UTF-8 sanitization

pkg/logql/log/fmt.go:

• functionMap (lines 29-73) - Template function registry
• templateFunctions (lines 76-119) - List of allowed Sprig functions
• ansiRegex (line 491) - Compiled regex for ANSI escape sequences

✅ Assessment: All Global Variables are Safe

1. Read-Only Constants & Immutable Data:

• All global variables are either true constants or immutable after initialization
• EmptyLabelsResult, allCategories, trueBytes, falseBytes are never modified
• Error variables and function maps are set once and never changed

2. Thread-Safe Constructs:

• smp (stringMapPool): Uses sync.Pool which is explicitly thread-safe
• ansiRegex: Compiled regex is safe for concurrent read access
• functionMap: Map is populated in init() and only read afterward

3. Proper Initialization:

• init() function (lines 182-189 in fmt.go) properly initializes functionMap with Sprig functions
• Pool initialization follows Go best practices with proper New function
• All globals are initialized before any concurrent access

4. Performance Benefits:

• These globals provide legitimate performance optimizations:
  • Pool reuse reduces allocations
  • Pre-compiled regexes avoid repeated compilation
  • Static byte slices avoid repeated allocations
  • Pre-computed empty results optimize common paths

No Concerns Identified:

❌ No concurrency issues - All globals are read-only or use thread-safe constructs
❌ No initialization races - Proper use of init() and static initialization
❌ No mutable shared state - All data structures are either immutable or safely shared
❌ No resource leaks - Pool usage follows established patterns

Recommendation: ✅ APPROVED

The global variable usage in this PR is exemplary for performance-focused code. All globals serve clear performance purposes and are implemented using Go concurrency best practices. The optimization approach of using pools, pre-compiled regexes, and cached immutable values is standard in high-performance Go applications like Loki.

---