From c6be49126181ddb5422c608f722481b1a5329fb7 Mon Sep 17 00:00:00 2001 From: Tommy Reilly Date: Tue, 5 Mar 2024 14:18:01 -0500 Subject: [PATCH 1/2] Use object file pool cache for unwind information --- cmd/eh-frame/main.go | 16 +++++++--- pkg/profiler/cpu/bpf/maps/maps.go | 2 +- pkg/stack/unwind/compact_unwind_table.go | 7 +++-- pkg/stack/unwind/compact_unwind_table_test.go | 30 ++++++++++++++++--- pkg/stack/unwind/unwind_table.go | 11 ++++--- pkg/stack/unwind/unwind_table_test.go | 6 ++-- 6 files changed, 51 insertions(+), 21 deletions(-) diff --git a/cmd/eh-frame/main.go b/cmd/eh-frame/main.go index f4e6694f26..0a3bd726e4 100644 --- a/cmd/eh-frame/main.go +++ b/cmd/eh-frame/main.go @@ -20,8 +20,10 @@ import ( "os" "github.com/alecthomas/kong" + "github.com/prometheus/client_golang/prometheus" "github.com/parca-dev/parca-agent/pkg/logger" + "github.com/parca-dev/parca-agent/pkg/objectfile" "github.com/parca-dev/parca-agent/pkg/stack/unwind" ) @@ -36,7 +38,7 @@ type flags struct { // developers. func main() { logger := logger.NewLogger("warn", logger.LogFormatLogfmt, "eh-frame") - + objFilePool := objectfile.NewPool(logger, prometheus.NewRegistry(), "", 10, 0) flags := flags{} kong.Parse(&flags) @@ -54,8 +56,15 @@ func main() { pc = &flags.RelativePC } + file, err := objFilePool.Open(executablePath) + if err != nil { + // nolint:forbidigo + fmt.Println("failed with:", err) + return + } + if flags.Final { - ut, arch, err := unwind.GenerateCompactUnwindTable(executablePath) + ut, arch, err := unwind.GenerateCompactUnwindTable(file) if err != nil { // nolint:forbidigo fmt.Println("failed with:", err) @@ -70,8 +79,7 @@ func main() { } ptb := unwind.NewUnwindTableBuilder(logger) - err := ptb.PrintTable(os.Stdout, executablePath, flags.Compact, pc) - if err != nil { + if err := ptb.PrintTable(os.Stdout, file, flags.Compact, pc); err != nil { // nolint:forbidigo fmt.Println("failed with:", err) } diff --git a/pkg/profiler/cpu/bpf/maps/maps.go b/pkg/profiler/cpu/bpf/maps/maps.go index 41ac1b3cab..e10fa438c7 100644 --- a/pkg/profiler/cpu/bpf/maps/maps.go +++ b/pkg/profiler/cpu/bpf/maps/maps.go @@ -1900,7 +1900,7 @@ func (m *Maps) setUnwindTableForMapping(buf *profiler.EfficientBuffer, pid int, // Generate the unwind table. // PERF(javierhonduco): Not reusing a buffer here yet, let's profile and decide whether this // change would be worth it. - ut, arch, err := unwind.GenerateCompactUnwindTable(fullExecutablePath) + ut, arch, err := unwind.GenerateCompactUnwindTable(f) level.Debug(m.logger).Log("msg", "found unwind entries", "executable", mapping.Executable, "len", len(ut)) if err != nil { diff --git a/pkg/stack/unwind/compact_unwind_table.go b/pkg/stack/unwind/compact_unwind_table.go index e052b2a194..28fdfb5112 100644 --- a/pkg/stack/unwind/compact_unwind_table.go +++ b/pkg/stack/unwind/compact_unwind_table.go @@ -21,6 +21,7 @@ import ( "sort" "github.com/parca-dev/parca-agent/internal/dwarf/frame" + "github.com/parca-dev/parca-agent/pkg/objectfile" ) type BpfCfaType uint8 @@ -278,11 +279,11 @@ func CompactUnwindTableRepresentation(unwindTable UnwindTable, arch elf.Machine) // GenerateCompactUnwindTable produces the compact unwind table for a given // executable. -func GenerateCompactUnwindTable(fullExecutablePath string) (CompactUnwindTable, elf.Machine, error) { +func GenerateCompactUnwindTable(file *objectfile.ObjectFile) (CompactUnwindTable, elf.Machine, error) { var ut CompactUnwindTable // Fetch FDEs. - fdes, arch, err := ReadFDEs(fullExecutablePath) + fdes, arch, err := ReadFDEs(file) if err != nil { return ut, arch, err } @@ -294,7 +295,7 @@ func GenerateCompactUnwindTable(fullExecutablePath string) (CompactUnwindTable, // Generate the compact unwind table. ut, err = BuildCompactUnwindTable(fdes, arch) if err != nil { - return ut, arch, fmt.Errorf("build compact unwind table for executable %q: %w", fullExecutablePath, err) + return ut, arch, fmt.Errorf("build compact unwind table for executable %q: %w", file.Path, err) } // This should not be necessary, as per the sorting above, but diff --git a/pkg/stack/unwind/compact_unwind_table_test.go b/pkg/stack/unwind/compact_unwind_table_test.go index 43ed6e9e9f..6f92974848 100644 --- a/pkg/stack/unwind/compact_unwind_table_test.go +++ b/pkg/stack/unwind/compact_unwind_table_test.go @@ -17,9 +17,13 @@ package unwind import ( "debug/elf" "path/filepath" + "sync" "testing" + "github.com/go-kit/log" "github.com/parca-dev/parca-agent/internal/dwarf/frame" + "github.com/parca-dev/parca-agent/pkg/objectfile" + "github.com/prometheus/client_golang/prometheus" "github.com/stretchr/testify/require" ) @@ -451,12 +455,30 @@ func requireNoRedundantRows(t *testing.T, ut CompactUnwindTable) { } } +var ( + testPool *objectfile.Pool + once sync.Once +) + +func objectFile(tb testing.TB, path string) *objectfile.ObjectFile { + tb.Helper() + once.Do(func() { + testPool = objectfile.NewPool(log.NewNopLogger(), prometheus.NewRegistry(), "", 100, 10) + tb.Cleanup(func() { + testPool.Close() + }) + }) + o, err := testPool.Open(path) + require.NoError(tb, err) + return o +} + func TestIsSorted(t *testing.T) { matches, err := filepath.Glob("../../../testdata/vendored/x86/*") require.NoError(t, err) for _, match := range matches { - ut, _, err := GenerateCompactUnwindTable(match) + ut, _, err := GenerateCompactUnwindTable(objectFile(t, match)) require.NoError(t, err) requireSorted(t, ut) } @@ -468,7 +490,7 @@ func TestNoRepeatedPCs(t *testing.T) { require.NoError(t, err) for _, match := range matches { - ut, _, err := GenerateCompactUnwindTable(match) + ut, _, err := GenerateCompactUnwindTable(objectFile(t, match)) require.NoError(t, err) requireNoDuplicatedPC(t, ut) } @@ -479,7 +501,7 @@ func TestNoRedundantRows(t *testing.T) { require.NoError(t, err) for _, match := range matches { - ut, _, err := GenerateCompactUnwindTable(match) + ut, _, err := GenerateCompactUnwindTable(objectFile(t, match)) require.NoError(t, err) requireNoRedundantRows(t, ut) } @@ -495,7 +517,7 @@ func BenchmarkGenerateCompactUnwindTable(b *testing.B) { var cut CompactUnwindTable var err error for n := 0; n < b.N; n++ { - cut, _, err = GenerateCompactUnwindTable(objectFilePath) + cut, _, err = GenerateCompactUnwindTable(objectFile(b, objectFilePath)) } require.NoError(b, err) diff --git a/pkg/stack/unwind/unwind_table.go b/pkg/stack/unwind/unwind_table.go index 194458ce29..a680851f01 100644 --- a/pkg/stack/unwind/unwind_table.go +++ b/pkg/stack/unwind/unwind_table.go @@ -24,6 +24,7 @@ import ( "github.com/go-kit/log/level" "github.com/parca-dev/parca-agent/internal/dwarf/frame" + "github.com/parca-dev/parca-agent/pkg/objectfile" ) var ( @@ -79,12 +80,11 @@ func registerToString(reg uint64, arch elf.Machine) string { } // PrintTable is a debugging helper that prints the unwinding table to the given io.Writer. -func (ptb *UnwindTableBuilder) PrintTable(writer io.Writer, path string, compact bool, pc *uint64) error { - fdes, arch, err := ReadFDEs(path) +func (ptb *UnwindTableBuilder) PrintTable(writer io.Writer, file *objectfile.ObjectFile, compact bool, pc *uint64) error { + fdes, arch, err := ReadFDEs(file) if err != nil { return err } - // The frame package can raise in case of malformed unwind data. defer func() { if r := recover(); r != nil { @@ -199,9 +199,8 @@ func (ptb *UnwindTableBuilder) PrintTable(writer io.Writer, path string, compact return nil } -func ReadFDEs(path string) (frame.FrameDescriptionEntries, elf.Machine, error) { - // TODO(kakkoyun): Migrate objectfile and pool. - obj, err := elf.Open(path) +func ReadFDEs(file *objectfile.ObjectFile) (frame.FrameDescriptionEntries, elf.Machine, error) { + obj, err := file.ELF() if err != nil { return nil, elf.EM_NONE, fmt.Errorf("failed to open elf: %w", err) } diff --git a/pkg/stack/unwind/unwind_table_test.go b/pkg/stack/unwind/unwind_table_test.go index 8809da4742..287d1a6fd1 100644 --- a/pkg/stack/unwind/unwind_table_test.go +++ b/pkg/stack/unwind/unwind_table_test.go @@ -24,7 +24,7 @@ import ( // TODO(Sylfrena): Add equivalent test for arm64. func TestBuildUnwindTable(t *testing.T) { - fdes, _, err := ReadFDEs("../../../testdata/out/x86/basic-cpp") + fdes, _, err := ReadFDEs(objectFile(t, "../../../testdata/out/x86/basic-cpp")) require.NoError(t, err) unwindTable, err := BuildUnwindTable(fdes) @@ -52,7 +52,7 @@ func TestSpecialOpcodes(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - fdes, _, err := ReadFDEs(tt.executable) + fdes, _, err := ReadFDEs(objectFile(t, tt.executable)) require.NoError(t, err) unwindTable, err := BuildUnwindTable(fdes) @@ -71,7 +71,7 @@ func benchmarkParsingDWARFUnwindInformation(b *testing.B, executable string) { var rbpOffset int64 for n := 0; n < b.N; n++ { - fdes, _, err := ReadFDEs(executable) + fdes, _, err := ReadFDEs(objectFile(b, executable)) if err != nil { panic("could not read FDEs") } From f4ea3c456325a9e28ba48ac1fc7319823636fd47 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci-lite[bot]" <117423508+pre-commit-ci-lite[bot]@users.noreply.github.com> Date: Wed, 6 Mar 2024 19:54:08 +0000 Subject: [PATCH 2/2] [pre-commit.ci lite] apply automatic fixes --- pkg/stack/unwind/compact_unwind_table_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/stack/unwind/compact_unwind_table_test.go b/pkg/stack/unwind/compact_unwind_table_test.go index 6f92974848..8a4f861162 100644 --- a/pkg/stack/unwind/compact_unwind_table_test.go +++ b/pkg/stack/unwind/compact_unwind_table_test.go @@ -21,9 +21,10 @@ import ( "testing" "github.com/go-kit/log" + "github.com/prometheus/client_golang/prometheus" + "github.com/parca-dev/parca-agent/internal/dwarf/frame" "github.com/parca-dev/parca-agent/pkg/objectfile" - "github.com/prometheus/client_golang/prometheus" "github.com/stretchr/testify/require" )