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

Bring back zero-alloc label-value access for metric vecs #226

Merged
merged 3 commits into from
Aug 17, 2016
Merged
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
23 changes: 23 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,26 @@
## 0.8.0 / 2016-08-17
* [CHANGE] Registry is doing more consistency checks. This might break
existing setups that used to export inconsistent metrics.
* [CHANGE] Pushing to Pushgateway moved to package `push` and changed to allow
arbitrary grouping.
* [CHANGE] Removed `SelfCollector`.
* [CHANGE] Removed `PanicOnCollectError` and `EnableCollectChecks` methods.
* [CHANGE] Moved packages to the prometheus/common repo: `text`, `model`,
`extraction`.
* [CHANGE] Deprecated a number of functions.
* [FEATURE] Allow custom registries. Added `Registerer` and `Gatherer`
interfaces.
* [FEATURE] Separated HTTP exposition, allowing custom HTTP handlers (package
`promhttp`) and enabling the creation of other exposition mechanisms.
* [FEATURE] `MustRegister` is variadic now, allowing registration of many
collectors in one call.
* [FEATURE] Added HTTP API v1 package.
* [ENHANCEMENT] Numerous documentation improvements.
* [ENHANCEMENT] Improved metric sorting.
* [ENHANCEMENT] Inlined fnv64a hashing for improved performance.
* [ENHANCEMENT] Several test improvements.
* [BUGFIX] Handle collisions in MetricVec.

