Repository: libp2p/go-libp2p
Pull request #2054 (2023-02-04 22:21:54)

metrics: use a single slice pool for all metrics tracer
This PR:
1. Adds a global string slice pool, to be used by all of our metrics collectors. Using a single pool reduces code duplication and makes the `sync.Pool` more efficient.
2. Makes use of this pool for the event bus metrics, bringing down the number of allocs for events from 1 to 0.

cc @sukunrt You can use this for the AutoNAT metrics.


Patch:
diff --git a/p2p/host/eventbus/basic_metrics.go b/p2p/host/eventbus/basic_metrics.go
index 0a6edf8a..3c306b6a 100644
--- a/p2p/host/eventbus/basic_metrics.go
+++ b/p2p/host/eventbus/basic_metrics.go
@@ -5,6 +5,8 @@ import (
 	"strings"
 	"sync"
 
+	"github.com/libp2p/go-libp2p/p2p/metricshelper"
+
 	"github.com/prometheus/client_golang/prometheus"
 )
 
@@ -107,23 +109,43 @@ func NewMetricsTracer(opts ...MetricsTracerOption) MetricsTracer {
 }
 
 func (m *metricsTracer) EventEmitted(typ reflect.Type) {
-	eventsEmitted.WithLabelValues(strings.TrimPrefix(typ.String(), "event.")).Inc()
+	tags := metricshelper.GetStringSlice()
+	defer metricshelper.PutStringSlice(tags)
+
+	*tags = append(*tags, strings.TrimPrefix(typ.String(), "event."))
+	eventsEmitted.WithLabelValues(*tags...).Inc()
 }
 
 func (m *metricsTracer) AddSubscriber(typ reflect.Type) {
-	totalSubscribers.WithLabelValues(strings.TrimPrefix(typ.String(), "event.")).Inc()
+	tags := metricshelper.GetStringSlice()
+	defer metricshelper.PutStringSlice(tags)
+
+	*tags = append(*tags, strings.TrimPrefix(typ.String(), "event."))
+	totalSubscribers.WithLabelValues(*tags...).Inc()
 }
 
 func (m *metricsTracer) RemoveSubscriber(typ reflect.Type) {
-	totalSubscribers.WithLabelValues(strings.TrimPrefix(typ.String(), "event.")).Dec()
+	tags := metricshelper.GetStringSlice()
+	defer metricshelper.PutStringSlice(tags)
+
+	*tags = append(*tags, strings.TrimPrefix(typ.String(), "event."))
+	totalSubscribers.WithLabelValues(*tags...).Dec()
 }
 
 func (m *metricsTracer) SubscriberQueueLength(name string, n int) {
-	subscriberQueueLength.WithLabelValues(name).Set(float64(n))
+	tags := metricshelper.GetStringSlice()
+	defer metricshelper.PutStringSlice(tags)
+
+	*tags = append(*tags, name)
+	subscriberQueueLength.WithLabelValues(*tags...).Set(float64(n))
 }
 
 func (m *metricsTracer) SubscriberQueueFull(name string, isFull bool) {
-	observer := subscriberQueueFull.WithLabelValues(name)
+	tags := metricshelper.GetStringSlice()
+	defer metricshelper.PutStringSlice(tags)
+
+	*tags = append(*tags, name)
+	observer := subscriberQueueFull.WithLabelValues(*tags...)
 	if isFull {
 		observer.Set(1)
 	} else {
@@ -132,5 +154,9 @@ func (m *metricsTracer) SubscriberQueueFull(name string, isFull bool) {
 }
 
 func (m *metricsTracer) SubscriberEventQueued(name string) {
-	subscriberEventQueued.WithLabelValues(name).Inc()
+	tags := metricshelper.GetStringSlice()
+	defer metricshelper.PutStringSlice(tags)
+
+	*tags = append(*tags, name)
+	subscriberEventQueued.WithLabelValues(*tags...).Inc()
 }
diff --git a/p2p/metricshelper/pool.go b/p2p/metricshelper/pool.go
new file mode 100644
index 00000000..3290ed5a
--- /dev/null
+++ b/p2p/metricshelper/pool.go
@@ -0,0 +1,26 @@
+package metricshelper
+
+import (
+	"fmt"
+	"sync"
+)
+
+const capacity = 8
+
+var stringPool = sync.Pool{New: func() any {
+	s := make([]string, 0, capacity)
+	return &s
+}}
+
+func GetStringSlice() *[]string {
+	s := stringPool.Get().(*[]string)
+	*s = (*s)[:0]
+	return s
+}
+
+func PutStringSlice(s *[]string) {
+	if c := cap(*s); c < capacity {
+		panic(fmt.Sprintf("expected a string slice with capacity 8 or greater, got %d", c))
+	}
+	stringPool.Put(s)
+}
diff --git a/p2p/net/swarm/swarm_metrics.go b/p2p/net/swarm/swarm_metrics.go
index 16f431b5..30edd8bc 100644
--- a/p2p/net/swarm/swarm_metrics.go
+++ b/p2p/net/swarm/swarm_metrics.go
@@ -10,6 +10,7 @@ import (
 
 	"github.com/libp2p/go-libp2p/core/crypto"
 	"github.com/libp2p/go-libp2p/core/network"
+	"github.com/libp2p/go-libp2p/p2p/metricshelper"
 
 	ma "github.com/multiformats/go-multiaddr"
 
@@ -93,19 +94,6 @@ func NewMetricsTracer() *metricsTracer {
 	return &metricsTracer{}
 }
 
-var stringPool = sync.Pool{New: func() any {
-	s := make([]string, 0, 8)
-	return &s
-}}
-
-func getStringSlice() *[]string {
-	s := stringPool.Get().(*[]string)
-	*s = (*s)[:0]
-	return s
-}
-
-func putStringSlice(s *[]string) { stringPool.Put(s) }
-
 func getDirection(dir network.Direction) string {
 	switch dir {
 	case network.DirOutbound:
@@ -132,8 +120,8 @@ func appendConnectionState(tags []string, cs network.ConnectionState) []string {
 }
 
 func (m *metricsTracer) OpenedConnection(dir network.Direction, p crypto.PubKey, cs network.ConnectionState) {
-	tags := getStringSlice()
-	defer putStringSlice(tags)
+	tags := metricshelper.GetStringSlice()
+	defer metricshelper.PutStringSlice(tags)
 
 	*tags = append(*tags, getDirection(dir))
 	*tags = appendConnectionState(*tags, cs)
@@ -146,8 +134,9 @@ func (m *metricsTracer) OpenedConnection(dir network.Direction, p crypto.PubKey,
 }
 
 func (m *metricsTracer) ClosedConnection(dir network.Direction, duration time.Duration, cs network.ConnectionState) {
-	tags := getStringSlice()
-	defer putStringSlice(tags)
+	tags := metricshelper.GetStringSlice()
+	defer metricshelper.PutStringSlice(tags)
+
 	*tags = append(*tags, getDirection(dir))
 	*tags = appendConnectionState(*tags, cs)
 	connsClosed.WithLabelValues(*tags...).Inc()
@@ -159,8 +148,9 @@ func (m *metricsTracer) ClosedConnection(dir network.Direction, duration time.Du
 }
 
 func (m *metricsTracer) CompletedHandshake(t time.Duration, cs network.ConnectionState) {
-	tags := getStringSlice()
-	defer putStringSlice(tags)
+	tags := metricshelper.GetStringSlice()
+	defer metricshelper.PutStringSlice(tags)
+
 	*tags = appendConnectionState(*tags, cs)
 	connHandshakeLatency.WithLabelValues(*tags...).Observe(t.Seconds())
 }


Test patch:
diff --git a/p2p/host/eventbus/basic_metrics_test.go b/p2p/host/eventbus/basic_metrics_test.go
index 8322f73e..d27fcbe1 100644
--- a/p2p/host/eventbus/basic_metrics_test.go
+++ b/p2p/host/eventbus/basic_metrics_test.go
@@ -9,8 +9,11 @@ import (
 
 func BenchmarkEventEmitted(b *testing.B) {
 	b.ReportAllocs()
-	types := []reflect.Type{reflect.TypeOf(new(event.EvtLocalAddressesUpdated)), reflect.TypeOf(new(event.EvtNATDeviceTypeChanged)),
-		reflect.TypeOf(new(event.EvtLocalProtocolsUpdated))}
+	types := []reflect.Type{
+		reflect.TypeOf(new(event.EvtLocalAddressesUpdated)),
+		reflect.TypeOf(new(event.EvtNATDeviceTypeChanged)),
+		reflect.TypeOf(new(event.EvtLocalProtocolsUpdated)),
+	}
 	mt := NewMetricsTracer()
 	for i := 0; i < b.N; i++ {
 		mt.EventEmitted(types[i%len(types)])
diff --git a/p2p/metricshelper/pool_test.go b/p2p/metricshelper/pool_test.go
new file mode 100644
index 00000000..85021e55
--- /dev/null
+++ b/p2p/metricshelper/pool_test.go
@@ -0,0 +1,21 @@
+package metricshelper
+
+import (
+	"math/rand"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestStringSlicePool(t *testing.T) {
+	for i := 0; i < 1e5; i++ {
+		s := GetStringSlice()
+		require.Empty(t, *s)
+		require.Equal(t, 8, cap(*s))
+		*s = append(*s, "foo")
+		*s = append(*s, "bar")
+		if rand.Int()%3 == 0 {
+			PutStringSlice(s)
+		}
+	}
+}