Skip to content

Commit

Permalink
perf: speed up adding fields, reduce memalloc if field name is alread…
Browse files Browse the repository at this point in the history
…y prefixed with "app." (#406)

## Which problem is this PR solving?

- Closes #401 

There is a remarkable amount of memory allocation occurring under load
to perform the "app." prefixing of field names by the Beeline.

## Short description of the changes

This change skips the memory allocations needed for string concat and
usage if the "app." prefix is already present on the field name
provided.

* doc comments updated to recommend users prefix their field names
  with "app." when calling these beeline.AddField*() functions
* doc comments also updated to make the handling of Error as value
  more clear

### Benchmarks

Existing behavior is no slower or memory hungry, but for every field
name provided by the Beeline user that starts with "app.", that is one
less memory allocation of the size of the field name string.

```
BenchmarkBeelineAddField/oldAddField/whatever
BenchmarkBeelineAddField/oldAddField/whatever-12                19654003                60.52 ns/op            8 B/op          1 allocs/op
BenchmarkBeelineAddField/AddField/no-prefix
BenchmarkBeelineAddField/AddField/no-prefix-12                  18939754                60.65 ns/op            8 B/op          1 allocs/op
BenchmarkBeelineAddField/AddField/half-prefixed
BenchmarkBeelineAddField/AddField/half-prefixed-12              22405790                51.22 ns/op            4 B/op          0 allocs/op
BenchmarkBeelineAddField/AddField/all-prefixed
BenchmarkBeelineAddField/AddField/all-prefixed-12               27381916                42.88 ns/op            0 B/op          0 allocs/op
```

---------

Co-authored-by: JamieDanielson <jamieedanielson@gmail.com>
  • Loading branch information
robbkidd and JamieDanielson authored Nov 30, 2023
1 parent e4fea20 commit 644691f
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 11 deletions.
34 changes: 27 additions & 7 deletions beeline.go
Original file line number Diff line number Diff line change
Expand Up @@ -272,15 +272,20 @@ func Close() {
// a Handler, feel free to call AddField freely within your code. Pass it the
// context from the request (`r.Context()`) and the key and value you wish to
// add.This function is good for span-level data, eg timers or the arguments to
// a specific function call, etc. Fields added here are prefixed with `app.`
// a specific function call, etc.
//
// Field keys added will be prefixed with 'app.' if the 'app.' prefix is not
// already present on the key name. If you provide a key that starts with
// 'app.', both speed and memory allocations are improved, especially within hot
// paths of your application.
//
// Errors are treated as a special case for convenience: if `val` is of type
// `error` then the key is set to the error's message in the span.
// `error` then the field's value is set to the error's message.
func AddField(ctx context.Context, key string, val interface{}) {
span := trace.GetSpanFromContext(ctx)
if span != nil {
if val != nil {
namespacedKey := "app." + key // Avoid excess parsing/allocation work
if val != nil { // TODO: move this to first check to save looking up the current span when there is no value?
namespacedKey := getNamespacedKey(key)
if valErr, ok := val.(error); ok {
// treat errors specially because it's a pain to have to
// remember to stringify them
Expand All @@ -297,16 +302,31 @@ func AddField(ctx context.Context, key string, val interface{}) {
// Additionally, these fields are packaged up and passed along to downstream
// processes if they are also using a beeline. This function is good for adding
// context that is better scoped to the request than this specific unit of work,
// eg user IDs, globally relevant feature flags, errors, etc. Fields added here
// are prefixed with `app.`
// eg user IDs, globally relevant feature flags, errors, etc.
//
// Field keys added will be prefixed with 'app.' if the 'app.' prefix is not
// already present on the key name. If you provide a key that starts with
// 'app.', both speed and memory allocations are improved, especially within hot
// paths of your application.
func AddFieldToTrace(ctx context.Context, key string, val interface{}) {
namespacedKey := "app." + key // Avoid excess parsing/allocation work
tr := trace.GetTraceFromContext(ctx)
if tr != nil {
namespacedKey := getNamespacedKey(key)
tr.AddField(namespacedKey, val)
}
}

// getNamespacedKey ensures a key name is prefixed with "app." if the prefix
// isn't already present. If the key is already namespaced, this reduces the
// number of memory allocations needed to add a field to a span.
func getNamespacedKey(key string) string {
const prefix = "app."
if strings.HasPrefix(key, prefix) {
return key
}
return prefix + key
}

// StartSpan lets you start a new span as a child of an already instrumented
// handler. If there isn't an existing wrapped handler in the context when this
// is called, it will start a new trace. Spans automatically get a `duration_ms`
Expand Down
23 changes: 20 additions & 3 deletions beeline_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,9 +105,26 @@ func BenchmarkBeelineAddField(b *testing.B) {
setupLibhoney(b)

ctx, _ := StartSpan(context.Background(), "parent")
for n := 0; n < b.N; n++ {
AddField(ctx, "foo", 1)
}

b.Run("AddField/no-prefix", func(b *testing.B) {
for n := 0; n < b.N; n++ {
AddField(ctx, "foo", 1)
}
})
b.Run("AddField/half-prefixed", func(b *testing.B) {
for n := 0; n < b.N; n++ {
if n%2 == 0 {
AddField(ctx, "app.foo", 1)
} else {
AddField(ctx, "foo", 1)
}
}
})
b.Run("AddField/all-prefixed", func(b *testing.B) {
for n := 0; n < b.N; n++ {
AddField(ctx, "app.foo", 1)
}
})
}

func setupLibhoney(t testing.TB) *transmission.MockSender {
Expand Down
2 changes: 1 addition & 1 deletion trace/trace.go
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ func newSpan() *Span {
// AddField adds a key/value pair to this span
//
// Errors are treated as a special case for convenience: if `val` is of type
// `error` then the key is set to the error's message in the span.
// `error` then the field's value is set to the error's message.
func (s *Span) AddField(key string, val interface{}) {
// The call to event's AddField is protected by a lock, but this is not always sufficient
// See send for why this lock exists
Expand Down

0 comments on commit 644691f

Please sign in to comment.