From ae6939214cd9f7c7a1c511550ac7da1be1c2a924 Mon Sep 17 00:00:00 2001 From: "Stephen McQuay (smcquay)" Date: Fri, 15 Dec 2017 11:03:24 -0800 Subject: [PATCH 1/2] Adds a faster counter --- prometheus/counter_test.go | 20 +++++++++++++++++--- prometheus/value.go | 22 +++++++++++++++++++--- 2 files changed, 36 insertions(+), 6 deletions(-) diff --git a/prometheus/counter_test.go b/prometheus/counter_test.go index 8d5cd0b..b5eded9 100644 --- a/prometheus/counter_test.go +++ b/prometheus/counter_test.go @@ -28,11 +28,25 @@ func TestCounterAdd(t *testing.T) { ConstLabels: Labels{"a": "1", "b": "2"}, }).(*counter) counter.Inc() - if expected, got := 1., math.Float64frombits(counter.valBits); expected != got { + if expected, got := 0.0, math.Float64frombits(counter.valBits); expected != got { + t.Errorf("Expected %f, got %f.", expected, got) + } + if expected, got := int64(1), counter.valInt; expected != got { t.Errorf("Expected %f, got %f.", expected, got) } counter.Add(42) - if expected, got := 43., math.Float64frombits(counter.valBits); expected != got { + if expected, got := 0.0, math.Float64frombits(counter.valBits); expected != got { + t.Errorf("Expected %f, got %f.", expected, got) + } + if expected, got := int64(43), counter.valInt; expected != got { + t.Errorf("Expected %f, got %f.", expected, got) + } + + counter.Add(24.42) + if expected, got := 24.42, math.Float64frombits(counter.valBits); expected != got { + t.Errorf("Expected %f, got %f.", expected, got) + } + if expected, got := int64(43), counter.valInt; expected != got { t.Errorf("Expected %f, got %f.", expected, got) } @@ -43,7 +57,7 @@ func TestCounterAdd(t *testing.T) { m := &dto.Metric{} counter.Write(m) - if expected, got := `label: label: counter: `, m.String(); expected != got { + if expected, got := `label: label: counter: `, m.String(); expected != got { t.Errorf("expected %q, got %q", expected, got) } } diff --git a/prometheus/value.go b/prometheus/value.go index 4a9cca6..23a1521 100644 --- a/prometheus/value.go +++ b/prometheus/value.go @@ -46,6 +46,9 @@ type value struct { // operations. http://golang.org/pkg/sync/atomic/#pkg-note-BUG valBits uint64 + // valInt is used to store values that are exact integers + valInt int64 + selfCollector desc *Desc @@ -82,15 +85,25 @@ func (v *value) SetToCurrentTime() { v.Set(float64(time.Now().UnixNano()) / 1e9) } +// add adjusts the underlying int64 +func (v *value) add(delta int64) { + atomic.AddInt64(&v.valInt, delta) +} + func (v *value) Inc() { - v.Add(1) + v.add(1) } func (v *value) Dec() { - v.Add(-1) + v.add(-1) } func (v *value) Add(val float64) { + if math.Trunc(val) == val { + atomic.AddInt64(&v.valInt, int64(val)) + return + } + for { oldBits := atomic.LoadUint64(&v.valBits) newBits := math.Float64bits(math.Float64frombits(oldBits) + val) @@ -105,7 +118,10 @@ func (v *value) Sub(val float64) { } func (v *value) Write(out *dto.Metric) error { - val := math.Float64frombits(atomic.LoadUint64(&v.valBits)) + fval := math.Float64frombits(atomic.LoadUint64(&v.valBits)) + ival := atomic.LoadInt64(&v.valInt) + val := fval + float64(ival) + return populateMetric(v.valType, val, v.labelPairs, out) } From 35559538c7d1caa7b8bb623247a0b2cc80298f49 Mon Sep 17 00:00:00 2001 From: "Stephen McQuay (smcquay)" Date: Fri, 15 Dec 2017 11:00:37 -0800 Subject: [PATCH 2/2] Implements review commentary Specifically @beorn7 pointed out that the previous implementation had some shortcomings around large numbers. I've changed the code to match the suggestion in review, as well as added a few test cases. --- prometheus/counter_test.go | 84 ++++++++++++++++++++++++++++++++++++++ prometheus/value.go | 6 +-- 2 files changed, 87 insertions(+), 3 deletions(-) diff --git a/prometheus/counter_test.go b/prometheus/counter_test.go index b5eded9..0b79728 100644 --- a/prometheus/counter_test.go +++ b/prometheus/counter_test.go @@ -126,3 +126,87 @@ func expectPanic(t *testing.T, op func(), errorMsg string) { op() } + +func TestCounterAddInf(t *testing.T) { + counter := NewCounter(CounterOpts{ + Name: "test", + Help: "test help", + }).(*counter) + + counter.Inc() + if expected, got := 0.0, math.Float64frombits(counter.valBits); expected != got { + t.Errorf("Expected %f, got %f.", expected, got) + } + if expected, got := int64(1), counter.valInt; expected != got { + t.Errorf("Expected %f, got %f.", expected, got) + } + + counter.Add(math.Inf(1)) + if expected, got := math.Inf(1), math.Float64frombits(counter.valBits); expected != got { + t.Errorf("valBits expected %f, got %f.", expected, got) + } + if expected, got := int64(1), counter.valInt; expected != got { + t.Errorf("valInts expected %d, got %d.", expected, got) + } + + counter.Inc() + if expected, got := math.Inf(1), math.Float64frombits(counter.valBits); expected != got { + t.Errorf("Expected %f, got %f.", expected, got) + } + if expected, got := int64(2), counter.valInt; expected != got { + t.Errorf("Expected %d, got %d.", expected, got) + } + + m := &dto.Metric{} + counter.Write(m) + + if expected, got := `counter: `, m.String(); expected != got { + t.Errorf("expected %q, got %q", expected, got) + } +} + +func TestCounterAddLarge(t *testing.T) { + counter := NewCounter(CounterOpts{ + Name: "test", + Help: "test help", + }).(*counter) + + // large overflows the underlying type and should therefore be stored in valBits + large := float64(math.MaxInt64 + 1) + counter.Add(large) + if expected, got := large, math.Float64frombits(counter.valBits); expected != got { + t.Errorf("valBits expected %f, got %f.", expected, got) + } + if expected, got := int64(0), counter.valInt; expected != got { + t.Errorf("valInts expected %d, got %d.", expected, got) + } + + m := &dto.Metric{} + counter.Write(m) + + if expected, got := fmt.Sprintf("counter: ", large), m.String(); expected != got { + t.Errorf("expected %q, got %q", expected, got) + } +} + +func TestCounterAddSmall(t *testing.T) { + counter := NewCounter(CounterOpts{ + Name: "test", + Help: "test help", + }).(*counter) + small := 0.000000000001 + counter.Add(small) + if expected, got := small, math.Float64frombits(counter.valBits); expected != got { + t.Errorf("valBits expected %f, got %f.", expected, got) + } + if expected, got := int64(0), counter.valInt; expected != got { + t.Errorf("valInts expected %d, got %d.", expected, got) + } + + m := &dto.Metric{} + counter.Write(m) + + if expected, got := fmt.Sprintf("counter: ", small), m.String(); expected != got { + t.Errorf("expected %q, got %q", expected, got) + } +} diff --git a/prometheus/value.go b/prometheus/value.go index 23a1521..5626e80 100644 --- a/prometheus/value.go +++ b/prometheus/value.go @@ -45,7 +45,6 @@ type value struct { // to go first in the struct to guarantee alignment for atomic // operations. http://golang.org/pkg/sync/atomic/#pkg-note-BUG valBits uint64 - // valInt is used to store values that are exact integers valInt int64 @@ -99,8 +98,9 @@ func (v *value) Dec() { } func (v *value) Add(val float64) { - if math.Trunc(val) == val { - atomic.AddInt64(&v.valInt, int64(val)) + ival := int64(val) + if float64(ival) == val { + v.add(ival) return }