Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add USDT tracepoints for key GC activities #883

Merged
merged 1 commit into from
Aug 3, 2023
Merged

Conversation

caizixian
Copy link
Member

@caizixian caizixian commented Aug 3, 2023

This work is described in Improving Garbage Collection Observability with Performance Tracing by Claire Huang, Stephen M. Blackburn, and Zixian Cai, to appear in MPLR'23.

These tracepoints are compiled into nops with minimal overhead when not attached as measured in the paper. The tracing tools that make use of these tracepoints will be released separately in another PR.

@caizixian caizixian requested a review from wks August 3, 2023 01:50
@caizixian caizixian marked this pull request as draft August 3, 2023 01:51
@caizixian caizixian marked this pull request as ready for review August 3, 2023 02:03
@caizixian caizixian added G-performance Goal: Performance C-feature Category: Feature A-utils Area: Utilities A-benchmarking Area: Benchmarking MMTk labels Aug 3, 2023
@caizixian
Copy link
Member Author

The clippy warnings on macOS seems to be because the probe! macro compiles to an empty block on macOS. I think this is fine.

@qinsoon
Copy link
Member

qinsoon commented Aug 3, 2023

The clippy warnings on macOS seems to be because the probe! macro compiles to an empty block on macOS. I think this is fine.

Yeah. Just suppress the warning.

src/plan/global.rs Outdated Show resolved Hide resolved
src/scheduler/worker.rs Show resolved Hide resolved
src/util/alloc/allocator.rs Show resolved Hide resolved
@k-sareen
Copy link
Collaborator

k-sareen commented Aug 3, 2023

Did you have the performance overheads of the extra indirection via _trace functions? I presume it's none since the compiler would inline it

@caizixian
Copy link
Member Author

Did you have the performance overheads of the extra indirection via _trace functions? I presume it's none since the compiler would inline it

I believe so.

@caizixian caizixian requested a review from qinsoon August 3, 2023 05:16
@wks
Copy link
Collaborator

wks commented Aug 3, 2023

Can you also include an example bpftrace script somewhere in the mmtk-core repo? For example, in mmtk-core/examples/bpftrace/gcvis.bt? It would be more helpful if we also include a script that can convert its result into a trace format that can be opened by perfetto or other well-known tools.

@caizixian
Copy link
Member Author

caizixian commented Aug 3, 2023

Can you also include an example bpftrace script somewhere in the mmtk-core repo? For example, in mmtk-core/examples/bpftrace/gcvis.bt? It would be more helpful if we also include a script that can convert its result into a trace format that can be opened by perfetto or other well-known tools.

I plan to place the tracing scripts under tool/ following the convention of bpftrace and bcc. I want to this in a separate PR, because I want to clean up the scripts to not have hardcoded paths etc.

I want to get the tracepoints merged first, so that people wanting to use this don't need to patch their mmtk-core.

@wks
Copy link
Collaborator

wks commented Aug 3, 2023

Can we also include USDT trace points for the following functions in mmtk-core?

  • MMTK::harness_begin
  • MMTK::harness_end
  • GCController::run
  • GCWorker::run

Note that we have a long-term goal of removing the memory_manager module (see #658) in favour for a more Rust-style API organised in modules, structs, traits and methods, etc. So it is preferrable not to add probes in memory_manager.

We usually need to initialise some states in the bpftrace trace. Since mmtk-core doesn't provide a official C API (the bindings may create extern "C" functions if they need to), we can't directly use uprobe or uretprobe for those points. We can simply add USDT in those functions so that we can trace those points in a VM-agnostic way.

Copy link
Collaborator

@wks wks left a comment

Choose a reason for hiding this comment

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

I think this PR looks good in general. The only missing pieces that I think we must include in this PR are the USDT trace points in the beginning of GC threads and harness_{begin,end}, without which it would be incomplete.

I am OK with postponing the splitting of process_edges trace point to another PR, because the current method is known to work.

I have another general suggestion, but doesn't have to be included in this PR, either. We may let GCWork implement a very general fn size(&self) -> Option<usize> method so that when we do eBPF tracing, we can add a size argument to every work packet if it makes sense. I think it makes sense for quite many packet types, including ProcessEdgesWork, ScanObjects, Process{,Region}ModBuf as well as weak reference and finalizer processing. VM bindings can also use that to describe the size of their work packets, too, if that makes sense. Examples include the number of "potential pinning parents" a Ruby VM has, the number of elements in weak global tables, such as "string tables" in OpenJDK and fstring_table in Ruby, etc.

Copy link
Collaborator

@wks wks left a comment

Choose a reason for hiding this comment

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

LGTM

@caizixian caizixian merged commit cc5a998 into mmtk:master Aug 3, 2023
13 of 14 checks passed
@caizixian caizixian deleted the ebpf branch August 3, 2023 12:06
caizixian added a commit to caizixian/mmtk-core that referenced this pull request Aug 11, 2023
These tools utilize the tracepoints added in mmtk#883

Co-authored-by: Claire Huang <[email protected]>
caizixian added a commit that referenced this pull request Aug 14, 2023
These tools utilize the tracepoints added in #883. The GC visualization
tools requires some post-processing and is just a bit more complicated
in general. That will be added in a separate PR.

---------

Co-authored-by: Claire Huang <[email protected]>
wenyuzhao pushed a commit to wenyuzhao/mmtk-core that referenced this pull request Aug 17, 2023
These tools utilize the tracepoints added in mmtk#883. The GC visualization
tools requires some post-processing and is just a bit more complicated
in general. That will be added in a separate PR.

---------

Co-authored-by: Claire Huang <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-benchmarking Area: Benchmarking MMTk A-utils Area: Utilities C-feature Category: Feature G-performance Goal: Performance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants