Deprecate PutLocation and introduce SetLocation (#14019)

Part of
#14016

Author: dmathieu

.chloggen/pprofile-deprecate-put-location.yaml

@@ -0,0 +1,25 @@
+# Use this changelog template to create an entry for release notes.
+
+# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
+change_type: deprecation
+
+# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver)
+component: pdata/pprofile
+
+# A brief description of the change.  Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: Deprecated `PutLocation` helper method
+
+# One or more tracking issues or pull requests related to the change
+issues: [14019]
+
+# (Optional) One or more lines of additional information to render under the primary note.
+# These lines will be padded with 2 spaces and then inserted directly into the document.
+# Use pipe (|) for multiline entries.
+subtext:
+
+# Optional: The change log or logs in which this entry should be included.
+# e.g. '[user]' or '[user, api]'
+# Include 'user' if the change is relevant to end users.
+# Include 'api' if there is a change to a library API.
+# Default: '[user]'
+change_logs: [api]

.chloggen/pprofile-set-location.yaml

@@ -0,0 +1,25 @@
+# Use this changelog template to create an entry for release notes.
+
+# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
+change_type: enhancement
+
+# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver)
+component: pdata/pprofile
+
+# A brief description of the change.  Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: Introduce `SetLocation` helper method
+
+# One or more tracking issues or pull requests related to the change
+issues: [14019]
+
+# (Optional) One or more lines of additional information to render under the primary note.
+# These lines will be padded with 2 spaces and then inserted directly into the document.
+# Use pipe (|) for multiline entries.
+subtext:
+
+# Optional: The change log or logs in which this entry should be included.
+# e.g. '[user]' or '[user, api]'
+# Include 'user' if the change is relevant to end users.
+# Include 'api' if there is a change to a library API.
+# Default: '[user]'
+change_logs: [api]

pdata/pprofile/locations.go

@@ -30,6 +30,8 @@ var (
 
 // PutLocation updates a LocationTable and a Stack's LocationIndices to
 // add or update a location.
+//
+// Deprecated: [v0.137.0] use SetLocation instead.
 func PutLocation(table LocationSlice, record Stack, loc Location) error {
 	for i, locIdx := range record.LocationIndices().All() {
 		idx := int(locIdx)
@@ -47,22 +49,30 @@ func PutLocation(table LocationSlice, record Stack, loc Location) error {
 		return errTooManyLocationIndicesEntries
 	}
 
+	id, err := SetLocation(table, loc)
+	if err != nil {
+		return err
+	}
+	record.LocationIndices().Append(id)
+	return nil
+}
+
+// SetLocation updates a LocationTable, adding or providing a value and returns
+// its index.
+func SetLocation(table LocationSlice, loc Location) (int32, error) {
 	for j, a := range table.All() {
 		if a.Equal(loc) {
 			if j > math.MaxInt32 {
-				return errTooManyLocationTableEntries
+				return 0, errTooManyLocationTableEntries
 			}
-			// Add the index of the existing location to the indices.
-			record.LocationIndices().Append(int32(j)) //nolint:gosec // G115 overflow checked
-			return nil
+			return int32(j), nil //nolint:gosec // G115 overflow checked
 		}
 	}
 
 	if table.Len() >= math.MaxInt32 {
-		return errTooManyLocationTableEntries
+		return 0, errTooManyLocationTableEntries
 	}
 
 	loc.CopyTo(table.AppendEmpty())
-	record.LocationIndices().Append(int32(table.Len() - 1)) //nolint:gosec // G115 overflow checked
-	return nil
+	return int32(table.Len() - 1), nil //nolint:gosec // G115 overflow checked
 }

pdata/pprofile/locations_test.go

@@ -87,6 +87,45 @@ func TestPutLocation(t *testing.T) {
 	require.Equal(t, indicesLen, stack.LocationIndices().Len())
 }
 
+func TestSetLocation(t *testing.T) {
+	table := NewLocationSlice()
+	l := NewLocation()
+	l.SetAddress(1)
+	l2 := NewLocation()
+	l2.SetAddress(2)
+
+	// Put a first value
+	idx, err := SetLocation(table, l)
+	require.NoError(t, err)
+	assert.Equal(t, 1, table.Len())
+	assert.Equal(t, int32(0), idx)
+
+	// Put the same string
+	// This should be a no-op.
+	idx, err = SetLocation(table, l)
+	require.NoError(t, err)
+	assert.Equal(t, 1, table.Len())
+	assert.Equal(t, int32(0), idx)
+
+	// Set a new value
+	// This sets the index and adds to the table.
+	idx, err = SetLocation(table, l2)
+	require.NoError(t, err)
+	assert.Equal(t, 2, table.Len())
+	assert.Equal(t, int32(table.Len()-1), idx) //nolint:gosec // G115
+
+	// Set an existing value
+	idx, err = SetLocation(table, l)
+	require.NoError(t, err)
+	assert.Equal(t, 2, table.Len())
+	assert.Equal(t, int32(0), idx)
+	// Set another existing value
+	idx, err = SetLocation(table, l2)
+	require.NoError(t, err)
+	assert.Equal(t, 2, table.Len())
+	assert.Equal(t, int32(table.Len()-1), idx) //nolint:gosec // G115
+}
+
 func BenchmarkFromLocationIndices(b *testing.B) {
 	table := NewLocationSlice()
 
@@ -172,3 +211,72 @@ func BenchmarkPutLocation(b *testing.B) {
 		})
 	}
 }
+
+func BenchmarkSetLocation(b *testing.B) {
+	for _, bb := range []struct {
+		name     string
+		location Location
+
+		runBefore func(*testing.B, LocationSlice)
+	}{
+		{
+			name:     "with a new location",
+			location: NewLocation(),
+		},
+		{
+			name: "with an existing location",
+			location: func() Location {
+				l := NewLocation()
+				l.SetAddress(1)
+				return l
+			}(),
+
+			runBefore: func(_ *testing.B, table LocationSlice) {
+				l := table.AppendEmpty()
+				l.SetAddress(1)
+			},
+		},
+		{
+			name:     "with a duplicate location",
+			location: NewLocation(),
+
+			runBefore: func(_ *testing.B, table LocationSlice) {
+				_, err := SetLocation(table, NewLocation())
+				require.NoError(b, err)
+			},
+		},
+		{
+			name: "with a hundred locations to loop through",
+			location: func() Location {
+				l := NewLocation()
+				l.SetMappingIndex(1)
+				return l
+			}(),
+
+			runBefore: func(_ *testing.B, table LocationSlice) {
+				for i := range 100 {
+					l := table.AppendEmpty()
+					l.SetAddress(uint64(i)) //nolint:gosec // overflow checked
+				}
+
+				l := table.AppendEmpty()
+				l.SetMappingIndex(1)
+			},
+		},
+	} {
+		b.Run(bb.name, func(b *testing.B) {
+			table := NewLocationSlice()
+
+			if bb.runBefore != nil {
+				bb.runBefore(b, table)
+			}
+
+			b.ResetTimer()
+			b.ReportAllocs()
+
+			for b.Loop() {
+				_, _ = SetLocation(table, bb.location)
+			}
+		})
+	}
+}

pdata/testdata/profile.go

@@ -57,7 +57,9 @@ func fillProfileOne(dic pprofile.ProfilesDictionary, profile pprofile.Profile) {
 
 	loc := pprofile.NewLocation()
 	loc.SetAddress(1)
-	_ = pprofile.PutLocation(dic.LocationTable(), dic.StackTable().AppendEmpty(), loc)
+	id, _ := pprofile.SetLocation(dic.LocationTable(), loc)
+	stack := dic.StackTable().AppendEmpty()
+	stack.LocationIndices().Append(id)
 
 	sample := profile.Sample().AppendEmpty()
 	sample.SetStackIndex(1)
@@ -76,7 +78,9 @@ func fillProfileTwo(dic pprofile.ProfilesDictionary, profile pprofile.Profile) {
 
 	loc := pprofile.NewLocation()
 	loc.SetAddress(2)
-	_ = pprofile.PutLocation(dic.LocationTable(), dic.StackTable().AppendEmpty(), loc)
+	id, _ := pprofile.SetLocation(dic.LocationTable(), loc)
+	stack := dic.StackTable().AppendEmpty()
+	stack.LocationIndices().Append(id)
 
 	sample := profile.Sample().AppendEmpty()
 	sample.SetStackIndex(1)