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

feat(next-swc): introduce experimental tracing support for swc #35803

Merged
merged 7 commits into from May 2, 2022

Conversation

kwonoj
Copy link
Contributor

@kwonoj kwonoj commented Mar 31, 2022

Context: #34687 (comment)

It is a bit tricky to understand underlying performance characteristics inside swc when build steps involving swc's internals. Technically it should be possible to use a profiler with some custom swc build, but it is not easy job to set it up.

SWC recently introduced trace features for those reasons (swc-project/swc#3953), allow emitting Trace event format compatible (https://docs.google.com/document/d/1CvAClvFfyA5R-PhYUmn5OOQtYMH4h6I0nSsKchNAySU/preview) output. This PR ports over those features into next-swc under experimental config value to user to try out.

With this PR, trace will emit core swc's traces as well as one custom transformation in next-swc (emotion). Expect to expand trace over all existing custom transform passes eventually in further PRs.

To configure traces, next.config.js exposes this new experimental config value:

// next.config.js

{
...
  experimental: {
    swcTraceProfiling: true
  }
}

SWC will generate trace-${unixtimestamp}.json by default.

It is important to aware enabling trace all time is highly discouraged: observing code path is not free, and if build scales up trace will cost expensive time as well. User may want to use env variable, or other dynamic way to selectively enable trace instead.

Generated trace is compatible to trace event format, so existing visualizer can be used out of the box. Notably,

And possibly other flamegraph visualizer as well. Technically it is possible to make output to work with chrome devtools' profiler, but that's not supported now.

This is example trace screenshot from bench/nested-deps.

image

As config already implies, this is experimental due to 1. config / internal swc impl might change, 2. there are missing traces for custom passes in next-swc.

Bug

  • Related issues linked using fixes #number
  • Integration tests added
  • Errors have helpful link attached, see contributing.md

Feature

  • Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR.
  • Related issues linked using fixes #number
  • Integration tests added
  • Documentation added
  • Telemetry added. In case of a feature if it's used or not.
  • Errors have helpful link attached, see contributing.md

Documentation / Examples

  • Make sure the linting passes by running yarn lint

@ijjk

This comment has been minimized.

@ijjk

This comment has been minimized.

@ijjk

This comment has been minimized.

@ijjk

This comment has been minimized.

@ijjk

This comment has been minimized.

@ijjk

This comment has been minimized.

Brooooooklyn
Brooooooklyn previously approved these changes Apr 6, 2022
Copy link
Contributor

@Brooooooklyn Brooooooklyn left a comment

Choose a reason for hiding this comment

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

Great work!

@padmaia
Copy link
Member

padmaia commented Apr 6, 2022

Should we output the trace to the .next folder where the other trace goes? I think that file is just named trace so maybe swc-trace? Also, I think an environment variable would make sense once it's not experimental, but I would defer to @timneutkens for API decisions.

@kwonoj
Copy link
Contributor Author

kwonoj commented Apr 6, 2022

Should we output the trace to the .next folder where the other trace goes? I think that file is just named trace so maybe swc-trace

I do not have strong preferences for this - this is mostly modeling after others like node --perf-basic-prof which leaves v8 traces to current cwd. Totaly can change if we prefer to move it to elsewhere.

@ijjk

This comment has been minimized.

@kwonoj kwonoj force-pushed the feat-tracing branch 2 times, most recently from b8f9130 to 7560bd4 Compare April 20, 2022 17:22
@kwonoj
Copy link
Contributor Author

kwonoj commented Apr 20, 2022

Updated PR bit

  • rename swcTrace config to swcTraceProfiling, to avoid confusion between next/trace slightly
  • put trace to ./.next by default if filename not specified. Also, creates a path to dir if not exists.

@kwonoj kwonoj force-pushed the feat-tracing branch 2 times, most recently from dcff441 to 43c6655 Compare April 22, 2022 17:42
@kwonoj kwonoj force-pushed the feat-tracing branch 3 times, most recently from 05126e8 to b290b0c Compare April 28, 2022 04:26
timneutkens
timneutkens previously approved these changes Apr 28, 2022
Brooooooklyn
Brooooooklyn previously approved these changes Apr 28, 2022
@kwonoj kwonoj dismissed stale reviews from Brooooooklyn and timneutkens via 2331f8a April 28, 2022 19:30
@kwonoj kwonoj force-pushed the feat-tracing branch 2 times, most recently from 2331f8a to 5fde511 Compare April 28, 2022 19:55
@padmaia padmaia merged commit 837e0a6 into vercel:canary May 2, 2022
@kwonoj kwonoj deleted the feat-tracing branch May 5, 2022 07:04
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 4, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants