pdh: avoid reflection and per-scrape allocations when collecting (#2735)

Signed-off-by: Jan-Otto Kröpke <mail@jkroepke.de>

AGENTS.md

@@ -112,6 +112,9 @@ exercised a live Windows role.
   instance occurrences within a sample; their suffixes are not stable identities.
   Do not remove every name ending in `_Total`: legitimate process and database
   names can use that suffix. Use the shared aggregate-instance policy.
+  `pdh.Collector.Collect` reuses the backing array of its destination slice
+  and `Row.Values`; do not keep rows, sub-slices or pointers into it across
+  scrapes.
 - PDH collectors add counters by English path. On a Windows installation
   without en-US resources, the 009 name table holds localized names for v2
   (manifest-based) counter sets such as `SMB Server Shares` or `HTTP Service`;

internal/pdh/collector.go

@@ -18,6 +18,7 @@
 package pdh
 
 import (
+	"context"
 	"errors"
 	"fmt"
 	"log/slog"
@@ -50,6 +51,7 @@ type Collector[T any] struct {
 	object                string
 	resultType            CounterType
 	counters              []Counter
+	counterIndex          map[string]int // index in counters by counter name
 	handle                pdhQueryHandle
 	totalCounterRequested bool
 	mu                    sync.RWMutex
@@ -73,10 +75,12 @@ type Row struct {
 	Values []float64
 }
 
-// rowAccessor creates rows of type T and sets their names and counter values.
+// rowAccessor initializes rows of type T and sets their names and counter values.
 // field is the index the counterField of the counter was created with.
 type rowAccessor[T any] struct {
-	newRow   func(instance string) T
+	// resetRow initializes row for instance. row can hold the values of a
+	// previous sample; resetRow can reuse its memory.
+	resetRow func(row *T, instance string)
 	setName  func(row *T, instance string)
 	setValue func(row *T, field int, value float64)
 }
@@ -84,7 +88,9 @@ type rowAccessor[T any] struct {
 // counterField maps a counter to the field index passed to rowAccessor.setValue.
 type counterField struct {
 	// tag is the counter name, optionally suffixed with ",secondvalue".
-	tag        string
+	tag string
+	// index is the byte offset of the field in a struct row, or the index
+	// in Row.Values.
 	index      int
 	minOSBuild string
 }
@@ -132,15 +138,30 @@ func NewQueryCollector[T any](logger *slog.Logger, query *Query, resultType Coun
 }
 
 func newStructCollector[T any](logger *slog.Logger, query *Query, resultType CounterType, object string, instances []string) (*Collector[T], error) {
-	valueType := reflect.TypeFor[T]()
-	if valueType.Kind() != reflect.Struct {
-		return nil, fmt.Errorf("expected a struct, got %s", valueType)
+	fields, rows, errs, err := structFields[T]()
+	if err != nil {
+		return nil, err
 	}
 
-	nameIndex := -1
+	return newCollector(logger, query, resultType, object, instances, fields, rows, false, errs)
+}
+
+// structFields returns the counter fields declared by the perfdata tags of
+// the struct T and an accessor that writes them. The index of a counterField
+// is the byte offset of its float64 field in T, so the accessor writes values
+// without reflection.
+func structFields[T any]() ([]counterField, rowAccessor[T], []error, error) {
+	valueType := reflect.TypeFor[T]()
+	if valueType.Kind() != reflect.Struct {
+		return nil, rowAccessor[T]{}, nil, fmt.Errorf("expected a struct, got %s", valueType)
+	}
+
+	nameOffset := -1
 
 	if f, ok := valueType.FieldByName("Name"); ok && f.Type.Kind() == reflect.String {
-		nameIndex = f.Index[0]
+		if offset, ok := fieldOffset(valueType, f.Index); ok {
+			nameOffset = offset
+		}
 	}
 
 	var errs []error
@@ -159,34 +180,100 @@ func newStructCollector[T any](logger *slog.Logger, query *Query, resultType Cou
 			continue
 		}
 
+		offset, ok := fieldOffset(valueType, f.Index)
+		if !ok {
+			errs = append(errs, fmt.Errorf("field %s must not be promoted through an embedded pointer", f.Name))
+
+			continue
+		}
+
 		fields = append(fields, counterField{
 			tag:        counterName,
-			index:      f.Index[0],
+			index:      offset,
 			minOSBuild: f.Tag.Get("perfdata_min_build"),
 		})
 	}
 
-	rows := rowAccessor[T]{
-		newRow: func(instance string) T {
-			var row T
-
-			if nameIndex != -1 {
-				reflect.ValueOf(&row).Elem().Field(nameIndex).SetString(instance)
-			}
-
-			return row
-		},
-		setName: func(row *T, instance string) {
-			if nameIndex != -1 {
-				reflect.ValueOf(row).Elem().Field(nameIndex).SetString(instance)
-			}
-		},
-		setValue: func(row *T, field int, value float64) {
-			reflect.ValueOf(row).Elem().Field(field).SetFloat(value)
-		},
+	// structRowAccessor writes through these offsets with unsafe, so an index
+	// that is not the offset of a float64 field, e.g. a field number, would
+	// corrupt memory. Refuse it instead.
+	if err := validateFloatOffsets(valueType, fields); err != nil {
+		return nil, rowAccessor[T]{}, nil, err
 	}
 
-	return newCollector(logger, query, resultType, object, instances, fields, rows, false, errs)
+	return fields, structRowAccessor[T](nameOffset), errs, nil
+}
+
+// validateFloatOffsets checks that the index of every field is the byte
+// offset of a float64 field stored in the struct t.
+func validateFloatOffsets(t reflect.Type, fields []counterField) error {
+	floatOffsets := make(map[int]struct{}, len(fields))
+
+	for _, f := range reflect.VisibleFields(t) {
+		if f.Type.Kind() != reflect.Float64 {
+			continue
+		}
+
+		if offset, ok := fieldOffset(t, f.Index); ok {
+			floatOffsets[offset] = struct{}{}
+		}
+	}
+
+	size := int(t.Size())
+	floatSize := int(unsafe.Sizeof(float64(0)))
+
+	for _, f := range fields {
+		if _, ok := floatOffsets[f.index]; !ok || f.index%floatSize != 0 || f.index+floatSize > size {
+			return fmt.Errorf("perfdata field %q: %d is not the byte offset of a float64 field of %s", f.tag, f.index, t)
+		}
+	}
+
+	return nil
+}
+
+// fieldOffset returns the byte offset of the field with the index sequence
+// index in the struct t. ok is false if the field is reached through an
+// embedded pointer and is therefore not stored in t itself.
+func fieldOffset(t reflect.Type, index []int) (int, bool) {
+	var offset uintptr
+
+	for _, i := range index {
+		if t.Kind() != reflect.Struct {
+			return 0, false
+		}
+
+		f := t.Field(i)
+		offset += f.Offset
+		t = f.Type
+	}
+
+	return int(offset), true
+}
+
+// structRowAccessor returns the accessor of the struct T. nameOffset is the
+// byte offset of its string field Name, or -1. The offsets passed to setValue
+// come from [structFields] and address float64 fields of T, so values are
+// written without reflection.
+func structRowAccessor[T any](nameOffset int) rowAccessor[T] {
+	setName := func(row *T, instance string) {
+		if nameOffset >= 0 {
+			*(*string)(unsafe.Add(unsafe.Pointer(row), nameOffset)) = instance
+		}
+	}
+
+	return rowAccessor[T]{
+		resetRow: func(row *T, instance string) {
+			var zero T
+
+			*row = zero
+
+			setName(row, instance)
+		},
+		setName: setName,
+		setValue: func(row *T, offset int, value float64) {
+			*(*float64)(unsafe.Add(unsafe.Pointer(row), offset)) = value
+		},
+	}
 }
 
 // NewDynamicCollector creates a collector for counters that are only known at runtime.
@@ -199,9 +286,24 @@ func NewDynamicCollector(logger *slog.Logger, resultType CounterType, object str
 		fields[i] = counterField{tag: counter, index: i}
 	}
 
-	rows := rowAccessor[Row]{
-		newRow: func(instance string) Row {
-			return Row{Name: instance, Values: make([]float64, len(counters))}
+	return newCollector(logger, nil, resultType, object, instances, fields, dynamicRowAccessor(len(counters)), true, nil)
+}
+
+// dynamicRowAccessor returns the accessor of rows with n values. A reused row
+// keeps its Values array if it is large enough.
+func dynamicRowAccessor(n int) rowAccessor[Row] {
+	return rowAccessor[Row]{
+		resetRow: func(row *Row, instance string) {
+			row.Name = instance
+
+			if cap(row.Values) < n {
+				row.Values = make([]float64, n)
+
+				return
+			}
+
+			row.Values = row.Values[:n]
+			clear(row.Values)
 		},
 		setName: func(row *Row, instance string) {
 			row.Name = instance
@@ -210,8 +312,6 @@ func NewDynamicCollector(logger *slog.Logger, resultType CounterType, object str
 			row.Values[field] = value
 		},
 	}
-
-	return newCollector(logger, nil, resultType, object, instances, fields, rows, true, nil)
 }
 
 func newCollector[T any](logger *slog.Logger, query *Query, resultType CounterType, object string, instances []string,
@@ -246,6 +346,7 @@ func newCollector[T any](logger *slog.Logger, query *Query, resultType CounterTy
 		object:                object,
 		resultType:            resultType,
 		counters:              make([]Counter, 0, len(fields)),
+		counterIndex:          make(map[string]int, len(fields)),
 		handle:                handle,
 		totalCounterRequested: slices.ContainsFunc(instances, func(instance string) bool { return IsTotalInstance(object, instance) }),
 		mu:                    sync.RWMutex{},
@@ -254,15 +355,13 @@ func newCollector[T any](logger *slog.Logger, query *Query, resultType CounterTy
 		partialRows:           partialRows,
 	}
 
-	counterIndex := make(map[string]int, len(fields))
-
 	for _, f := range fields {
 		counterName, secondValue := strings.CutSuffix(f.tag, ",secondvalue")
 
-		i, ok := counterIndex[counterName]
+		i, ok := collector.counterIndex[counterName]
 		if !ok {
 			i = len(collector.counters)
-			counterIndex[counterName] = i
+			collector.counterIndex[counterName] = i
 			collector.counters = append(collector.counters, Counter{
 				Name:                  counterName,
 				Instances:             make(map[string]pdhCounterHandle, len(instances)),
@@ -438,13 +537,14 @@ func (c *Collector[T]) MetricType(counterName string) (prometheus.ValueType, boo
 	c.mu.RLock()
 	defer c.mu.RUnlock()
 
-	for _, counter := range c.counters {
-		if counter.Name == counterName {
-			return counter.MetricType, counter.MetricType != 0
-		}
+	i, ok := c.counterIndex[counterName]
+	if !ok {
+		return 0, false
 	}
 
-	return 0, false
+	metricType := c.counters[i].MetricType
+
+	return metricType, metricType != 0
 }
 
 // Frequency returns the time base in ticks per second of the raw timer counter
@@ -461,16 +561,22 @@ func (c *Collector[T]) Frequency(counterName string) (int64, bool) {
 	c.mu.RLock()
 	defer c.mu.RUnlock()
 
-	for _, counter := range c.counters {
-		if counter.Name == counterName {
-			return counter.Frequency, counter.Frequency > 0
-		}
+	i, ok := c.counterIndex[counterName]
+	if !ok {
+		return 0, false
 	}
 
-	return 0, false
+	frequency := c.counters[i].Frequency
+
+	return frequency, frequency > 0
 }
 
 // Collect replaces the content of dst with one row per collected instance.
+//
+// Collect reuses the backing array of dst and, for [Row], the Values slices of
+// its rows, so a sample does not allocate them again. Rows, sub-slices of dst
+// and Row.Values are only valid until the next Collect into the same slice;
+// copy what must be kept longer.
 func (c *Collector[T]) Collect(dst *[]T) error {
 	if c == nil {
 		return ErrPerformanceCounterNotInitialized
@@ -530,7 +636,16 @@ func (c *Collector[T]) collect(dst *[]T, state *collectState) (err error) {
 		}
 	}
 
-	*dst = (*dst)[:0:0]
+	// Keep the backing array; rowSet initializes every row it reuses.
+	previousLen := len(*dst)
+	*dst = (*dst)[:0]
+
+	defer func() {
+		// Release the instance names of rows that this sample no longer uses.
+		if n := len(*dst); n < previousLen {
+			clear((*dst)[n:previousLen])
+		}
+	}()
 
 	rows := newRowSet(c, dst, state)
 
@@ -607,9 +722,9 @@ const pdhBufferSlack = 8
 // getCounterArray reads the counter array of instance into buf and returns the number of items.
 // ok is false if the counter has no data in this sample.
 func (c *Collector[T]) getCounterArray(instance pdhCounterHandle, buf *[]byte) (uint32, bool, error) {
-	getArray, name := GetRawCounterArray, "GetRawCounterArray"
+	name := "GetRawCounterArray"
 	if c.resultType == CounterTypeFormatted {
-		getArray, name = GetFormattedCounterArrayDouble, "GetFormattedCounterArrayDouble"
+		name = "GetFormattedCounterArrayDouble"
 	}
 
 	var itemCount uint32
@@ -618,18 +733,28 @@ func (c *Collector[T]) getCounterArray(instance pdhCounterHandle, buf *[]byte) (
 	bytesNeeded := uint32(len(*buf))
 
 	for {
-		ret := getArray(instance, &bytesNeeded, &itemCount, unsafe.SliceData(*buf))
+		// Direct calls instead of a function value keep bytesNeeded and
+		// itemCount on the stack.
+		var ret uint32
+		if c.resultType == CounterTypeFormatted {
+			ret = GetFormattedCounterArrayDouble(instance, &bytesNeeded, &itemCount, unsafe.SliceData(*buf))
+		} else {
+			ret = GetRawCounterArray(instance, &bytesNeeded, &itemCount, unsafe.SliceData(*buf))
+		}
 
 		if ret == ErrorSuccess {
 			return itemCount, true, nil
 		}
 
-		if err := NewPdhError(ret); ret != MoreData {
-			if isKnownCounterDataError(err) {
+		// Check the status before creating an error: NewPdhError formats the
+		// message text, which a growing buffer or a counter without data in
+		// this sample does not need.
+		if ret != MoreData {
+			if isKnownCounterDataStatus(ret) {
 				return 0, false, nil
 			}
 
-			return 0, false, fmt.Errorf("%s: %w", name, err)
+			return 0, false, fmt.Errorf("%s: %w", name, NewPdhError(ret))
 		}
 
 		if bytesNeeded <= uint32(len(*buf)) {
@@ -734,11 +859,15 @@ func (r *rowSet[T]) row(counter *Counter, szName *uint16, status uint32) (int, b
 	r.occurrences[instanceName] = max(r.occurrences[instanceName], occurrence+1)
 
 	if status != CstatusValidData && status != CstatusNewData {
-		r.c.logger.Debug("skipping counter item with invalid data status",
-			slog.String("counter", counter.Name),
-			slog.String("instance", instanceName),
-			slog.Uint64("status", uint64(status)),
-		)
+		// Check the level first, so the attributes are not built for every
+		// invalid item while debug logging is off.
+		if r.c.logger.Enabled(context.Background(), slog.LevelDebug) {
+			r.c.logger.LogAttrs(context.Background(), slog.LevelDebug, "skipping counter item with invalid data status",
+				slog.String("counter", counter.Name),
+				slog.String("instance", instanceName),
+				slog.Uint64("status", uint64(status)),
+			)
+		}
 
 		return 0, false
 	}
@@ -752,10 +881,21 @@ func (r *rowSet[T]) row(counter *Counter, szName *uint16, status uint32) (int, b
 		return index, true
 	}
 
-	index := len(*r.dst)
+	rows := *r.dst
+	index := len(rows)
 	r.index[key] = index
 
-	*r.dst = append(*r.dst, r.c.rows.newRow(instanceName))
+	// Reuse a row of a previous sample within the capacity of dst.
+	if index < cap(rows) {
+		rows = rows[:index+1]
+	} else {
+		var zero T
+
+		rows = append(rows, zero)
+	}
+
+	r.c.rows.resetRow(&rows[index], instanceName)
+	*r.dst = rows
 
 	n := len(r.c.counters)
 	r.valid = slices.Grow(r.valid, n)[:len(r.valid)+n]
@@ -814,7 +954,7 @@ func (r *rowSet[T]) nameDuplicates() {
 
 			for {
 				suffix++
-				instanceName = name + "#" + strconv.Itoa(suffix)
+				instanceName = r.nameCache.suffixed(name, suffix)
 
 				if _, literalName := r.occurrences[instanceName]; !literalName {
 					break
@@ -866,12 +1006,20 @@ func (r *rowSet[T]) finish() {
 			continue
 		}
 
-		rows[kept] = rows[row]
+		if row != kept {
+			rows[kept] = rows[row]
+
+			// Do not leave two rows that share a Row.Values array.
+			var zero T
+
+			rows[row] = zero
+		}
+
 		kept++
 	}
 
 	if dropped := len(rows) - kept; dropped > 0 {
-		r.c.logger.Debug("omitting instances without a valid value for every counter",
+		r.c.logger.LogAttrs(context.Background(), slog.LevelDebug, "omitting instances without a valid value for every counter",
 			slog.String("object", r.c.object),
 			slog.Int("instances", dropped),
 		)
@@ -944,8 +1092,11 @@ func formatCounterPath(object, instance, counterName string) string {
 // Names that a sample did not decode are removed at its end, because instance names that
 // contain process IDs, like those of GPU Engine, change all the time.
 type instanceNameCache struct {
-	names  map[string]*cachedInstanceName
-	sample uint64
+	names map[string]*cachedInstanceName
+	// suffixedNames maps a name and an occurrence suffix to the name with
+	// the suffix, e.g. svchost and 3 to svchost#3.
+	suffixedNames map[instanceKey]*cachedInstanceName
+	sample        uint64
 }
 
 type cachedInstanceName struct {
@@ -989,22 +1140,50 @@ func (cache *instanceNameCache) decode(p *uint16) string {
 	return name
 }
 
-// removeUnseen removes the names that the current sample did not decode.
+// suffixed returns name with the occurrence suffix #suffix. Duplicate names,
+// like those of the many svchost processes, need the same suffixed names in
+// every sample.
+func (cache *instanceNameCache) suffixed(name string, suffix int) string {
+	key := instanceKey{name: name, occurrence: suffix}
+
+	if cached, ok := cache.suffixedNames[key]; ok {
+		cached.sample = cache.sample
+
+		return cached.name
+	}
+
+	if cache.suffixedNames == nil {
+		cache.suffixedNames = map[instanceKey]*cachedInstanceName{}
+	}
+
+	suffixedName := name + "#" + strconv.Itoa(suffix)
+	cache.suffixedNames[key] = &cachedInstanceName{name: suffixedName, sample: cache.sample}
+
+	return suffixedName
+}
+
+// removeUnseen removes the names that the current sample did not use.
 func (cache *instanceNameCache) removeUnseen() {
 	for raw, cached := range cache.names {
 		if cached.sample != cache.sample {
 			delete(cache.names, raw)
 		}
 	}
+
+	for key, cached := range cache.suffixedNames {
+		if cached.sample != cache.sample {
+			delete(cache.suffixedNames, key)
+		}
+	}
 }
 
-func isKnownCounterDataError(err error) bool {
-	var pdhErr *Error
-
-	return errors.As(err, &pdhErr) && (pdhErr.ErrorCode == InvalidData ||
-		pdhErr.ErrorCode == CalcNegativeDenominator ||
-		pdhErr.ErrorCode == CalcNegativeValue ||
-		pdhErr.ErrorCode == CstatusInvalidData ||
-		pdhErr.ErrorCode == CstatusNoInstance ||
-		pdhErr.ErrorCode == NoData)
+// isKnownCounterDataStatus reports whether status means that a counter has no
+// valid data in a sample, which is not a collection error.
+func isKnownCounterDataStatus(status uint32) bool {
+	switch status {
+	case InvalidData, CalcNegativeDenominator, CalcNegativeValue, CstatusInvalidData, CstatusNoInstance, NoData:
+		return true
+	default:
+		return false
+	}
 }

internal/pdh/collector_internal_test.go

@@ -43,25 +43,18 @@ type rowSetValues struct {
 // newRowSetTestCollector returns a collector with the counters A and B, which
 // have instances, and Optional, which was skipped at Build and has none.
 func newRowSetTestCollector(partialRows bool) *Collector[rowSetValues] {
+	var row rowSetValues
+
 	return &Collector[rowSetValues]{
 		partialRows: partialRows,
 		logger:      slog.New(slog.DiscardHandler),
-		rows: rowAccessor[rowSetValues]{
-			newRow: func(instance string) rowSetValues {
-				return rowSetValues{Name: instance}
-			},
-			setName: func(row *rowSetValues, instance string) {
-				row.Name = instance
-			},
-			setValue: func(row *rowSetValues, field int, value float64) {
-				reflect.ValueOf(row).Elem().Field(field).SetFloat(value)
-			},
-		},
+		rows:        structRowAccessor[rowSetValues](int(unsafe.Offsetof(row.Name))),
 		counters: []Counter{
-			{Name: "A", Instances: map[string]pdhCounterHandle{"*": 1}, FieldIndexValue: 1, FieldIndexSecondValue: -1},
-			{Name: "B", Instances: map[string]pdhCounterHandle{"*": 2}, FieldIndexValue: 2, FieldIndexSecondValue: 3},
-			{Name: "Optional", Instances: map[string]pdhCounterHandle{}, FieldIndexValue: 4, FieldIndexSecondValue: -1},
+			{Name: "A", Instances: map[string]pdhCounterHandle{"*": 1}, FieldIndexValue: int(unsafe.Offsetof(row.A)), FieldIndexSecondValue: -1},
+			{Name: "B", Instances: map[string]pdhCounterHandle{"*": 2}, FieldIndexValue: int(unsafe.Offsetof(row.B)), FieldIndexSecondValue: int(unsafe.Offsetof(row.BSecond))},
+			{Name: "Optional", Instances: map[string]pdhCounterHandle{}, FieldIndexValue: int(unsafe.Offsetof(row.Optional)), FieldIndexSecondValue: -1},
 		},
+		counterIndex: map[string]int{"A": 0, "B": 1, "Optional": 2},
 	}
 }
 
@@ -348,7 +341,7 @@ func TestSetRawValueElapsedTime(t *testing.T) {
 
 	var values rowSetValues
 
-	counter := &Counter{Type: PERF_ELAPSED_TIME, Frequency: 10_000_000, FieldIndexValue: 1, FieldIndexSecondValue: -1}
+	counter := &Counter{Type: PERF_ELAPSED_TIME, Frequency: 10_000_000, FieldIndexValue: int(unsafe.Offsetof(values.A)), FieldIndexSecondValue: -1}
 
 	// 2.5 s at 10 MHz. Integer division truncated this to 2.
 	require.True(t, c.setRawValue(&values, counter, RawCounter{FirstValue: 100_000_000, SecondValue: 125_000_000}))
@@ -359,7 +352,7 @@ func TestSetRawValueElapsedTime(t *testing.T) {
 	require.False(t, c.setRawValue(&values, counter, RawCounter{FirstValue: 1, SecondValue: 2}))
 
 	// A counter with only a second value field must not set field -1.
-	counter = &Counter{Type: PERF_ELAPSED_TIME, Frequency: 1, FieldIndexValue: -1, FieldIndexSecondValue: 3}
+	counter = &Counter{Type: PERF_ELAPSED_TIME, Frequency: 1, FieldIndexValue: -1, FieldIndexSecondValue: int(unsafe.Offsetof(values.BSecond))}
 	require.True(t, c.setRawValue(&values, counter, RawCounter{FirstValue: 1, SecondValue: 2}))
 }
 
@@ -474,17 +467,14 @@ func TestCollectRecoversPanic(t *testing.T) {
 
 	require.NoError(t, c.Collect(&dst))
 
-	// Point the counter at a field that does not exist, so that reflect panics.
-	c.mu.Lock()
-	c.counters[0].FieldIndexValue = 42
-	c.mu.Unlock()
+	setValue := setPanicOnSetValue(c)
 
 	err = c.Collect(&dst)
 	require.ErrorContains(t, err, "panic while collecting performance counters of Process")
 
 	// Collection still works after the panic.
 	c.mu.Lock()
-	c.counters[0].FieldIndexValue = 1
+	c.rows.setValue = setValue
 	c.mu.Unlock()
 
 	require.NoError(t, c.Collect(&dst))
@@ -515,19 +505,23 @@ func TestCollectKeepsInstanceNames(t *testing.T) {
 	require.Same(t, unsafe.StringData(systemName(first)), unsafe.StringData(systemName(second)), "name decoded again")
 
 	// Assertions run without holding the lock, because a failed assertion would leave it locked for Close.
-	cachedNames := func(fieldIndex int) int {
+	setValue := setPanicOnSetValue(c)
+
+	cachedNames := func() int {
 		c.mu.Lock()
 		defer c.mu.Unlock()
 
-		c.counters[0].FieldIndexValue = fieldIndex
-
 		return len(c.state.names.names)
 	}
 
-	// Point the counter at a field that does not exist, so that the first item panics.
-	require.Greater(t, cachedNames(42), 1)
+	// Let the first item panic.
+	require.Greater(t, cachedNames(), 1)
 	require.Error(t, c.Collect(&first))
-	require.Equal(t, 1, cachedNames(1), "names not reached by the failed sample kept")
+	require.Equal(t, 1, cachedNames(), "names not reached by the failed sample kept")
+
+	c.mu.Lock()
+	c.rows.setValue = setValue
+	c.mu.Unlock()
 
 	require.NoError(t, c.Collect(&first))
 	require.Equal(t, systemName(second), systemName(first))
@@ -597,3 +591,15 @@ func TestRowSetReusesState(t *testing.T) {
 		}, dst)
 	}
 }
+
+// setPanicOnSetValue lets every value that c sets panic and returns the
+// previous setter.
+func setPanicOnSetValue[T any](c *Collector[T]) func(row *T, field int, value float64) {
+	c.mu.Lock()
+	defer c.mu.Unlock()
+
+	setValue := c.rows.setValue
+	c.rows.setValue = func(*T, int, float64) { panic("test panic") }
+
+	return setValue
+}

internal/pdh/collector_rows_internal_test.go

@@ -0,0 +1,346 @@
+// SPDX-License-Identifier: Apache-2.0
+//
+// Copyright The Prometheus Authors
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+//go:build windows
+
+package pdh
+
+import (
+	"log/slog"
+	"reflect"
+	"runtime"
+	"testing"
+	"unsafe"
+
+	"github.com/stretchr/testify/require"
+	"golang.org/x/sys/windows"
+)
+
+type embeddedCounters struct {
+	Name  string
+	Inner float64 `perfdata:"Inner"`
+}
+
+type pointerCounters struct {
+	Pointer float64 `perfdata:"Pointer"`
+}
+
+type structFieldsValues struct {
+	_                int32 // Gives the embedded struct a non-zero offset.
+	embeddedCounters       //nolint:embeddedstructfieldcheck // Tests the offset of a promoted field.
+	*pointerCounters
+
+	Outer   float64 `perfdata:"Outer"`
+	Base    float64 `perfdata:"Outer,secondvalue"`
+	Invalid int64   `perfdata:"Invalid"`
+}
+
+// TestStructFields checks that the counter fields of a struct, including
+// fields promoted from an embedded struct, are written at their offsets.
+func TestStructFields(t *testing.T) {
+	t.Parallel()
+
+	fields, rows, errs, err := structFields[structFieldsValues]()
+	require.NoError(t, err)
+
+	require.Len(t, errs, 2)
+	require.ErrorContains(t, errs[0], "field Pointer must not be promoted through an embedded pointer")
+	require.ErrorContains(t, errs[1], "field Invalid must be a float64")
+
+	offsets := make(map[string]int, len(fields))
+	for _, f := range fields {
+		offsets[f.tag] = f.index
+	}
+
+	require.Len(t, offsets, 3)
+
+	var row structFieldsValues
+
+	row.Invalid = 42
+	rows.resetRow(&row, "instance")
+	require.Equal(t, structFieldsValues{Name: "instance"}, row)
+
+	rows.setValue(&row, offsets["Inner"], 1)
+	rows.setValue(&row, offsets["Outer"], 2)
+	rows.setValue(&row, offsets["Outer,secondvalue"], 3)
+	rows.setName(&row, "renamed")
+
+	require.Equal(t, structFieldsValues{
+		Name:  "renamed",
+		Inner: 1,
+		Outer: 2,
+		Base:  3,
+	}, row)
+
+	_, _, _, err = structFields[float64]()
+	require.ErrorContains(t, err, "expected a struct, got float64")
+}
+
+// TestStructFieldsIndexIsByteOffset checks that the index of a counter field
+// is the byte offset that structRowAccessor writes to, not a field number.
+func TestStructFieldsIndexIsByteOffset(t *testing.T) {
+	t.Parallel()
+
+	type values struct {
+		Name string
+		A    float64 `perfdata:"A"`
+		B    float64 `perfdata:"B"`
+		C    float64 `perfdata:"B,secondvalue"`
+	}
+
+	fields, _, errs, err := structFields[values]()
+	require.NoError(t, err)
+	require.Empty(t, errs)
+
+	var row values
+
+	require.Equal(t, []int{
+		int(unsafe.Offsetof(row.A)),
+		int(unsafe.Offsetof(row.B)),
+		int(unsafe.Offsetof(row.C)),
+	}, []int{fields[0].index, fields[1].index, fields[2].index})
+}
+
+// TestValidateFloatOffsets checks that field numbers or other indexes that do
+// not address a float64 field are rejected before they are written through.
+func TestValidateFloatOffsets(t *testing.T) {
+	t.Parallel()
+
+	type values struct {
+		Name string
+		A    float64 `perfdata:"A"`
+		B    float64 `perfdata:"B"`
+		C    float64 `perfdata:"C"`
+	}
+
+	valueType := reflect.TypeFor[values]()
+
+	var row values
+
+	valid := []counterField{
+		{tag: "A", index: int(unsafe.Offsetof(row.A))},
+		{tag: "C", index: int(unsafe.Offsetof(row.C))},
+	}
+	require.NoError(t, validateFloatOffsets(valueType, valid))
+
+	// Field numbers, as reflect.StructField.Index[0] would give them.
+	fieldNumbers := make([]counterField, 0, 3)
+
+	for _, f := range reflect.VisibleFields(valueType) {
+		if tag, ok := f.Tag.Lookup("perfdata"); ok {
+			fieldNumbers = append(fieldNumbers, counterField{tag: tag, index: f.Index[0]})
+		}
+	}
+
+	require.ErrorContains(t, validateFloatOffsets(valueType, fieldNumbers), "is not the byte offset of a float64 field")
+
+	for _, index := range []int{
+		int(unsafe.Offsetof(row.Name)),  // a string, not a float64
+		int(unsafe.Offsetof(row.A)) + 4, // misaligned
+		int(unsafe.Sizeof(row)),         // past the end
+		int(unsafe.Sizeof(row)) - 4,     // overlaps the end
+		-8,                              // before the start
+	} {
+		require.Error(t, validateFloatOffsets(valueType, []counterField{{tag: "X", index: index}}), "index %d", index)
+	}
+}
+
+// TestStructRowAccessorWithoutName checks that a struct without a Name field
+// only gets its values.
+func TestStructRowAccessorWithoutName(t *testing.T) {
+	t.Parallel()
+
+	type values struct {
+		Value float64 `perfdata:"Value"`
+	}
+
+	fields, rows, errs, err := structFields[values]()
+	require.NoError(t, err)
+	require.Empty(t, errs)
+	require.Len(t, fields, 1)
+
+	row := values{Value: 1}
+	rows.resetRow(&row, "instance")
+	rows.setName(&row, "instance")
+	require.Equal(t, values{}, row)
+
+	rows.setValue(&row, fields[0].index, 2)
+	require.Equal(t, values{Value: 2}, row)
+}
+
+// TestDynamicRowAccessor checks that a reused Row keeps a large enough Values
+// array and that no value of an earlier sample remains.
+func TestDynamicRowAccessor(t *testing.T) {
+	t.Parallel()
+
+	rows := dynamicRowAccessor(2)
+
+	values := []float64{1, 2, 3}
+	row := Row{Name: "old", Values: values}
+
+	rows.resetRow(&row, "new")
+	require.Equal(t, Row{Name: "new", Values: []float64{0, 0}}, row)
+	require.Same(t, unsafe.SliceData(values), unsafe.SliceData(row.Values), "Values allocated again")
+
+	rows.setValue(&row, 1, 4)
+	require.Equal(t, []float64{0, 4}, row.Values)
+
+	row = Row{Name: "old", Values: []float64{1}}
+	rows.resetRow(&row, "new")
+	require.Equal(t, Row{Name: "new", Values: []float64{0, 0}}, row)
+}
+
+// TestRowSetReusesRows checks that a sample reuses the backing array of dst
+// and that no value of an earlier sample remains in a reused row.
+func TestRowSetReusesRows(t *testing.T) {
+	t.Parallel()
+
+	c := newRowSetTestCollector(false)
+	c.counters[1].FieldIndexSecondValue = -1
+
+	dst := make([]rowSetValues, 3, 4)
+	for i := range dst {
+		dst[i] = rowSetValues{Name: "stale", A: 9, B: 9, BSecond: 9, Optional: 9}
+	}
+
+	backing := unsafe.SliceData(dst)
+	dst = dst[:0]
+
+	rows := newRowSet(c, &dst, &collectState{})
+	addDuplicateTestItems(&rows, CounterTypeRaw, 0, []string{"b", "c"}, []float64{4, 5})
+	addDuplicateTestItems(&rows, CounterTypeRaw, 1, []string{"b", "c"}, []float64{6, 7})
+	rows.finish()
+
+	require.Same(t, backing, unsafe.SliceData(dst), "rows allocated again")
+	require.Equal(t, []rowSetValues{
+		{Name: "b", A: 4, B: 6},
+		{Name: "c", A: 5, B: 7},
+	}, dst)
+}
+
+// TestRowSetSampleDoesNotAllocate checks that a sample of known instances,
+// including duplicate names, into a reused slice does not allocate.
+func TestRowSetSampleDoesNotAllocate(t *testing.T) {
+	c := newRowSetTestCollector(true)
+
+	const instances = 100
+
+	items := make([]RawCounterItem, instances)
+	for i := range items {
+		items[i] = RawCounterItem{
+			// Every name occurs four times, like svchost.
+			SzName:   windows.StringToUTF16Ptr("instance_" + string(rune('A'+i%25))),
+			RawValue: RawCounter{CStatus: CstatusValidData, FirstValue: int64(i)},
+		}
+	}
+
+	buf := unsafe.Slice((*byte)(unsafe.Pointer(unsafe.SliceData(items))), len(items)*int(unsafe.Sizeof(RawCounterItem{})))
+	state := &collectState{}
+
+	var dst []rowSetValues
+
+	sample := func() {
+		dst = dst[:0]
+		rows := newRowSet(c, &dst, state)
+
+		rows.addRawItems(0, buf, instances)
+		rows.addRawItems(1, buf, instances)
+		rows.finish()
+		state.names.removeUnseen()
+
+		state.valid = rows.valid
+	}
+
+	sample()
+
+	require.Len(t, dst, instances)
+	require.Equal(t, "instance_A#3", dst[75].Name)
+	require.Zero(t, testing.AllocsPerRun(10, sample))
+
+	runtime.KeepAlive(items)
+}
+
+// TestInstanceNameCacheSuffixed checks that suffixed names are built once
+// while their instances exist.
+func TestInstanceNameCacheSuffixed(t *testing.T) {
+	t.Parallel()
+
+	var cache instanceNameCache
+
+	cache.startSample()
+
+	first := cache.suffixed("svchost", 2)
+	require.Equal(t, "svchost#2", first)
+	require.Equal(t, "svchost#3", cache.suffixed("svchost", 3))
+	cache.removeUnseen()
+
+	cache.startSample()
+
+	second := cache.suffixed("svchost", 2)
+	require.Same(t, unsafe.StringData(first), unsafe.StringData(second), "name built again")
+	cache.removeUnseen()
+
+	require.Len(t, cache.suffixedNames, 1, "name of a vanished instance kept")
+	require.Contains(t, cache.suffixedNames, instanceKey{name: "svchost", occurrence: 2})
+}
+
+// TestCollectReusesRows checks that Collect keeps the backing array of dst and
+// releases the rows a sample does not use.
+func TestCollectReusesRows(t *testing.T) {
+	t.Parallel()
+
+	type processor struct {
+		Name          string
+		ProcessorTime float64 `perfdata:"% Processor Time"`
+	}
+
+	c, err := NewCollector[processor](slog.New(slog.DiscardHandler), CounterTypeRaw, "Processor", InstancesAll)
+	require.NoError(t, err)
+
+	t.Cleanup(c.Close)
+
+	dst := make([]processor, 1024)
+	for i := range dst {
+		dst[i] = processor{Name: "stale", ProcessorTime: -1}
+	}
+
+	backing := unsafe.SliceData(dst)
+
+	require.NoError(t, c.Collect(&dst))
+	require.NotEmpty(t, dst)
+	require.Less(t, len(dst), 1024)
+	require.Same(t, backing, unsafe.SliceData(dst), "rows allocated again")
+
+	for _, row := range dst {
+		require.NotEqual(t, "stale", row.Name)
+		require.GreaterOrEqual(t, row.ProcessorTime, 0.0)
+	}
+
+	for _, row := range dst[len(dst):cap(dst)] {
+		require.Equal(t, processor{}, row, "row of an earlier sample kept")
+	}
+}
+
+func TestIsKnownCounterDataStatus(t *testing.T) {
+	t.Parallel()
+
+	for _, status := range []uint32{InvalidData, CalcNegativeDenominator, CalcNegativeValue, CstatusInvalidData, CstatusNoInstance, NoData} {
+		require.True(t, isKnownCounterDataStatus(status), "0x%08X", status)
+	}
+
+	for _, status := range []uint32{ErrorSuccess, MoreData, CstatusNoObject, CstatusNoCounter, InvalidHandle} {
+		require.False(t, isKnownCounterDataStatus(status), "0x%08X", status)
+	}
+}

internal/pdh/collector_rowset_bench_test.go

@@ -19,10 +19,13 @@ package pdh
 
 import (
 	"fmt"
+	"log/slog"
 	"runtime"
+	"strings"
 	"testing"
 	"unsafe"
 
+	"github.com/prometheus/client_golang/prometheus"
 	"golang.org/x/sys/windows"
 )
 
@@ -49,7 +52,7 @@ func BenchmarkRowSetInstanceNames(b *testing.B) {
 	b.ReportAllocs()
 
 	for b.Loop() {
-		dst = dst[:0:0]
+		dst = dst[:0]
 		rows := newRowSet(c, &dst, state)
 
 		rows.addRawItems(0, buf, instances)
@@ -62,3 +65,134 @@ func BenchmarkRowSetInstanceNames(b *testing.B) {
 
 	runtime.KeepAlive(items)
 }
+
+// processShape has the shape of the Process object: one row per process with
+// 20 counters, two of which also use their second value.
+type processShape struct {
+	Name string
+
+	C00 float64 `perfdata:"C00"`
+	C01 float64 `perfdata:"C01"`
+	C02 float64 `perfdata:"C02"`
+	C03 float64 `perfdata:"C03"`
+	C04 float64 `perfdata:"C04"`
+	C05 float64 `perfdata:"C05"`
+	C06 float64 `perfdata:"C06"`
+	C07 float64 `perfdata:"C07"`
+	C08 float64 `perfdata:"C08"`
+	C09 float64 `perfdata:"C09"`
+	C10 float64 `perfdata:"C10"`
+	C11 float64 `perfdata:"C11"`
+	C12 float64 `perfdata:"C12"`
+	C13 float64 `perfdata:"C13"`
+	C14 float64 `perfdata:"C14"`
+	C15 float64 `perfdata:"C15"`
+	C16 float64 `perfdata:"C16"`
+	C17 float64 `perfdata:"C17"`
+	C18 float64 `perfdata:"C18"`
+	C19 float64 `perfdata:"C19"`
+	B00 float64 `perfdata:"C00,secondvalue"`
+	B01 float64 `perfdata:"C01,secondvalue"`
+}
+
+// newProcessShapeCollector returns a collector of processShape rows without
+// a PDH query.
+func newProcessShapeCollector(b *testing.B) *Collector[processShape] {
+	b.Helper()
+
+	fields, rows, errs, err := structFields[processShape]()
+	if err != nil || len(errs) != 0 {
+		b.Fatal(err, errs)
+	}
+
+	c := &Collector[processShape]{
+		logger:       slog.New(slog.DiscardHandler),
+		rows:         rows,
+		counterIndex: map[string]int{},
+	}
+
+	for _, f := range fields {
+		counterName, secondValue := strings.CutSuffix(f.tag, ",secondvalue")
+
+		i, ok := c.counterIndex[counterName]
+		if !ok {
+			i = len(c.counters)
+			c.counterIndex[counterName] = i
+			c.counters = append(c.counters, Counter{
+				Name:                  counterName,
+				MetricType:            prometheus.CounterValue,
+				Instances:             map[string]pdhCounterHandle{"*": pdhCounterHandle(i + 1)},
+				FieldIndexValue:       -1,
+				FieldIndexSecondValue: -1,
+			})
+		}
+
+		if secondValue {
+			c.counters[i].FieldIndexSecondValue = f.index
+		} else {
+			c.counters[i].FieldIndexValue = f.index
+		}
+	}
+
+	return c
+}
+
+// BenchmarkRowSetProcessShape measures the work of one Collect of an object
+// shaped like Process, 500 instances with 20 counters, apart from the PDH
+// calls. Like Collect, it reuses dst between samples.
+func BenchmarkRowSetProcessShape(b *testing.B) {
+	const instances = 500
+
+	c := newProcessShapeCollector(b)
+
+	items := make([]RawCounterItem, instances)
+	for i := range items {
+		items[i] = RawCounterItem{
+			SzName:   windows.StringToUTF16Ptr(fmt.Sprintf("process_%d", i)),
+			RawValue: RawCounter{CStatus: CstatusValidData, FirstValue: int64(i), SecondValue: int64(2 * i)},
+		}
+	}
+
+	buf := unsafe.Slice((*byte)(unsafe.Pointer(unsafe.SliceData(items))), len(items)*int(unsafe.Sizeof(RawCounterItem{})))
+	state := &collectState{}
+
+	var dst []processShape
+
+	b.ReportAllocs()
+
+	for b.Loop() {
+		dst = dst[:0]
+		rows := newRowSet(c, &dst, state)
+
+		for counterIndex := range c.counters {
+			rows.addRawItems(counterIndex, buf, instances)
+		}
+
+		rows.finish()
+		state.names.removeUnseen()
+
+		state.valid = rows.valid
+	}
+
+	if len(dst) != instances {
+		b.Fatalf("got %d rows, want %d", len(dst), instances)
+	}
+
+	runtime.KeepAlive(items)
+}
+
+// BenchmarkCollectorMetricType measures the lookup of the metric type of
+// every counter, as the performancecounter collector does for every scrape.
+func BenchmarkCollectorMetricType(b *testing.B) {
+	c := newProcessShapeCollector(b)
+
+	b.ReportAllocs()
+
+	for b.Loop() {
+		for i := range c.counters {
+			if _, ok := c.MetricType(c.counters[len(c.counters)-1-i].Name); !ok {
+				b.Fatal("unknown counter")
+			}
+		}
+	}
+}

internal/pdh/pdh.go

@@ -50,6 +50,7 @@ package pdh
 
 import (
 	"fmt"
+	"syscall"
 	"time"
 	"unsafe"
 
@@ -472,8 +473,10 @@ func GetFormattedCounterValueDouble(hCounter pdhCounterHandle, lpdwType *uint32,
 //			time.Sleep(2000 * time.Millisecond)
 //		}
 //	}
+//
+// Like [GetRawCounterArray], it uses syscall.SyscallN to keep the sizes on the stack.
 func GetFormattedCounterArrayDouble(hCounter pdhCounterHandle, lpdwBufferSize *uint32, lpdwBufferCount *uint32, itemBuffer *byte) uint32 {
-	ret, _, _ := pdhGetFormattedCounterArrayW.Call(
+	ret, _, _ := syscall.SyscallN(pdhGetFormattedCounterArrayW.Addr(),
 		uintptr(hCounter),
 		uintptr(FmtDouble),
 		uintptr(unsafe.Pointer(lpdwBufferSize)),
@@ -631,8 +634,11 @@ func GetRawCounterValue(hCounter pdhCounterHandle, lpdwType *uint32, pValue *Raw
 // ItemBuffer
 // Caller-allocated buffer that receives the array of RawCounterItem structures; the structures contain the raw instance counter values.
 // Set to NULL if lpdwBufferSize is zero.
+//
+// It is called for every counter in every sample. syscall.SyscallN, unlike
+// LazyProc.Call, does not move the pointed-to sizes to the heap.
 func GetRawCounterArray(hCounter pdhCounterHandle, lpdwBufferSize *uint32, lpdwBufferCount *uint32, itemBuffer *byte) uint32 {
-	ret, _, _ := pdhGetRawCounterArrayW.Call(
+	ret, _, _ := syscall.SyscallN(pdhGetRawCounterArrayW.Addr(),
 		uintptr(hCounter),
 		uintptr(unsafe.Pointer(lpdwBufferSize)),
 		uintptr(unsafe.Pointer(lpdwBufferCount)),