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.

Sign up for GitHub

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

Jump to bottom

xctrace support #286

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open

xctrace support #286

wants to merge 16 commits into from

Conversation

ldm0
Copy link

@ldm0 ldm0 commented Mar 26, 2023 •
edited
Loading

Basically working. Required to be done:

Fixes #246

xctrace is a tracer used internally by Apple Instruments. It generates a .trace folder containing a bunch of files in an unknown binary format. People have reverse engineerd it but it's internal format changed after XCode 12.

There's a xctrace export command, which can convert .trace into a readable XML file. However, it's output can't be used to generate flamegraphs until XCode 14.3, which introduces VM address and symbol name to the XML output.

In this PR, I added a stack collapser for XML files generated by xctrace export, making it possible to convert Instruments or xctrace‘s output into flamegraph.

@ldm0 ldm0 marked this pull request as draft March 26, 2023 17:59
jonhoo
jonhoo reviewed Mar 27, 2023
View reviewed changes
Cargo.toml Outdated Show resolved Hide resolved
@ldm0 ldm0 force-pushed the ldm_xctrace_support branch 4 times, most recently from 8988028 to 4fdfd61 Compare April 2, 2023 09:21
@codecov
Copy link

codecov bot commented Apr 8, 2023 •
edited
Loading

Codecov Report

Attention: Patch coverage is 83.59375% with 42 lines in your changes missing coverage. Please review.

Project coverage is 90.95%. Comparing base (8e691e6) to head (c54a267).

Files with missing lines Patch % Lines
src/collapse/xctrace.rs 83.39% 42 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main     #286      +/-   ##
==========================================
- Coverage   91.38%   90.95%   -0.43%     
==========================================
  Files          20       21       +1     
  Lines        4444     4699     +255     
==========================================
+ Hits         4061     4274     +213     
- Misses        383      425      +42

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ldm0 ldm0 force-pushed the ldm_xctrace_support branch from 4fdfd61 to 959928b Compare April 11, 2023 04:56
@kraktus
Copy link

kraktus commented Apr 16, 2023

Hey, I was going to start working on integrating this only to see the work was already done! Thanks @ldm0!

I can help add the documentation if needed. Also not sure if this feature need to be tied to the quick-xml update?

@ldm0
Copy link
Author

ldm0 commented Apr 16, 2023 •
edited
Loading

I can help add the documentation if needed. Also not sure if this feature need to be tied to the quick-xml update?

Thanks! I've written some documentation already, will submit it tomorrow. I think introducing multiple versions of the same dependency is bad(in terms of artifact size and complexity), so I'm waiting for the quick-xml update.

BTW, I've forked flamegraph with this inferno branch and you can try it: cargo install --git https://github.com/ldm0/flamegraph --branch xctrace, I replaced the default tracer(dtrace) with xctrace on macOS. (Make sure you get an XCode>=14.3 though)

@ldm0 ldm0 force-pushed the ldm_xctrace_support branch from 959928b to 49ff175 Compare September 17, 2023 17:02
@ldm0 ldm0 force-pushed the ldm_xctrace_support branch 4 times, most recently from 41bad5a to fa5ffda Compare November 26, 2024 11:50
@ldm0 ldm0 marked this pull request as ready for review November 26, 2024 11:53
@ldm0 ldm0 force-pushed the ldm_xctrace_support branch from fa5ffda to fef528b Compare November 26, 2024 13:19
@ldm0
Copy link
Author

ldm0 commented Nov 27, 2024

@jonhoo

Hi, now that #332 has been merged, this PR is ready for review.
I've personally used it for quite a long time and it has worked well. Please take a look.

@kraktus
Copy link

kraktus commented Dec 4, 2024

Same, I've been using it locally without issue

jonhoo
jonhoo requested changes Dec 31, 2024
View reviewed changes
Copy link
Owner

@jonhoo jonhoo left a comment

Choose a reason for hiding this comment

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

That's exciting, thanks! Took a first pass now. Nothing too deep to change, though some stuff that'll make this easier to maintain longer-term :)

src/collapse/guess.rs Outdated Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
src/collapse/xctrace.rs Outdated Show resolved Hide resolved
src/collapse/xctrace.rs Outdated Show resolved Hide resolved
src/collapse/xctrace.rs Show resolved Hide resolved
src/collapse/xctrace.rs Outdated Show resolved Hide resolved
src/collapse/xctrace.rs Show resolved Hide resolved
src/collapse/xctrace.rs Outdated Show resolved Hide resolved
src/collapse/xctrace.rs Outdated Show resolved Hide resolved
src/collapse/xctrace.rs Outdated Show resolved Hide resolved
jonhoo
jonhoo reviewed Jan 5, 2025
View reviewed changes
src/collapse/xctrace.rs Outdated Show resolved Hide resolved
@ldm0 ldm0 force-pushed the ldm_xctrace_support branch from 3af45ef to dde90c0 Compare January 5, 2025 11:23
kraktus
kraktus reviewed Jan 5, 2025
View reviewed changes
src/collapse/xctrace.rs Outdated Show resolved Hide resolved
jonhoo
jonhoo requested changes Jan 5, 2025
View reviewed changes
Copy link
Owner

@jonhoo jonhoo left a comment

Choose a reason for hiding this comment

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

Nice, it's already looking a lot better, thank you!

src/collapse/xctrace.rs Outdated Show resolved Hide resolved
src/collapse/xctrace.rs Outdated Show resolved Hide resolved
src/collapse/xctrace.rs Outdated Show resolved Hide resolved
@ldm0 ldm0 requested a review from jonhoo January 10, 2025 08:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kraktus kraktus kraktus left review comments

@jonhoo jonhoo Awaiting requested review from jonhoo

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Use xctrace on macOS
3 participants
@ldm0 @kraktus @jonhoo