Skip to content

Commit

Permalink
Use object file pool cache for unwind information
Browse files Browse the repository at this point in the history
  • Loading branch information
gnurizen committed Mar 6, 2024
1 parent 1d9eb04 commit 2fdae14
Show file tree
Hide file tree
Showing 6 changed files with 52 additions and 21 deletions.
16 changes: 12 additions & 4 deletions cmd/eh-frame/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand All @@ -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)

Expand All @@ -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)
Expand All @@ -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)
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/profiler/cpu/bpf/maps/maps.go
Original file line number Diff line number Diff line change
Expand Up @@ -1701,7 +1701,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 {
Expand Down
7 changes: 4 additions & 3 deletions pkg/stack/unwind/compact_unwind_table.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}
Expand All @@ -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
Expand Down
31 changes: 27 additions & 4 deletions pkg/stack/unwind/compact_unwind_table_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -451,12 +455,31 @@ func requireNoRedundantRows(t *testing.T, ut CompactUnwindTable) {
}
}

var testPool *objectfile.Pool

func pool(t testing.TB) *objectfile.Pool {
var once sync.Once
once.Do(func() {
testPool = objectfile.NewPool(log.NewNopLogger(), prometheus.NewRegistry(), "", 100, 10)
t.Cleanup(func() {
testPool.Close()
})
})
return testPool
}

func objectFile(t testing.TB, path string) *objectfile.ObjectFile {
o, err := pool(t).Open(path)
require.NoError(t, 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)
}
Expand All @@ -468,7 +491,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)
}
Expand All @@ -479,7 +502,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)
}
Expand All @@ -495,7 +518,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)
Expand Down
11 changes: 5 additions & 6 deletions pkg/stack/unwind/unwind_table.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/stack/unwind/unwind_table_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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")
}
Expand Down

0 comments on commit 2fdae14

Please sign in to comment.