Conversation
✅ Deploy Preview for rspack canceled.
|
📦 Binary Size-limit
🙈 Size remains the same at 48.09MB |
CodSpeed Performance ReportMerging #12048 will not alter performanceComparing Summary
|
4836b9e to
4ff5db8
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR adds Tracy profiler integration to rspack for enhanced memory profiling capabilities. The implementation enables Tracy profiling by setting the TRACY environment variable during build time, which activates the tracy-client feature and uses Tracy's ProfiledAllocator for allocation tracking.
Key changes:
- Added tracy-client (v0.18.2) as a workspace dependency with required features
- Implemented conditional feature flags and allocator switching logic between mimalloc, sftrace, and Tracy
- Updated build script to support Tracy-specific compilation and debug symbol extraction
Reviewed Changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| deny.toml | Added windows-core to skip list for external dependencies |
| crates/rspack_binding_api/src/lib.rs | Added Tracy client initialization with demangler registration |
| crates/rspack_binding_api/Cargo.toml | Added tracy-client feature flag and dependency with alphabetized formatting |
| crates/rspack_allocator/src/lib.rs | Added conditional global allocator for Tracy's ProfiledAllocator with System allocator |
| crates/rspack_allocator/Cargo.toml | Added tracy-client and tracy-client-sys dependencies with cargo-shear metadata |
| crates/node_binding/scripts/build.js | Added TRACY environment variable support and dsymutil call for debug symbol extraction |
| crates/node_binding/Cargo.toml | Added tracy-client feature flag |
| crates/node_binding/.gitignore | Added *.dSYM to ignore debug symbol directories |
| Cargo.toml | Added tracy-client workspace dependency and split-debuginfo profile setting |
| Cargo.lock | Added tracy-client ecosystem dependencies (loom, generator, nu-ansi-term, etc.) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| sftrace-setup = { workspace = true, optional = true } | ||
| sftrace-setup = { workspace = true, optional = true } | ||
| tracy-client = { workspace = true, optional = true } | ||
| tracy-client-sys = { version = "=0.26.0" } |
There was a problem hiding this comment.
The tracy-client-sys dependency should be marked as optional to avoid unnecessarily building it when the tracy-client feature is not enabled. This dependency is only needed when the tracy-client feature is active.
Suggestion:
tracy-client-sys = { version = "=0.26.0", optional = true }| tracy-client-sys = { version = "=0.26.0" } | |
| tracy-client-sys = { version = "=0.26.0", optional = true } |
| if(process.env.TRACY){ | ||
| // split debug symbols for tracy | ||
| spawnSync('dsymutil', [ | ||
| path.resolve(__dirname, "..", "rspack.darwin-arm64.node") | ||
| ], { | ||
| stdio: "inherit", | ||
| shell: true, | ||
| }) | ||
| } |
There was a problem hiding this comment.
The dsymutil command and hardcoded path rspack.darwin-arm64.node are macOS-specific. This code will fail on other platforms (Linux, Windows) when TRACY is set. Consider adding a platform check or making the artifact path dynamic based on the build target.
Suggestion:
if(process.env.TRACY && process.platform === 'darwin'){
// split debug symbols for tracy
const target = process.env.RUST_TARGET || 'darwin-arm64';
const artifactPath = path.resolve(__dirname, "..", `rspack.${target}.node`);
spawnSync('dsymutil', [artifactPath], {
stdio: "inherit",
shell: true,
})
}| renameSync("rspack.wasm32-wasi.wasm", "rspack.browser.wasm") | ||
| } | ||
|
|
||
| if(process.env.TRACY){ |
There was a problem hiding this comment.
Missing space after if keyword. JavaScript style conventions typically require a space between if and the opening parenthesis.
Suggestion:
if (process.env.TRACY) {| if(process.env.TRACY){ | |
| if (process.env.TRACY){ |
Summary
add tracy profiler support for better memory profiling
Related links
Checklist