## 0.7.0 / 2015-07-27
* [CHANGE] Rename ExporterLabelPrefix to ExportedLabelPrefix.
* [BUGFIX] Closed gaps in metric consistency check.
Expand Down
2 changes: 1 addition & 1 deletion VERSION
Original file line number Diff line number Diff line change
@@ -1 +1 @@
0.7.0
0.8.0
2 changes: 1 addition & 1 deletion prometheus/counter.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ func (c *counter) Add(v float64) {
// CounterVec embeds MetricVec. See there for a full list of methods with
// detailed documentation.
type CounterVec struct {
MetricVec
*MetricVec
}

// NewCounterVec creates a new CounterVec based on the provided CounterOpts and
Expand Down
2 changes: 1 addition & 1 deletion prometheus/gauge.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func NewGauge(opts GaugeOpts) Gauge {
// (e.g. number of operations queued, partitioned by user and operation
// type). Create instances with NewGaugeVec.
type GaugeVec struct {
MetricVec
*MetricVec
}

// NewGaugeVec creates a new GaugeVec based on the provided GaugeOpts and
Expand Down
2 changes: 1 addition & 1 deletion prometheus/histogram.go
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@ func (h *histogram) Write(out *dto.Metric) error {
// (e.g. HTTP request latencies, partitioned by status code and method). Create
// instances with NewHistogramVec.
type HistogramVec struct {
MetricVec
*MetricVec
}

// NewHistogramVec creates a new HistogramVec based on the provided HistogramOpts and
Expand Down
2 changes: 1 addition & 1 deletion prometheus/summary.go
Original file line number Diff line number Diff line change
Expand Up @@ -390,7 +390,7 @@ func (s quantSort) Less(i, j int) bool {
// (e.g. HTTP request latencies, partitioned by status code and method). Create
// instances with NewSummaryVec.
type SummaryVec struct {
MetricVec
*MetricVec
}

// NewSummaryVec creates a new SummaryVec based on the provided SummaryOpts and
Expand Down
2 changes: 1 addition & 1 deletion prometheus/untyped.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ func NewUntyped(opts UntypedOpts) Untyped {
// labels. This is used if you want to count the same thing partitioned by
// various dimensions. Create instances with NewUntypedVec.
type UntypedVec struct {
MetricVec
*MetricVec
}

// NewUntypedVec creates a new UntypedVec based on the provided UntypedOpts and
Expand Down
192 changes: 110 additions & 82 deletions prometheus/vec.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ type MetricVec struct {

// newMetricVec returns an initialized MetricVec. The concrete value is
// returned for embedding into another struct.
func newMetricVec(desc *Desc, newMetric func(lvs ...string) Metric) MetricVec {
return MetricVec{
func newMetricVec(desc *Desc, newMetric func(lvs ...string) Metric) *MetricVec {
return &MetricVec{
children: map[uint64][]metricWithLabelValues{},
desc: desc,
newMetric: newMetric,
Expand Down Expand Up @@ -102,7 +102,7 @@ func (m *MetricVec) GetMetricWithLabelValues(lvs ...string) (Metric, error) {
return nil, err
}

return m.getOrCreateMetric(h, lvs), nil
return m.getOrCreateMetricWithLabelValues(h, lvs), nil
}

// GetMetricWith returns the Metric for the given Labels map (the label names
Expand All @@ -123,7 +123,7 @@ func (m *MetricVec) GetMetricWith(labels Labels) (Metric, error) {
return nil, err
}

return m.getOrCreateMetric(h, labels), nil
return m.getOrCreateMetricWithLabels(h, labels), nil
}

// WithLabelValues works as GetMetricWithLabelValues, but panics if an error
Expand Down Expand Up @@ -171,7 +171,7 @@ func (m *MetricVec) DeleteLabelValues(lvs ...string) bool {
if err != nil {
return false
}
return m.deleteByHash(h, lvs)
return m.deleteByHashWithLabelValues(h, lvs)
}

// Delete deletes the metric where the variable labels are the same as those
Expand All @@ -193,21 +193,40 @@ func (m *MetricVec) Delete(labels Labels) bool {
return false
}

return m.deleteByHash(h, labels)
return m.deleteByHashWithLabels(h, labels)
}

// deleteByHash removes the metric from the hash bucket h. If there are
// multiple matches in the bucket, use lvs to select a metric and remove only
// that metric.
//
// lvs MUST be of type Labels or []string or this method will panic.
func (m *MetricVec) deleteByHash(h uint64, lvs interface{}) bool {
// deleteByHashWithLabelValues removes the metric from the hash bucket h. If
// there are multiple matches in the bucket, use lvs to select a metric and
// remove only that metric.
func (m *MetricVec) deleteByHashWithLabelValues(h uint64, lvs []string) bool {
metrics, ok := m.children[h]
if !ok {
return false
}

i := m.findMetric(metrics, lvs)
i := m.findMetricWithLabelValues(metrics, lvs)
if i >= len(metrics) {
return false
}

if len(metrics) > 1 {
Copy link
Member

Choose a reason for hiding this comment

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

If we really care about duplicated code, we could even move these 5 lines out in another function.

Copy link
Member Author

Choose a reason for hiding this comment

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

True, but I guess it would not improve readability. So I better leave it as it is.

m.children[h] = append(metrics[:i], metrics[i+1:]...)
} else {
delete(m.children, h)
}
return true
}

// deleteByHashWithLabels removes the metric from the hash bucket h. If there
// are multiple matches in the bucket, use lvs to select a metric and remove
// only that metric.
func (m *MetricVec) deleteByHashWithLabels(h uint64, labels Labels) bool {
metrics, ok := m.children[h]
if !ok {
return false
}
i := m.findMetricWithLabels(metrics, labels)
if i >= len(metrics) {
return false
}
Expand Down Expand Up @@ -258,119 +277,128 @@ func (m *MetricVec) hashLabels(labels Labels) (uint64, error) {
return h, nil
}

// getOrCreateMetric retrieves the metric by hash and label value or creates it
// and returns the new one.
// getOrCreateMetricWithLabelValues retrieves the metric by hash and label value
// or creates it and returns the new one.
//
// lvs MUST be of type Labels or []string or this method will panic.
// This function holds the mutex.
func (m *MetricVec) getOrCreateMetricWithLabelValues(hash uint64, lvs []string) Metric {
m.mtx.RLock()
metric, ok := m.getMetricWithLabelValues(hash, lvs)
m.mtx.RUnlock()
if ok {
return metric
}

m.mtx.Lock()
defer m.mtx.Unlock()
metric, ok = m.getMetricWithLabelValues(hash, lvs)
if !ok {
// Copy to avoid allocation in case wo don't go down this code path.
copiedLVs := make([]string, len(lvs))
copy(copiedLVs, lvs)
metric = m.newMetric(copiedLVs...)
m.children[hash] = append(m.children[hash], metricWithLabelValues{values: copiedLVs, metric: metric})
}
return metric
}

// getOrCreateMetricWithLabelValues retrieves the metric by hash and label value
// or creates it and returns the new one.
//
// This function holds the mutex.
func (m *MetricVec) getOrCreateMetric(hash uint64, lvs interface{}) Metric {
func (m *MetricVec) getOrCreateMetricWithLabels(hash uint64, labels Labels) Metric {
m.mtx.RLock()
metric, ok := m.getMetric(hash, lvs)
metric, ok := m.getMetricWithLabels(hash, labels)
m.mtx.RUnlock()
if ok {
return metric
}

m.mtx.Lock()
defer m.mtx.Unlock()
metric, ok = m.getMetric(hash, lvs)
metric, ok = m.getMetricWithLabels(hash, labels)
if !ok {
lvs := m.copyLabelValues(lvs)
lvs := m.extractLabelValues(labels)
metric = m.newMetric(lvs...)
m.children[hash] = append(m.children[hash], metricWithLabelValues{values: lvs, metric: metric})
}
return metric
}

// getMetric while handling possible collisions in the hash space. Must be
// called while holding read mutex.
//
// lvs must be of type Labels or []string.
func (m *MetricVec) getMetric(h uint64, lvs interface{}) (Metric, bool) {
// getMetricWithLabelValues gets a metric while handling possible collisions in
// the hash space. Must be called while holding read mutex.
func (m *MetricVec) getMetricWithLabelValues(h uint64, lvs []string) (Metric, bool) {
metrics, ok := m.children[h]
if ok {
return m.selectMetric(metrics, lvs)
if i := m.findMetricWithLabelValues(metrics, lvs); i < len(metrics) {
return metrics[i].metric, true
}
}

return nil, false
}

func (m *MetricVec) selectMetric(metrics []metricWithLabelValues, lvs interface{}) (Metric, bool) {
i := m.findMetric(metrics, lvs)

if i < len(metrics) {
return metrics[i].metric, true
// getMetricWithLabels gets a metric while handling possible collisions in
// the hash space. Must be called while holding read mutex.
func (m *MetricVec) getMetricWithLabels(h uint64, labels Labels) (Metric, bool) {
metrics, ok := m.children[h]
if ok {
if i := m.findMetricWithLabels(metrics, labels); i < len(metrics) {
return metrics[i].metric, true
}
}

return nil, false
}

// findMetric returns the index of the matching metric or len(metrics) if not
// found.
func (m *MetricVec) findMetric(metrics []metricWithLabelValues, lvs interface{}) int {
// findMetricWithLabelValues returns the index of the matching metric or
// len(metrics) if not found.
func (m *MetricVec) findMetricWithLabelValues(metrics []metricWithLabelValues, lvs []string) int {
for i, metric := range metrics {
if m.matchLabels(metric.values, lvs) {
if m.matchLabelValues(metric.values, lvs) {
return i
}
}

return len(metrics)
}

func (m *MetricVec) matchLabels(values []string, lvs interface{}) bool {
switch lvs := lvs.(type) {
case []string:
if len(values) != len(lvs) {
return false
}

for i, v := range values {
if v != lvs[i] {
return false
}
// findMetricWithLabels returns the index of the matching metric or len(metrics)
// if not found.
func (m *MetricVec) findMetricWithLabels(metrics []metricWithLabelValues, labels Labels) int {
for i, metric := range metrics {
if m.matchLabels(metric.values, labels) {
return i
}
}
return len(metrics)
}

return true
case Labels:
if len(lvs) != len(values) {
func (m *MetricVec) matchLabelValues(values []string, lvs []string) bool {
if len(values) != len(lvs) {
return false
}
for i, v := range values {
if v != lvs[i] {
return false
}

for i, k := range m.desc.variableLabels {
if values[i] != lvs[k] {
return false
}
}

return true
default:
// If we reach this condition, there is an unexpected type being used
// as a labels value. Either add branch here for the new type or fix
// the bug causing the type to be passed in.
panic("unsupported type")
}
return true
}

// copyLabelValues copies the labels values into common string slice format to
// use when allocating the metric and to keep track of hash collision
// ambiguity.
//
// lvs must be of type Labels or []string or this method will panic.
func (m *MetricVec) copyLabelValues(lvs interface{}) []string {
var labelValues []string
switch lvs := lvs.(type) {
case []string:
labelValues = make([]string, len(lvs))
copy(labelValues, lvs)
case Labels:
labelValues = make([]string, len(lvs))
for i, k := range m.desc.variableLabels {
labelValues[i] = lvs[k]
func (m *MetricVec) matchLabels(values []string, labels Labels) bool {
if len(labels) != len(values) {
return false
}
for i, k := range m.desc.variableLabels {
if values[i] != labels[k] {
return false
}
default:
panic(fmt.Sprintf("unsupported type for lvs: %#v", lvs))
}
return true
}

func (m *MetricVec) extractLabelValues(labels Labels) []string {
labelValues := make([]string, len(labels))
for i, k := range m.desc.variableLabels {
labelValues[i] = labels[k]
}
return labelValues
}
4 changes: 2 additions & 2 deletions prometheus/vec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ func TestCounterVecEndToEndWithCollision(t *testing.T) {
t.Errorf("got label value %q, want %q", got, want)
}
if got, want := m.GetCounter().GetValue(), 1.; got != want {
t.Errorf("got value %d, want %d", got, want)
t.Errorf("got value %f, want %f", got, want)
}
m.Reset()
if err := vec.WithLabelValues("!0IC=VloaY").Write(m); err != nil {
Expand All @@ -235,7 +235,7 @@ func TestCounterVecEndToEndWithCollision(t *testing.T) {
t.Errorf("got label value %q, want %q", got, want)
}
if got, want := m.GetCounter().GetValue(), 2.; got != want {
t.Errorf("got value %d, want %d", got, want)
t.Errorf("got value %f, want %f", got, want)
}
}

Expand Down