Skip to content

perf(ebpf): cache uprobe symbol lookups by binary identity - #330

Merged
mayankpande88 merged 1 commit into
mainfrom
perf/cache-uprobe-symbol-lookups
Sep 10, 2026
Merged

perf(ebpf): cache uprobe symbol lookups by binary identity#330
mayankpande88 merged 1 commit into
mainfrom
perf/cache-uprobe-symbol-lookups

Conversation

@mayankpande88

Copy link
Copy Markdown
Contributor

Problem

Attaching the Node.js probes reads the full ELF symbol table on every pid.

node is a large statically linked binary with V8 embedded. readSymbols loads both .symtab and .dynsym — allocating a Go string per symbol name — to locate six libuv functions. Each found symbol is then disassembled to find its RET instructions for uretprobes.

None of that work is per-process. Address() and ReturnOffsets() are pure functions of (binary, symbol name). But nothing cached them: instrumentNodejs is gated on p.nodejsChecked, a per-Process field, so every new pid repeats the whole thing. Node cluster mode runs many workers off one binary; pod churn repeats it again.

A customer CPU profile showed the cost:

Process.instrumentNodejs → AttachNodejsProbes
  → ELFFile.GetSymbol → readSymbols        17.5% of a core
debug/elf.getSymbols64                     11.2%

The heap profile showed the matching allocation churn (saferio.ReadData 18 MB, debug/elf.getString).

The negative case is worse than the positive one. Any executable reaching this path that doesn't export uv__io_poll was re-parsed for every pid, forever, to reach the same conclusion.

Why this instrumentation exists at all

libuv's uv__io_poll and the uv__*_io callbacks are internal C functions with no stable ABI, so their addresses can't be hardcoded — the symbol table is the only way to find them. They produce container_nodejs_event_loop_blocked_time_seconds_total, which is the primary Node.js health signal and isn't derivable from CPU/memory/latency. The metric is worth having; paying for it once per pid is not.

Fix

LookupSymbols caches ProbeTarget{address, returnOffsets} keyed by file identitydev + inode + size + mtime — not by path.

Path-keying would miss every time: probe paths are per-process (/proc/<pid>/root/usr/bin/node), so every pid produces a different string for the same file. Pods from one image share the read-only overlay layer, so dev+inode collapses them onto a single entry.

Absent symbols are cached as Found=false, so a non-Node binary is parsed once and never again — this is what removes the dominant negative case.

Caching is per (binary, symbol), not one map per binary. Keying only by binary and storing whichever names the first caller asked for would hand a later caller that map and cause it to read its own symbols as absent. That's reachable, not theoretical — the Node.js and Go-TLS probes can target the same executable. My first draft had exactly this bug and a test caught it. A partial hit resolves the remaining names in a single parse.

Bounded at 4096 entries (LRU) so a node cycling image versions can't grow it without limit.

Behaviour

Unchanged, deliberately — including stopping at the first missing libuv callback rather than continuing. This is a pure performance change.

(Separately: that break looks wrong — different libuv builds export different subsets, so one absent callback silently skips the rest. Left alone here so this PR stays measurable; worth its own look.)

Testing

go build ./...          ok  (whole repo)
go vet ./ebpftracer/    clean
--- PASS: TestLookupSymbolsCachesByFileIdentity      (hardlink → same cache key)
--- PASS: TestLookupSymbolsCachesNegativeResults     (absent symbol cached, not an error)
--- PASS: TestLookupSymbolsUnreadableBinaryIsNotCached
--- PASS: TestBinaryKeyDistinguishesFiles

./ebpftracer is not in CI's test exclusion (only ./containers is), so these run in CI.

Not yet measured

The 17.5% figure is the before number from the customer profile. I have not measured the after — that needs a Node-heavy node, and our own clusters barely run Node. Worth capturing a /debug/pprof/profile on such a node post-deploy to confirm the win rather than assuming it.

tls.go and python.go take the same per-pid full-table path and would benefit from the same treatment; not touched here to keep this reviewable.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a symbol cache to optimize Node.js uprobe and uretprobe attachments by caching resolved symbol addresses and return offsets based on file identity rather than path, preventing expensive, repeated ELF parsing. The review feedback correctly points out that the global mutex symbolCacheMu is redundant because the underlying lru.Cache is already thread-safe, and suggests removing it to simplify the code and avoid unnecessary lock contention.

Comment thread ebpftracer/symbol_cache.go
Comment thread ebpftracer/symbol_cache.go
Comment thread ebpftracer/symbol_cache.go
Attaching the Node.js probes read the full ELF symbol table on every pid.
node is a large statically linked binary with V8 embedded, and readSymbols
loads both .symtab and .dynsym — allocating a Go string per symbol name —
to locate six libuv functions. Each found symbol is then disassembled to
find its RET instructions for uretprobes.

None of that is per-process. The address and return offsets are pure
functions of (binary, symbol name), but nothing cached them:
instrumentNodejs is gated on a per-Process field, so every new pid repeated
the whole thing. Node cluster mode runs many workers off one binary, and
pod churn repeats it again.

A customer CPU profile showed the cost:

  Process.instrumentNodejs -> AttachNodejsProbes
    -> ELFFile.GetSymbol -> readSymbols        17.5% of a core
  debug/elf.getSymbols64                       11.2%

The negative case is worse than the positive one. Any executable reaching
this path that does not export uv__io_poll was re-parsed for every pid,
forever, to reach the same conclusion.

LookupSymbols caches ProbeTarget{address, returnOffsets} keyed by file
identity — dev+inode+size+mtime — not by path. Paths here are per-process
(/proc/<pid>/root/...), so a path-keyed cache would miss every time, which
is precisely the case being fixed; pods from one image share the read-only
overlay layer, so dev+inode collapses them onto one entry. Absent symbols
are cached too, so a non-Node binary is parsed once and never again.

Caching is per (binary, symbol) rather than one map per binary. Keying only
by binary and storing whichever names the first caller asked for would hand
a later caller that map and read its own symbols as absent — reachable,
since the Node.js and Go-TLS probes can target the same executable. A
partial hit resolves the remaining names in a single parse.

Attach behaviour is unchanged, including stopping at the first missing
libuv callback. ./ebpftracer is not excluded from CI tests, so the new
cases gate this.
@mayankpande88
mayankpande88 force-pushed the perf/cache-uprobe-symbol-lookups branch from 7e3ed37 to 8ae6afb Compare September 10, 2026 13:12
@mayankpande88
mayankpande88 merged commit 49e5999 into main Sep 10, 2026
7 checks passed
@mayankpande88
mayankpande88 deleted the perf/cache-uprobe-symbol-lookups branch September 10, 2026 13:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants