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

[Proposal] Support event tracing for profiling #3071

Closed
kwonoj opened this issue Dec 19, 2021 Discussed in #3065 · 10 comments
Closed

[Proposal] Support event tracing for profiling #3071

kwonoj opened this issue Dec 19, 2021 Discussed in #3065 · 10 comments

Comments

@kwonoj
Copy link
Member

kwonoj commented Dec 19, 2021

Discussed in #3065

Originally posted by kwonoj December 18, 2021

Status Last Updated Target version
Accepted Dec.19.2021 TBD

Summary

Expose a new flag, --trace or similar to emit profiling trace for SWC's internal behavior as trace event format.

Motivation

Performance is one of the notable characteristics of SWC. While SWC itself needs to deliver consistent peak performance, there will be problems from end user's codebases. By providing trace capabilities from SWC's core itself user can have detailed diagnostics to their own problem, also it can be used for the tracing for observability.

Slack for example heavily utilizes chrome's tracing feature for internal perf analysis (https://slack.engineering/chrome-tracing-for-fun-and-profit/) for their desktop client to solve real world user performance issue cannot be reproducible internally.

Explainer

Trace Event format is JSON representation of trace event data can be processed via various tools, including trace viewer. Chrome uses it for its trace profiling (https://www.chromium.org/developers/how-tos/trace-event-profiling-tool) as well as other tools like Bazel (https://docs.bazel.build/versions/main/skylark/performance.html). Chrome has an embedded trace viewer for the visualization (chrome://tracing/).

Detailed design

Thankfully, there are rust ecosystem allows us to adapt this easily. Moreover, SWC already have tracing (https://github.com/tokio-rs/tracing) internally. There is a layer tracing_chrome (https://github.com/thoren-d/tracing-chrome) on top of tracing subscriber which generates Event Trace Format JSON output from existing traces.

This PR (#3066) have quick & dirty poc demo which enables chrome traces with few dummy spans for transform_sync. Please refer generated JSON for the output format which can be visualized as below.

Screenshot from 2021-12-18 01-30-39

As a consumer's point of view, I expect these kinds of pseudo workflow to generate traces for a given operation.

// run @swc/cli
npx swc ./file.js ...options --trace categories1, categories2
// run @swc/core
import * as swc from '@swc/core';

swc.transformSync(code, {
  ...options,
  diagnostics: {
    enableTrace: ['*']
  }
});
  • Each trace request applies to a single operation. There will not be a global config / or function to override every operation in the scope.
  • Trace will offer categories: this indicates which part of trace will be recorded. For example, there can be categories like ecma_parser / ecma_transforms. As illustrated in second example, it is possible to apply some wildcard behavior. This mimics chromium's behavior for its categoryFilter
  • Trace does not offer way to customize its output including its path, by default it'll create random-name file under cwd.

Drawbacks

  • There'll be notable amount of effort to rewrite some of the traces as well as adding new one

Alternatives

Not much. Do not think it is worth to invent something new for swc only especially we have some of infrastructures in our codes.

Unresolved questions

There are multiple open questions / tasks that need to be resolved to make this happen. Notably,

  • Should release build offer tracing capabilities
  • Performance impact if release build supports trace
  • Adjust, write, update span across all codebase to create correct traces
  • Managing trace flushing
  • CategoryFilter support: first version may not support this, instead behaves as simple toggle
@kwonoj
Copy link
Member Author

kwonoj commented Dec 19, 2021

A few tasks we can think of for the first implementation

  • Expose new flag to enable traces. Will accept ['*'] as value only for now and it'll be boolean flag.
  • Install chrome_layer into tracing subscriber with disabling default subscriber config
  • Inject top-level span for each entrypoint (i.e transform)
  • Flush strategy for tracing subscriber

Once this works properly, we can start auditing each individual creates to have correct span & traces.

@RiESAEX
Copy link
Contributor

RiESAEX commented Feb 20, 2022

I looked into it a bit,
since tracing::span!() want a const value as it name, and most of visitors are chained by AndThen and Optional,
looks like we need to generate a visitor to wrap each AndThen/Optional we want to trace.

@kwonoj
Copy link
Member Author

kwonoj commented Feb 20, 2022

This is actually my planned work item, I'll dig a bit and come up with implementation proposal. Once we have reference implementation, we can make changes gradually as same as visit_mut refactoring.

@kwonoj
Copy link
Member Author

kwonoj commented Feb 24, 2022

@RiESAEX I choose to go with out of the box #instrument attr approach to mark most of overridden visitors. If we need custom spans, that can be dealt with separately either via span! or create span manually. #3731 illustrates it.

@RiESAEX
Copy link
Contributor

RiESAEX commented Feb 25, 2022

just an idea
May be we can create a macro to reduce copy&paste of #[tracing::instrument(level = "trace", skip_all)]
like

#[trace] // it generate #[tracing::instrument(level = "trace", skip_all)] for every implemented visit_mut_*
impl VisitMut for Arrow {}

@kdy1
Copy link
Member

kdy1 commented Feb 25, 2022

@RiESAEX I think it's good to have, and it can be language-agnostic if we only need to add the #[instrument] only for existing methods

@kdy1
Copy link
Member

kdy1 commented Feb 25, 2022

Done with #3738

@Nathamuni
Copy link

ORUVELE IRUKUMO...

@kwonoj
Copy link
Member Author

kwonoj commented Jul 24, 2023

This is reasonably achieved for the transform interface. minify / others are different but have other tracking issue.

@kwonoj kwonoj closed this as completed Jul 24, 2023
@swc-bot
Copy link
Collaborator

swc-bot commented Aug 24, 2023

This closed issue has been automatically locked because it had no new activity for a month. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@swc-project swc-project locked as resolved and limited conversation to collaborators Aug 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

No branches or pull requests

5 participants