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

metrics: make gauge_float64 and counter_float64 lock free #27025

Merged
merged 6 commits into from
Apr 4, 2023
Merged
Show file tree
Hide file tree
Changes from 2 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
50 changes: 32 additions & 18 deletions metrics/counter_float64.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
package metrics

import (
"sync"
"math"
"sync/atomic"
)

// CounterFloat64 holds a float64 value that can be incremented and decremented.
Expand Down Expand Up @@ -38,13 +39,13 @@ func NewCounterFloat64() CounterFloat64 {
if !Enabled {
return NilCounterFloat64{}
}
return &StandardCounterFloat64{count: 0.0}
return &StandardCounterFloat64{}
}

// NewCounterFloat64Forced constructs a new StandardCounterFloat64 and returns it no matter if
// the global switch is enabled or not.
func NewCounterFloat64Forced() CounterFloat64 {
return &StandardCounterFloat64{count: 0.0}
return &StandardCounterFloat64{}
}

// NewRegisteredCounterFloat64 constructs and registers a new StandardCounterFloat64.
Expand Down Expand Up @@ -113,41 +114,54 @@ func (NilCounterFloat64) Inc(i float64) {}
func (NilCounterFloat64) Snapshot() CounterFloat64 { return NilCounterFloat64{} }

// StandardCounterFloat64 is the standard implementation of a CounterFloat64 and uses the
// sync.Mutex package to manage a single float64 value.
// atomic to manage a single float64 value.
type StandardCounterFloat64 struct {
mutex sync.Mutex
count float64
floatBits uint64
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not use the new atomic.Uint64 instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because it would result in one extra & operation to pass it to helper functions.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the sake of clarity and adhering to what nowadays is the go-lang recommended practice, please switch to use atomic.Uint64 instead.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unless I am missing something the overhead can't be that bad (?)

}

// Clear sets the counter to zero.
func (c *StandardCounterFloat64) Clear() {
c.mutex.Lock()
defer c.mutex.Unlock()
c.count = 0.0
atomicClearFloat(&c.floatBits)
}

// Count returns the current value.
func (c *StandardCounterFloat64) Count() float64 {
c.mutex.Lock()
defer c.mutex.Unlock()
return c.count
return atomicGetFloat(&c.floatBits)
}

// Dec decrements the counter by the given amount.
func (c *StandardCounterFloat64) Dec(v float64) {
c.mutex.Lock()
defer c.mutex.Unlock()
c.count -= v
atomicAddFloat(&c.floatBits, -v)
}

// Inc increments the counter by the given amount.
func (c *StandardCounterFloat64) Inc(v float64) {
c.mutex.Lock()
defer c.mutex.Unlock()
c.count += v
atomicAddFloat(&c.floatBits, v)
}

// Snapshot returns a read-only copy of the counter.
func (c *StandardCounterFloat64) Snapshot() CounterFloat64 {
return CounterFloat64Snapshot(c.Count())
}

func atomicAddFloat(fbits *uint64, v float64) {
for {
loadedBits := atomic.LoadUint64(fbits)
newBits := math.Float64bits(math.Float64frombits(loadedBits) + v)
if atomic.CompareAndSwapUint64(fbits, loadedBits, newBits) {
break
}
}
}

func atomicGetFloat(addr *uint64) float64 {
return math.Float64frombits(atomic.LoadUint64(addr))
}

func atomicClearFloat(addr *uint64) {
atomic.StoreUint64(addr, 0)
}

func atomicStoreFloat(addr *uint64, v float64) {
atomic.StoreUint64(addr, math.Float64bits(v))
}
19 changes: 18 additions & 1 deletion metrics/counter_float_64_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
package metrics

import "testing"
import (
"sync"
"testing"
)

func BenchmarkCounterFloat64(b *testing.B) {
c := NewCounterFloat64()
Expand All @@ -10,6 +13,20 @@ func BenchmarkCounterFloat64(b *testing.B) {
}
}

func BenchmarkCounterFloat64Parallel(b *testing.B) {
c := NewCounterFloat64()
b.ResetTimer()
var wg sync.WaitGroup
for i := 0; i < b.N; i++ {
wg.Add(1)
go func(i float64) {
c.Inc(i)
wg.Done()
}(float64(i))
}
wg.Wait()
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this benchmark is a bit too parallel -- I suspect there's quite some noise from just maintaining b.N goroutines.

Perhaps a more realistic benchmark wold be do spin up 10 routines, and all ten routines does b.N calls to c.Inc.


func TestCounterFloat64Clear(t *testing.T) {
c := NewCounterFloat64()
c.Inc(1.0)
Expand Down
19 changes: 5 additions & 14 deletions metrics/gauge_float64.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
package metrics

import "sync"

// GaugeFloat64s hold a float64 value that can be set arbitrarily.
type GaugeFloat64 interface {
Snapshot() GaugeFloat64
Expand All @@ -23,9 +21,7 @@ func NewGaugeFloat64() GaugeFloat64 {
if !Enabled {
return NilGaugeFloat64{}
}
return &StandardGaugeFloat64{
value: 0.0,
}
return &StandardGaugeFloat64{}
}

// NewRegisteredGaugeFloat64 constructs and registers a new StandardGaugeFloat64.
Expand Down Expand Up @@ -83,10 +79,9 @@ func (NilGaugeFloat64) Update(v float64) {}
func (NilGaugeFloat64) Value() float64 { return 0.0 }

// StandardGaugeFloat64 is the standard implementation of a GaugeFloat64 and uses
// sync.Mutex to manage a single float64 value.
// atomic to manage a single float64 value.
type StandardGaugeFloat64 struct {
mutex sync.Mutex
value float64
floatBits uint64
}

// Snapshot returns a read-only copy of the gauge.
Expand All @@ -96,16 +91,12 @@ func (g *StandardGaugeFloat64) Snapshot() GaugeFloat64 {

// Update updates the gauge's value.
func (g *StandardGaugeFloat64) Update(v float64) {
g.mutex.Lock()
defer g.mutex.Unlock()
g.value = v
atomicStoreFloat(&g.floatBits, v)
}

// Value returns the gauge's current value.
func (g *StandardGaugeFloat64) Value() float64 {
g.mutex.Lock()
defer g.mutex.Unlock()
return g.value
return atomicGetFloat(&g.floatBits)
}

// FunctionalGaugeFloat64 returns value from given function
Expand Down
19 changes: 18 additions & 1 deletion metrics/gauge_float64_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
package metrics

import "testing"
import (
"sync"
"testing"
)

func BenchmarkGaugeFloat64(b *testing.B) {
g := NewGaugeFloat64()
Expand All @@ -10,6 +13,20 @@ func BenchmarkGaugeFloat64(b *testing.B) {
}
}

func BenchmarkGaugeFloat64Parallel(b *testing.B) {
c := NewGaugeFloat64()
b.ResetTimer()
var wg sync.WaitGroup
for i := 0; i < b.N; i++ {
wg.Add(1)
go func(i float64) {
c.Update(i)
wg.Done()
}(float64(i))
}
wg.Wait()
}

func TestGaugeFloat64(t *testing.T) {
g := NewGaugeFloat64()
g.Update(47.0)
Expand Down