Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Convert LabelsBuilder baseMap to *sync.Map from map[string]string since we share the map now to reduce allocations #11582

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion pkg/logql/log/fmt.go
Original file line number Diff line number Diff line change
Expand Up @@ -395,7 +395,13 @@ func (lf *LabelsFormatter) Process(ts int64, l []byte, lbs *LabelsBuilder) ([]by
continue
}
lf.buf.Reset()
if len(m) == 0 {
isEmpty := true
m.Range(func(_, _ interface{}) bool {
isEmpty = false
return false
})

if isEmpty {
lbs.IntoMap(m)
}
if err := f.tmpl.Execute(lf.buf, m); err != nil {
Expand Down
61 changes: 40 additions & 21 deletions pkg/logql/log/labels.go
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ type BaseLabelsBuilder struct {
// LabelsBuilder is the same as labels.Builder but tailored for this package.
type LabelsBuilder struct {
base labels.Labels
baseMap map[string]string
baseMap *sync.Map
buf labels.Labels
currentResult LabelsResult
groupedResult LabelsResult
Expand Down Expand Up @@ -457,58 +457,77 @@ func newStringMapPool() *stringMapPool {
return &stringMapPool{
pool: sync.Pool{
New: func() interface{} {
return make(map[string]string)
return &sync.Map{}
},
},
}
}

func (s *stringMapPool) Get() map[string]string {
m := s.pool.Get().(map[string]string)
clear(m)
func (s *stringMapPool) Get() *sync.Map {
m := s.pool.Get().(*sync.Map)
m.Range(func(k, v interface{}) bool {
m.Delete(k)
return true
})
return m
}

func (s *stringMapPool) Put(m map[string]string) {
func (s *stringMapPool) Put(m *sync.Map) {
s.pool.Put(m)
}

var smp = newStringMapPool()

// puts labels entries into an existing map, it is up to the caller to
// properly clear the map if it is going to be reused
func (b *LabelsBuilder) IntoMap(m map[string]string) {
func (b *LabelsBuilder) IntoMap(m *sync.Map) {
if !b.hasDel() && !b.hasAdd() && !b.HasErr() {
if b.baseMap == nil {
b.baseMap = b.base.Map()
for k, v := range b.baseMap {
m[k] = v
isEmpty := true
b.baseMap.Range(func(_, _ interface{}) bool {
isEmpty = false
return false
})
if isEmpty {
syncMap := sync.Map{}
base := b.base.Map()
for k, v := range base {
syncMap.Store(k, v)
}
b.baseMap = &syncMap
}
return
}
b.buf = b.UnsortedLabels(b.buf)
// todo should we also cache maps since limited by the result ?
// Maps also don't create a copy of the labels.
for _, l := range b.buf {
m[l.Name] = l.Value
m.Store(l.Name, l.Value)
}
}

func (b *LabelsBuilder) Map() map[string]string {
func (b *LabelsBuilder) Map() *sync.Map {
if !b.hasDel() && !b.hasAdd() && !b.HasErr() {
if b.baseMap == nil {
b.baseMap = b.base.Map()
isEmpty := true
b.baseMap.Range(func(_, _ interface{}) bool {
isEmpty = false
return false
})
if isEmpty {
syncMap := sync.Map{}
base := b.base.Map()
for k, v := range base {
syncMap.Store(k, v)
}
b.baseMap = &syncMap
}
return b.baseMap
}
b.buf = b.UnsortedLabels(b.buf)
// todo should we also cache maps since limited by the result ?
// Maps also don't create a copy of the labels.
res := smp.Get()
clear(res)
res.Range(func(k, v interface{}) bool {
res.Delete(k)
return true
})
for _, l := range b.buf {
res[l.Name] = l.Value
res.Store(l.Name, l.Value)
}
return res
}
Expand Down