Skip to content

Commit

Permalink
refactor: implement reslice with stdlib slices package
Browse files Browse the repository at this point in the history
avoid using vtpool for slice elements since they are never returned to the pool
  • Loading branch information
kruskall committed Feb 15, 2024
1 parent 4924888 commit 6fd1561
Show file tree
Hide file tree
Showing 6 changed files with 51 additions and 67 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,12 @@ func HTTPHeadersToModelpb(h http.Header, out []*modelpb.HTTPHeader) []*modelpb.H
if len(h) == 0 {
return nil
}
out = Reslice(out, len(h), modelpb.HTTPHeaderFromVTPool)
out = Reslice(out, len(h))
i := 0
for k, v := range h {
if out[i] == nil {
out[i] = &modelpb.HTTPHeader{}
}
out[i].Key = k
out[i].Value = v
i++
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,14 @@ func ToKv(m map[string]any, out []*modelpb.KeyValue) []*modelpb.KeyValue {
return nil
}

out = Reslice(out, len(m), modelpb.KeyValueFromVTPool)
out = Reslice(out, len(m))

i := 0
for k, v := range m {
value, _ := structpb.NewValue(v)
if out[i] == nil {
out[i] = &modelpb.KeyValue{}
}
out[i].Key = k
out[i].Value = value
i++
Expand Down
32 changes: 4 additions & 28 deletions input/elasticapm/internal/modeldecoder/modeldecoderutil/slice.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,34 +17,10 @@

package modeldecoderutil

import "slices"

// Reslice increases the slice's capacity, if necessary, to guarantee space for n elements.
// If specified, the newFn function is used to populate the extra (optionally) allocated space.
// The method returns a slice with len(slice)==n.
func Reslice[Slice ~[]model, model any](slice Slice, n int, newFn func() model) Slice {
// check if there is enough capacity to hold n elements
if diff := n - cap(slice); diff > 0 {
// start of the extra space
idx := cap(slice)

// Grow the slice
// Note: append gives no guarantee on the capacity of the resulting slice
// and might overallocate as long as there's enough space for n elements.
slice = append([]model(slice)[:cap(slice)], make([]model, diff)...)
if newFn != nil {
// extend the slice to its capacity
// Note: reset the slice length to n before returning
slice = slice[:cap(slice)]

// populate the extra space
for ; idx < len(slice); idx++ {
slice[idx] = newFn()
}
}
}

// slice has enough capacity to hold n elements and there is no need to grow the slice.
// Do not make assumption on the length of the slice and always return a slice with len(slice)==n.
// Most of the time len(slice)==0 because the argument is a newly reset slice from a
// protobuf model.
return slice[:n]
func Reslice[Slice ~[]model, model any](slice Slice, n int) Slice {
return slices.Grow(slice, n)[:n]
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,30 +28,14 @@ func TestReslice(t *testing.T) {
var s []*modelpb.APMEvent

originalSize := 10
s = Reslice(s, originalSize, modelpb.APMEventFromVTPool)
validateBackingArray(t, s, originalSize)
s = Reslice(s, originalSize)
assert.Equal(t, originalSize, len(s))

downsize := 4
s = Reslice(s, downsize, nil)
validateBackingArray(t, s, downsize)
s = Reslice(s, downsize)
assert.Equal(t, downsize, len(s))

upsize := 21
s = Reslice(s, upsize, modelpb.APMEventFromVTPool)
validateBackingArray(t, s, upsize)
s = Reslice(s, upsize)
assert.Equal(t, upsize, len(s))
}

func validateBackingArray(t *testing.T, out []*modelpb.APMEvent, expectedLen int) {
t.Helper()

// validate length
assert.Equal(t, expectedLen, len(out))

// validate backing array is fully populated
backing := out[:cap(out)]
for i := 0; i < cap(backing); i++ {
assert.NotNil(t, backing[i])
}
}
18 changes: 12 additions & 6 deletions input/elasticapm/internal/modeldecoder/rumv3/decoder.go
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ func mapToErrorModel(from *errorEvent, event *modelpb.APMEvent) {
log.ParamMessage = from.Log.ParamMessage.Val
}
if len(from.Log.Stacktrace) > 0 {
log.Stacktrace = modeldecoderutil.Reslice(log.Stacktrace, len(from.Log.Stacktrace), modelpb.StacktraceFrameFromVTPool)
log.Stacktrace = modeldecoderutil.Reslice(log.Stacktrace, len(from.Log.Stacktrace))
mapToStracktraceModel(from.Log.Stacktrace, log.Stacktrace)
}
out.Log = log
Expand Down Expand Up @@ -291,8 +291,11 @@ func mapToExceptionModel(from *errorException, out *modelpb.Exception) {
out.Code = modeldecoderutil.ExceptionCodeString(from.Code.Val)
}
if len(from.Cause) > 0 {
out.Cause = modeldecoderutil.Reslice(out.Cause, len(from.Cause), modelpb.ExceptionFromVTPool)
out.Cause = modeldecoderutil.Reslice(out.Cause, len(from.Cause))
for i := 0; i < len(from.Cause); i++ {
if out.Cause[i] == nil {
out.Cause[i] = &modelpb.Exception{}
}
mapToExceptionModel(&from.Cause[i], out.Cause[i])
}
}
Expand All @@ -307,7 +310,7 @@ func mapToExceptionModel(from *errorException, out *modelpb.Exception) {
out.Module = from.Module.Val
}
if len(from.Stacktrace) > 0 {
out.Stacktrace = modeldecoderutil.Reslice(out.Stacktrace, len(from.Stacktrace), modelpb.StacktraceFrameFromVTPool)
out.Stacktrace = modeldecoderutil.Reslice(out.Stacktrace, len(from.Stacktrace))
mapToStracktraceModel(from.Stacktrace, out.Stacktrace)
}
if from.Type.IsSet() {
Expand Down Expand Up @@ -676,7 +679,7 @@ func mapToSpanModel(from *span, event *modelpb.APMEvent) {
out.RepresentativeCount = 1 / from.SampleRate.Val
}
if len(from.Stacktrace) > 0 {
out.Stacktrace = modeldecoderutil.Reslice(out.Stacktrace, len(from.Stacktrace), modelpb.StacktraceFrameFromVTPool)
out.Stacktrace = modeldecoderutil.Reslice(out.Stacktrace, len(from.Stacktrace))
mapToStracktraceModel(from.Stacktrace, out.Stacktrace)
}
if from.Sync.IsSet() {
Expand All @@ -692,6 +695,9 @@ func mapToSpanModel(from *span, event *modelpb.APMEvent) {

func mapToStracktraceModel(from []stacktraceFrame, out []*modelpb.StacktraceFrame) {
for idx, eventFrame := range from {
if out[idx] == nil {
out[idx] = &modelpb.StacktraceFrame{}
}
fr := out[idx]
if eventFrame.AbsPath.IsSet() {
fr.AbsPath = eventFrame.AbsPath.Val
Expand Down Expand Up @@ -720,11 +726,11 @@ func mapToStracktraceModel(from []stacktraceFrame, out []*modelpb.StacktraceFram
fr.Module = eventFrame.Module.Val
}
if len(eventFrame.PostContext) > 0 {
fr.PostContext = modeldecoderutil.Reslice(fr.PostContext, len(eventFrame.PostContext), nil)
fr.PostContext = modeldecoderutil.Reslice(fr.PostContext, len(eventFrame.PostContext))
copy(fr.PostContext, eventFrame.PostContext)
}
if len(eventFrame.PreContext) > 0 {
fr.PreContext = modeldecoderutil.Reslice(fr.PreContext, len(eventFrame.PreContext), nil)
fr.PreContext = modeldecoderutil.Reslice(fr.PreContext, len(eventFrame.PreContext))
copy(fr.PreContext, eventFrame.PreContext)
}
}
Expand Down
36 changes: 24 additions & 12 deletions input/elasticapm/internal/modeldecoder/v2/decoder.go
Original file line number Diff line number Diff line change
Expand Up @@ -477,7 +477,7 @@ func mapToErrorModel(from *errorEvent, event *modelpb.APMEvent) {
log.ParamMessage = from.Log.ParamMessage.Val
}
if len(from.Log.Stacktrace) > 0 {
log.Stacktrace = modeldecoderutil.Reslice(log.Stacktrace, len(from.Log.Stacktrace), modelpb.StacktraceFrameFromVTPool)
log.Stacktrace = modeldecoderutil.Reslice(log.Stacktrace, len(from.Log.Stacktrace))
mapToStracktraceModel(from.Log.Stacktrace, log.Stacktrace)
}
out.Log = log
Expand Down Expand Up @@ -521,8 +521,11 @@ func mapToExceptionModel(from *errorException, out *modelpb.Exception) {
out.Code = modeldecoderutil.ExceptionCodeString(from.Code.Val)
}
if len(from.Cause) > 0 {
out.Cause = modeldecoderutil.Reslice(out.Cause, len(from.Cause), modelpb.ExceptionFromVTPool)
out.Cause = modeldecoderutil.Reslice(out.Cause, len(from.Cause))
for i := 0; i < len(from.Cause); i++ {
if out.Cause[i] == nil {
out.Cause[i] = &modelpb.Exception{}
}
mapToExceptionModel(&from.Cause[i], out.Cause[i])
}
}
Expand All @@ -537,7 +540,7 @@ func mapToExceptionModel(from *errorException, out *modelpb.Exception) {
out.Module = from.Module.Val
}
if len(from.Stacktrace) > 0 {
out.Stacktrace = modeldecoderutil.Reslice(out.Stacktrace, len(from.Stacktrace), modelpb.StacktraceFrameFromVTPool)
out.Stacktrace = modeldecoderutil.Reslice(out.Stacktrace, len(from.Stacktrace))
mapToStracktraceModel(from.Stacktrace, out.Stacktrace)
}
if from.Type.IsSet() {
Expand Down Expand Up @@ -803,18 +806,21 @@ func mapToMetricsetModel(from *metricset, event *modelpb.APMEvent) bool {
}

if len(from.Samples) > 0 {
event.Metricset.Samples = modeldecoderutil.Reslice(event.Metricset.Samples, len(from.Samples), modelpb.MetricsetSampleFromVTPool)
event.Metricset.Samples = modeldecoderutil.Reslice(event.Metricset.Samples, len(from.Samples))
i := 0
for name, sample := range from.Samples {
if event.Metricset.Samples[i] == nil {
event.Metricset.Samples[i] = &modelpb.MetricsetSample{}
}
ms := event.Metricset.Samples[i]
if len(sample.Counts) != 0 || len(sample.Values) != 0 {
ms.Histogram = modelpb.HistogramFromVTPool()
if n := len(sample.Values); n > 0 {
ms.Histogram.Values = modeldecoderutil.Reslice(ms.Histogram.Values, n, nil)
ms.Histogram.Values = modeldecoderutil.Reslice(ms.Histogram.Values, n)
copy(ms.Histogram.Values, sample.Values)
}
if n := len(sample.Counts); n > 0 {
ms.Histogram.Counts = modeldecoderutil.Reslice(ms.Histogram.Counts, n, nil)
ms.Histogram.Counts = modeldecoderutil.Reslice(ms.Histogram.Counts, n)
copy(ms.Histogram.Counts, sample.Counts)
}
}
Expand Down Expand Up @@ -1080,7 +1086,7 @@ func mapToSpanModel(from *span, event *modelpb.APMEvent) {
out.Composite = composite
}
if len(from.ChildIDs) > 0 {
event.ChildIds = modeldecoderutil.Reslice(event.ChildIds, len(from.ChildIDs), nil)
event.ChildIds = modeldecoderutil.Reslice(event.ChildIds, len(from.ChildIDs))
copy(event.ChildIds, from.ChildIDs)
}
if from.Context.Database.IsSet() {
Expand Down Expand Up @@ -1272,7 +1278,7 @@ func mapToSpanModel(from *span, event *modelpb.APMEvent) {
out.RepresentativeCount = 1
}
if len(from.Stacktrace) > 0 {
out.Stacktrace = modeldecoderutil.Reslice(out.Stacktrace, len(from.Stacktrace), modelpb.StacktraceFrameFromVTPool)
out.Stacktrace = modeldecoderutil.Reslice(out.Stacktrace, len(from.Stacktrace))
mapToStracktraceModel(from.Stacktrace, out.Stacktrace)
}
if from.Sync.IsSet() {
Expand All @@ -1299,7 +1305,7 @@ func mapToSpanModel(from *span, event *modelpb.APMEvent) {
mapOTelAttributesSpan(from.OTel, event)
}
if len(from.Links) > 0 {
out.Links = modeldecoderutil.Reslice(out.Links, len(from.Links), modelpb.SpanLinkFromVTPool)
out.Links = modeldecoderutil.Reslice(out.Links, len(from.Links))
mapSpanLinks(from.Links, out.Links)
}
if out.Type == "" {
Expand All @@ -1309,6 +1315,9 @@ func mapToSpanModel(from *span, event *modelpb.APMEvent) {

func mapToStracktraceModel(from []stacktraceFrame, out []*modelpb.StacktraceFrame) {
for idx, eventFrame := range from {
if out[idx] == nil {
out[idx] = &modelpb.StacktraceFrame{}
}
fr := out[idx]
if eventFrame.AbsPath.IsSet() {
fr.AbsPath = eventFrame.AbsPath.Val
Expand Down Expand Up @@ -1341,11 +1350,11 @@ func mapToStracktraceModel(from []stacktraceFrame, out []*modelpb.StacktraceFram
fr.Module = eventFrame.Module.Val
}
if len(eventFrame.PostContext) > 0 {
fr.PostContext = modeldecoderutil.Reslice(fr.PostContext, len(eventFrame.PostContext), nil)
fr.PostContext = modeldecoderutil.Reslice(fr.PostContext, len(eventFrame.PostContext))
copy(fr.PostContext, eventFrame.PostContext)
}
if len(eventFrame.PreContext) > 0 {
fr.PreContext = modeldecoderutil.Reslice(fr.PreContext, len(eventFrame.PreContext), nil)
fr.PreContext = modeldecoderutil.Reslice(fr.PreContext, len(eventFrame.PreContext))
copy(fr.PreContext, eventFrame.PreContext)
}
if len(eventFrame.Vars) > 0 {
Expand Down Expand Up @@ -1569,7 +1578,7 @@ func mapToTransactionModel(from *transaction, event *modelpb.APMEvent) {
if event.Span == nil {
event.Span = modelpb.SpanFromVTPool()
}
event.Span.Links = modeldecoderutil.Reslice(event.Span.Links, len(from.Links), modelpb.SpanLinkFromVTPool)
event.Span.Links = modeldecoderutil.Reslice(event.Span.Links, len(from.Links))
mapSpanLinks(from.Links, event.Span.Links)
}
if out.Type == "" {
Expand Down Expand Up @@ -1882,6 +1891,9 @@ func isOTelDoubleAttribute(k string) bool {

func mapSpanLinks(from []spanLink, out []*modelpb.SpanLink) {
for i, link := range from {
if out[i] == nil {
out[i] = &modelpb.SpanLink{}
}
out[i].SpanId = link.SpanID.Val
out[i].TraceId = link.TraceID.Val
}
Expand Down

0 comments on commit 6fd1561

Please sign in to comment.