Harmonize profile helper methods to have similar signatures

[dmathieu]

We have several helper methods in profiles that allow manipulating dictionary indices:

• PutAttribute(table KeyValueAndUnitSlice, record attributable, dic ProfilesDictionary, ...) error
  • Deprecate PutAttribute and introduce SetAttribute #14041
• PutLocation(table LocationSlice, record Stack, loc Location) error
  • Deprecate PutLocation and introduce SetLocation #14019
• SetFunction(table FunctionSlice, record Line, fn Function) error
  • Change SetFunction signature to return the function's ID, not update #14032
• SetLink(table LinkSlice, record Sample, li Link) error
  • Change SetLink signature to return the link's ID, not update #14031
• SetMapping(table MappingSlice, record Location, ma Mapping) error
  • Change SetMapping signature to return the mapping's ID, not update #14030
• SetString(table pcommon.StringSlice, val string) (int32, error)
• SetStack(table StackSlice, st Stack) (int32, error)
  • pprofile: introduce SetStack helper method #14007

We should harmonize those methods so they all have similar signatures.
Right now, some are named PutX, and others are named SetX. I suggest we name everything SetX.

Right now, some methods take a record and update it in place, while others return the ID of the data that was retrieved or added to the dictionary.
Updating in place is not the most efficient way to do this, especially for merging profiles, as for slices of records (for example, attributes in a stack), we need to create a new slice and use the helper method on it.
If we return an ID, we can update the ID directly within the existing slice.
So I suggest we switch every helper method to use the signature of returning an ID (as SetString does).

[dmathieu]

cc @mx-psi @bogdandrutu

[mx-psi]

I support unifying this. Your reasoning looks alright to me, but maybe to decide among the alternatives we could have some mini benchmarks of the alternatives?

[dmathieu]

Now

With the current implementation, this is how we would do a switchDictionary (#13106 (comment)) for a Stack

func (ms Stack) switchDictionary(src, dst ProfilesDictionary) error {
	oldIDs := pcommon.NewInt32Slice()
	ms.LocationIndices().MoveTo(oldIDs)

	for _, locIdx := range oldIDs.All() {
		err := PutLocation(dst.LocationTable(), ms, src.LocationTable().At(int(locIdx)))
		if err != nil {
			return fmt.Errorf("error setting stack location: %w", err)
		}
	}

	return nil
}

And a benchmark with a stack that has 100 locations:

func BenchmarkStackSwitchDictionary(b *testing.B) {
	d1 := NewProfilesDictionary()
	d2 := NewProfilesDictionary()
	s := NewStack()

	for i := range 100 {
		loc := NewLocation()
		loc.SetAddress(uint64(i))
		require.NoError(b, PutLocation(d1.LocationTable(), s, loc))
	}

	b.ResetTimer()
	b.ReportAllocs()

	b.ResetTimer()
	b.ReportAllocs()

	for b.Loop() {
		_ = s.switchDictionary(d1, d2)
	}
}

After

After we return the ID in PutLocation rather than updating the sample, we can have the following switchDictionary:

func (ms Stack) switchDictionary(src, dst ProfilesDictionary) error {
	for i, locIdx := range ms.LocationIndices().All() {
		id, err := PutLocation(dst.LocationTable(), src.LocationTable().At(int(locIdx)))
		if err != nil {
			return fmt.Errorf("error setting stack location: %w", err)
		}
		ms.LocationIndices().SetAt(i, id)
	}

	return nil
}

And the following benchmark:

func BenchmarkStackSwitchDictionary(b *testing.B) {
	d1 := NewProfilesDictionary()
	d2 := NewProfilesDictionary()
	s := NewStack()

	for i := range 100 {
		loc := NewLocation()
		loc.SetAddress(uint64(i))
		_, err := PutLocation(d1.LocationTable(), loc)
		require.NoError(b, err)
	}

	b.ResetTimer()
	b.ReportAllocs()

	b.ResetTimer()
	b.ReportAllocs()

	for b.Loop() {
		_ = s.switchDictionary(d1, d2)
	}
}

Results

Before, the benchmarks results are:

goos: darwin
goarch: arm64
pkg: go.opentelemetry.io/collector/pdata/pprofile
cpu: Apple M1 Max
BenchmarkStackSwitchDictionary-10          38931             30078 ns/op            1016 B/op          7 allocs/op
PASS
ok      go.opentelemetry.io/collector/pdata/pprofile    1.574s
PASS
ok      go.opentelemetry.io/collector/pdata/pprofile/pprofileotlp       0.360s

After, they are:

goos: darwin
goarch: arm64
pkg: go.opentelemetry.io/collector/pdata/pprofile
cpu: Apple M1 Max
BenchmarkStackSwitchDictionary-10       448911040                2.667 ns/op           0 B/op          0 allocs/op
PASS
ok      go.opentelemetry.io/collector/pdata/pprofile    1.531s
PASS
ok      go.opentelemetry.io/collector/pdata/pprofile/pprofileotlp       0.368s

This not only goes from 7 allocations to 0, but it also vastly decreases the duration of each operation.

[mx-psi]

Oki, looks alright to me, thanks for doing the check. All good from my side to file a PR