From 12fb84a059f00ab629fe7dcb3978568e1435494b Mon Sep 17 00:00:00 2001 From: dmathieu <42@dmathieu.com> Date: Wed, 30 Oct 2024 14:32:52 +0100 Subject: [PATCH] move traceCacheSize back into a private method --- internal/controller/cache_size.go | 38 ------------------------------- internal/controller/controller.go | 33 ++++++++++++++++++++++++++- 2 files changed, 32 insertions(+), 39 deletions(-) delete mode 100644 internal/controller/cache_size.go diff --git a/internal/controller/cache_size.go b/internal/controller/cache_size.go deleted file mode 100644 index d742522b..00000000 --- a/internal/controller/cache_size.go +++ /dev/null @@ -1,38 +0,0 @@ -package controller // import "go.opentelemetry.io/ebpf-profiler/internal/controller" - -import ( - "fmt" - "time" - - "github.com/tklauser/numcpus" - "go.opentelemetry.io/ebpf-profiler/util" -) - -// TraceCacheSize defines the maximum number of elements for the caches in tracehandler. -// The caches in tracehandler have a size-"processing overhead" trade-off: Every cache miss will -// trigger additional processing for that trace in userspace (Go). For most maps, we use -// maxElementsPerInterval as a base sizing factor. For the tracehandler caches, we also multiply -// with traceCacheIntervals. For typical/small values of maxElementsPerInterval, this can lead to -// non-optimal map sizing (reduced cache_hit:cache_miss ratio and increased processing overhead). -// Simply increasing traceCacheIntervals is problematic when maxElementsPerInterval is large -// (e.g. too many CPU cores present) as we end up using too much memory. A minimum size is -// therefore used here. -func TraceCacheSize(monitorInterval time.Duration, samplesPerSecond int) (uint32, error) { - const ( - traceCacheIntervals = 6 - traceCacheMinSize = 65536 - ) - - presentCores, err := numcpus.GetPresent() - if err != nil { - return 0, fmt.Errorf("failed to read CPU file: %w", err) - } - - maxElements := maxElementsPerInterval(monitorInterval, samplesPerSecond, uint16(presentCores)) - - size := maxElements * uint32(traceCacheIntervals) - if size < traceCacheMinSize { - size = traceCacheMinSize - } - return util.NextPowerOfTwo(size), nil -} diff --git a/internal/controller/controller.go b/internal/controller/controller.go index df4f26b2..750f1f92 100644 --- a/internal/controller/controller.go +++ b/internal/controller/controller.go @@ -6,6 +6,7 @@ import ( "time" log "github.com/sirupsen/logrus" + "github.com/tklauser/numcpus" "go.opentelemetry.io/ebpf-profiler/host" "go.opentelemetry.io/ebpf-profiler/hostmetadata" @@ -16,6 +17,7 @@ import ( "go.opentelemetry.io/ebpf-profiler/tracehandler" "go.opentelemetry.io/ebpf-profiler/tracer" tracertypes "go.opentelemetry.io/ebpf-profiler/tracer/types" + "go.opentelemetry.io/ebpf-profiler/util" ) const MiB = 1 << 20 @@ -51,7 +53,7 @@ func (c *Controller) Start(ctx context.Context) error { } traceHandlerCacheSize, err := - TraceCacheSize(c.config.MonitorInterval, c.config.SamplesPerSecond) + traceCacheSize(c.config.MonitorInterval, c.config.SamplesPerSecond) if err != nil { return fmt.Errorf("retrieve trace cache size: %w", err) } @@ -180,6 +182,35 @@ func startTraceHandling(ctx context.Context, rep reporter.TraceReporter, return err } +// traceCacheSize defines the maximum number of elements for the caches in tracehandler. +// The caches in tracehandler have a size-"processing overhead" trade-off: Every cache miss will +// trigger additional processing for that trace in userspace (Go). For most maps, we use +// maxElementsPerInterval as a base sizing factor. For the tracehandler caches, we also multiply +// with traceCacheIntervals. For typical/small values of maxElementsPerInterval, this can lead to +// non-optimal map sizing (reduced cache_hit:cache_miss ratio and increased processing overhead). +// Simply increasing traceCacheIntervals is problematic when maxElementsPerInterval is large +// (e.g. too many CPU cores present) as we end up using too much memory. A minimum size is +// therefore used here. +func traceCacheSize(monitorInterval time.Duration, samplesPerSecond int) (uint32, error) { + const ( + traceCacheIntervals = 6 + traceCacheMinSize = 65536 + ) + + presentCores, err := numcpus.GetPresent() + if err != nil { + return 0, fmt.Errorf("failed to read CPU file: %w", err) + } + + maxElements := maxElementsPerInterval(monitorInterval, samplesPerSecond, uint16(presentCores)) + + size := maxElements * uint32(traceCacheIntervals) + if size < traceCacheMinSize { + size = traceCacheMinSize + } + return util.NextPowerOfTwo(size), nil +} + func maxElementsPerInterval(monitorInterval time.Duration, samplesPerSecond int, presentCPUCores uint16) uint32 { return uint32(uint16(samplesPerSecond) * uint16(monitorInterval.Seconds()) * presentCPUCores)