diff --git a/collector/internal/controller.go b/collector/internal/controller.go index 17662a552..dc10eac58 100644 --- a/collector/internal/controller.go +++ b/collector/internal/controller.go @@ -34,7 +34,7 @@ func NewController(cfg *controller.Config, ReportInterval: intervals.ReportInterval(), ExecutablesCacheElements: 16384, // Next step: Calculate FramesCacheElements from numCores and samplingRate. - FramesCacheElements: 65536, + FramesCacheElements: 131072, CGroupCacheElements: 1024, SamplesPerSecond: cfg.SamplesPerSecond, }, nextConsumer) diff --git a/main.go b/main.go index ddc387858..97302c8f7 100644 --- a/main.go +++ b/main.go @@ -126,7 +126,7 @@ func mainWithExitCode() exitCode { ReportInterval: intervals.ReportInterval(), ExecutablesCacheElements: 16384, // Next step: Calculate FramesCacheElements from numCores and samplingRate. - FramesCacheElements: 65536, + FramesCacheElements: 131072, CGroupCacheElements: 1024, SamplesPerSecond: cfg.SamplesPerSecond, KernelVersion: kernelVersion, diff --git a/reporter/base_reporter.go b/reporter/base_reporter.go index 7092ae13f..8e11197d6 100644 --- a/reporter/base_reporter.go +++ b/reporter/base_reporter.go @@ -73,13 +73,7 @@ func (b *baseReporter) ExecutableKnown(fileID libpf.FileID) bool { } func (b *baseReporter) FrameKnown(frameID libpf.FrameID) bool { - known := false - if frameMapLock, exists := b.pdata.Frames.GetAndRefresh(frameID.FileID(), - pdata.FramesCacheLifetime); exists { - frameMap := frameMapLock.WLock() - defer frameMapLock.WUnlock(&frameMap) - _, known = (*frameMap).GetAndRefresh(frameID.AddressOrLine(), pdata.FrameMapLifetime) - } + _, known := b.pdata.Frames.GetAndRefresh(frameID, pdata.FrameMapLifetime) return known } @@ -145,53 +139,19 @@ func (b *baseReporter) ReportTraceEvent(trace *libpf.Trace, meta *samples.TraceE } func (b *baseReporter) FrameMetadata(args *FrameMetadataArgs) { - fileID := args.FrameID.FileID() - addressOrLine := args.FrameID.AddressOrLine() - log.Debugf("FrameMetadata [%x] %v+%v at %v:%v", - fileID, args.FunctionName, args.FunctionOffset, + args.FrameID.FileID(), args.FunctionName, args.FunctionOffset, args.SourceFile, args.SourceLine) - - if frameMapLock, exists := b.pdata.Frames.GetAndRefresh(fileID, - pdata.FramesCacheLifetime); exists { - frameMap := frameMapLock.WLock() - defer frameMapLock.WUnlock(&frameMap) - - sourceFile := args.SourceFile - if sourceFile == "" { - // The new SourceFile may be empty, and we don't want to overwrite - // an existing filePath with it. - if source, exists := (*frameMap).GetAndRefresh(addressOrLine, - pdata.FrameMapLifetime); exists { - sourceFile = source.FilePath - } - } - - (*frameMap).Add(addressOrLine, samples.SourceInfo{ - LineNumber: args.SourceLine, - FilePath: sourceFile, - FunctionOffset: args.FunctionOffset, - FunctionName: args.FunctionName, - }) - - return - } - - frameMap, err := lru.New[libpf.AddressOrLineno, samples.SourceInfo](1024, - func(k libpf.AddressOrLineno) uint32 { return uint32(k) }) - if err != nil { - log.Errorf("Failed to create inner frameMap for %x: %v", fileID, err) - return - } - frameMap.SetLifetime(pdata.FrameMapLifetime) - - frameMap.Add(addressOrLine, samples.SourceInfo{ + si := samples.SourceInfo{ LineNumber: args.SourceLine, FilePath: args.SourceFile, FunctionOffset: args.FunctionOffset, FunctionName: args.FunctionName, - }) - - mu := xsync.NewRWMutex(frameMap) - b.pdata.Frames.Add(fileID, &mu) + } + if si.FilePath == "" { + if oldsi, exists := b.pdata.Frames.Get(args.FrameID); exists { + si.FilePath = oldsi.FilePath + } + } + b.pdata.Frames.Add(args.FrameID, si) } diff --git a/reporter/internal/pdata/generate.go b/reporter/internal/pdata/generate.go index d8746f3d6..642754014 100644 --- a/reporter/internal/pdata/generate.go +++ b/reporter/internal/pdata/generate.go @@ -175,30 +175,21 @@ func (p *Pdata) setProfile( // Store interpreted frame information as a Line message: line := loc.Line().AppendEmpty() - fileIDInfoLock, exists := p.Frames.GetAndRefresh(traceInfo.Files[i], - FramesCacheLifetime) - if !exists { + if si, exists := p.Frames.GetAndRefresh( + libpf.NewFrameID(traceInfo.Files[i], traceInfo.Linenos[i]), + FramesCacheLifetime); exists { + line.SetLine(int64(si.LineNumber)) + + line.SetFunctionIndex(createFunctionEntry(funcMap, + si.FunctionName, si.FilePath)) + } else { // At this point, we do not have enough information for the frame. // Therefore, we report a dummy entry and use the interpreter as filename. + // To differentiate this case from the case where no information about + // the file ID is available at all, we use a different name for reported + // function. line.SetFunctionIndex(createFunctionEntry(funcMap, - "UNREPORTED", frameKind.String())) - } else { - fileIDInfo := fileIDInfoLock.RLock() - if si, exists := (*fileIDInfo).Get(traceInfo.Linenos[i]); exists { - line.SetLine(int64(si.LineNumber)) - - line.SetFunctionIndex(createFunctionEntry(funcMap, - si.FunctionName, si.FilePath)) - } else { - // At this point, we do not have enough information for the frame. - // Therefore, we report a dummy entry and use the interpreter as filename. - // To differentiate this case from the case where no information about - // the file ID is available at all, we use a different name for reported - // function. - line.SetFunctionIndex(createFunctionEntry(funcMap, - "UNRESOLVED", frameKind.String())) - } - fileIDInfoLock.RUnlock(&fileIDInfo) + "UNRESOLVED", frameKind.String())) } // To be compliant with the protocol, generate a dummy mapping entry. diff --git a/reporter/internal/pdata/generate_test.go b/reporter/internal/pdata/generate_test.go index b7e3548fc..ef5c92eff 100644 --- a/reporter/internal/pdata/generate_test.go +++ b/reporter/internal/pdata/generate_test.go @@ -7,10 +7,7 @@ import ( "github.com/stretchr/testify/require" "go.opentelemetry.io/collector/pdata/pprofile" - lru "github.com/elastic/go-freelru" - "go.opentelemetry.io/ebpf-profiler/libpf" - "go.opentelemetry.io/ebpf-profiler/libpf/xsync" "go.opentelemetry.io/ebpf-profiler/reporter/samples" "go.opentelemetry.io/ebpf-profiler/support" ) @@ -251,22 +248,10 @@ func TestFunctionTableOrder(t *testing.T) { t.Run(tt.name, func(t *testing.T) { d, err := New(100, 100, 100, nil) require.NoError(t, err) - for k, v := range tt.frames { - frameMap, err := lru.New[libpf.AddressOrLineno, samples.SourceInfo](4096, - func(k libpf.AddressOrLineno) uint32 { return uint32(k) }) - if err != nil { - t.Fatalf("Failed to create inner frameMap for %x: %v", k, err) - return - } - - for a, s := range v { - frameMap.Add(a, samples.SourceInfo{ - FunctionName: s.FunctionName, - }) + for fileID, addrWithSourceInfos := range tt.frames { + for addr, si := range addrWithSourceInfos { + d.Frames.Add(libpf.NewFrameID(fileID, addr), si) } - - mu := xsync.NewRWMutex(frameMap) - d.Frames.Add(k, &mu) } for k, v := range tt.executables { d.Executables.Add(k, v) diff --git a/reporter/internal/pdata/pdata.go b/reporter/internal/pdata/pdata.go index 02905936b..43caefd8d 100644 --- a/reporter/internal/pdata/pdata.go +++ b/reporter/internal/pdata/pdata.go @@ -7,7 +7,6 @@ import ( lru "github.com/elastic/go-freelru" "go.opentelemetry.io/ebpf-profiler/libpf" - "go.opentelemetry.io/ebpf-profiler/libpf/xsync" "go.opentelemetry.io/ebpf-profiler/reporter/samples" ) @@ -21,10 +20,7 @@ type Pdata struct { Executables *lru.SyncedLRU[libpf.FileID, samples.ExecInfo] // Frames maps frame information to its source location. - Frames *lru.SyncedLRU[ - libpf.FileID, - *xsync.RWMutex[*lru.LRU[libpf.AddressOrLineno, samples.SourceInfo]], - ] + Frames *lru.SyncedLRU[libpf.FrameID, samples.SourceInfo] // ExtraSampleAttrProd is an optional hook point for adding custom // attributes to samples. @@ -40,9 +36,8 @@ func New(samplesPerSecond int, executablesCacheElements, framesCacheElements uin } executables.SetLifetime(ExecutableCacheLifetime) // Allow GC to clean stale items. - frames, err := lru.NewSynced[libpf.FileID, - *xsync.RWMutex[*lru.LRU[libpf.AddressOrLineno, samples.SourceInfo]]]( - framesCacheElements, libpf.FileID.Hash32) + frames, err := + lru.NewSynced[libpf.FrameID, samples.SourceInfo](framesCacheElements, libpf.FrameID.Hash32) if err != nil { return nil, err }