-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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 support for logging events and intervals in the macOS profiler #4636
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
10 Ignored Deployments
|
4788865
to
984040e
Compare
✅ This change can build |
🟢 CI successful 🟢Thanks |
984040e
to
d2324d6
Compare
crates/utils/signposter/src/lib.rs
Outdated
/// Escapes a message for use with signposts, which will try to parse it as a | ||
/// format string. | ||
fn escape_message(message: &str) -> String { | ||
message.replace('%', "%%") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be Cow
with check for existence of %
crates/utils/signposter/src/lib.rs
Outdated
impl Log { | ||
/// Create a new log with the given subsystem and category. | ||
pub fn new_with_subsystem_and_category(subsystem: &str, category: LogCategory) -> Self { | ||
let subsystem = CString::new(subsystem).unwrap(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here, the content of subsystem
is not dropped while the function returns
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you explain what you mean here?
Is it possible use the https://crates.io/crates/tracing and make signposts a provider for that? |
@sokra Yes. Right now this implementation is very minimal as the goal was to get it working first and foremost to get some insights fast, but eventually we definitely should integrate it with tracing. |
https://github.com/vercel/turbo/actions/runs/4745099662/jobs/8426941687?pr=4636 I need to make the whole crate a noop for anything else than macOS. |
d2324d6
to
291c3ce
Compare
56b7bc0
to
338b1c2
Compare
338b1c2
to
dd68a02
Compare
I've added a tracing layer which is now used in turbopack-dev-server instead of using Be aware that tracing is currently force-disabled in |
Yes, it's possible and it can be done using |
See vercel/turbo#4866 This also updates Turbopack to turbopack-230511.2 with the following changes: * vercel/turbo#4636 <!-- Alex Kirszenberg - Add support for logging events and intervals in the macOS profiler --> * vercel/turbo#4793 <!-- OJ Kwon - ci(workflow): enable more test --> * vercel/turbo#4886 <!-- OJ Kwon - refactor(ecmascript-plugins): update serverdirective signature --> * vercel/turbo#4866 <!-- Alex Kirszenberg - Rename Turbopack/tasks crates to common prefixes -->
Description
Surprisingly, I couldn't find an adequate out-of-the-box crate for adding signposts to the macOS profiler. I had to dig in to semi-private macOS APIs to add this capability.
Here's what signposts look like in the profiler view:
Testing Instructions
N/A
link WEB-931