feat: speedup ebpf uprobe attaching#440
Conversation
Greptile SummaryThis PR speeds up memtrack allocator probe attachment. The main changes are:
Confidence Score: 4/5This is close, but the offset attach path should be fixed before merging.
crates/memtrack/src/ebpf/memtrack.rs Important Files Changed
Prompt To Fix All With AIFix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
crates/memtrack/src/ebpf/memtrack.rs:120-122
**Offset Misses Still Skip**
When `resolve_symbol_offsets` cannot produce a section-backed file offset for one allocator symbol, this branch returns before trying any symbol-name attach path. A discovered allocator can then have valid exported allocation entry points left uninstrumented with no retry and no visible error, so allocations through those symbols are silently missed. This path needs to fall back to libbpf's symbol attach behavior or make the missing required probe fail loudly instead of treating the allocator as attached.
Reviews (2): Last reviewed commit: "bench(memtrack): add offset resolution b..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| let offsets = names | ||
| .iter() | ||
| .filter_map(|name| symbol_offsets.get(name).copied()) |
There was a problem hiding this comment.
This collects only names that resolve_symbol_offsets can map to a section file offset. If one allocator symbol is a valid attach target but has no section-backed offset while another name in the same group does, the multi attach succeeds for the subset and the omitted symbol is never retried by the per-symbol fallback, so allocations through that symbol are silently missed.
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/memtrack/src/ebpf/memtrack.rs
Line: 522-524
Comment:
**Offset Filtering Drops Symbols**
This collects only names that `resolve_symbol_offsets` can map to a section file offset. If one allocator symbol is a valid attach target but has no section-backed offset while another name in the same group does, the multi attach succeeds for the subset and the omitted symbol is never retried by the per-symbol fallback, so allocations through that symbol are silently missed.
How can I resolve this? If you propose a fix, please make it concise.
| { | ||
| Ok(output) => output, | ||
| Err(error) if error.kind() == ErrorKind::NotFound => { | ||
| eprintln!("skipping non-PIE symbol offset test: gcc is not installed"); |
There was a problem hiding this comment.
The custom Rust rule forbids println! and eprintln! in favor of tracing macros. This new test skip diagnostic uses eprintln!, so the changed test code can fail a policy gate that enforces that rule.
Rule Used: Never use println!/eprintln! in Rust; use trac... (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/memtrack/src/ebpf/memtrack.rs
Line: 873
Comment:
**Test Diagnostics Use Eprintln**
The custom Rust rule forbids `println!` and `eprintln!` in favor of tracing macros. This new test skip diagnostic uses `eprintln!`, so the changed test code can fail a policy gate that enforces that rule.
**Rule Used:** Never use `println!`/`eprintln!` in Rust; use trac... ([source](https://app.greptile.com/codspeed/-/custom-context?memory=f04f734b-e556-482b-98b9-64c7ce663663))
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Merging this PR will not alter performance
|
…f func_name Resolve every defined symbol's libbpf-compatible file offset once per library (st_value - sh_addr + sh_offset) and attach uprobes by offset, replacing the per-probe func_name lookup that reparsed the ELF each time.
Symbol resolution parses each allocator ELF independently, so fan it out across cores with rayon before the serial attach step. Cuts offset resolution for all discovered allocators from ~190ms to ~29ms locally.
Divan bench comparing serial vs parallel symbol offset resolution over the discovered allocator libraries. Runs without sudo since it only parses ELFs, no eBPF attach.
not-matthias
force-pushed
the
cod-3070-speedup-allocator-attaching-on-nixos
branch
from
0e8688a to
3a322ae
Compare
| let Some(&offset) = offsets.get(symbol) else { | ||
| return; | ||
| }; |
There was a problem hiding this comment.
When resolve_symbol_offsets cannot produce a section-backed file offset for one allocator symbol, this branch returns before trying any symbol-name attach path. A discovered allocator can then have valid exported allocation entry points left uninstrumented with no retry and no visible error, so allocations through those symbols are silently missed. This path needs to fall back to libbpf's symbol attach behavior or make the missing required probe fail loudly instead of treating the allocator as attached.
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/memtrack/src/ebpf/memtrack.rs
Line: 120-122
Comment:
**Offset Misses Still Skip**
When `resolve_symbol_offsets` cannot produce a section-backed file offset for one allocator symbol, this branch returns before trying any symbol-name attach path. A discovered allocator can then have valid exported allocation entry points left uninstrumented with no retry and no visible error, so allocations through those symbols are silently missed. This path needs to fall back to libbpf's symbol attach behavior or make the missing required probe fail loudly instead of treating the allocator as attached.
How can I resolve this? If you propose a fix, please make it concise.1 participant
No description provided.