Skip to content

perf: disable napi tracing feature to reduce binary size#8702

Closed
Boshen wants to merge 1 commit into
mainfrom
perf/disable-napi-tracing-feature
Closed

perf: disable napi tracing feature to reduce binary size#8702
Boshen wants to merge 1 commit into
mainfrom
perf/disable-napi-tracing-feature

Conversation

@Boshen

@Boshen Boshen commented Mar 15, 2026

Copy link
Copy Markdown
Member

Summary

  • Remove the "tracing" feature from both napi and napi-derive workspace dependencies
  • This eliminates ~491 generated tracing::debug!() call sites in napi wrapper functions, saving ~99 KiB in the release binary
  • Rolldown's own tracing infrastructure (rolldown_tracing / RD_LOG) is unaffected

🤖 Generated with Claude Code

@Boshen @claude
perf: disable napi tracing feature to reduce binary size
009a79f 
Remove the `"tracing"` feature from both `napi` and `napi-derive` workspace
dependencies. This eliminates ~491 generated `tracing::debug!()` call sites
in napi wrapper functions, saving ~99 KiB in the release binary.

Rolldown has its own tracing infrastructure (`rolldown_tracing` / `RD_LOG`)
that is unaffected by this change.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings March 15, 2026 06:43
@netlify

netlify Bot commented Mar 15, 2026

Copy link
Copy Markdown

✅ Deploy Preview for rolldown-rs canceled.

Name Link
🔨 Latest commit 009a79f
🔍 Latest deploy log https://app.netlify.com/projects/rolldown-rs/deploys/69b6552110ea7f000842ee08

Copilot AI reviewed Mar 15, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the workspace’s N-API dependency configuration to drop the optional tracing feature from napi and napi-derive, and syncs Cargo.lock accordingly (reducing transitive dependencies brought in via N-API).

Changes:

  • Remove the tracing feature from the workspace napi dependency.
  • Remove the tracing feature from the workspace napi-derive dependency.
  • Update Cargo.lock to reflect the feature/dependency change.

Reviewed changes

Copilot reviewed 1 out of 2 changed files in this pull request and generated no comments.

File Description
Cargo.toml Drops tracing from napi / napi-derive feature sets in workspace deps.
Cargo.lock Removes the now-unneeded tracing dependency entry from the resolved lock graph.

You can also share your feedback on Copilot code review. Take the survey.

@Boshen Boshen requested a review from Brooooooklyn March 15, 2026 06:47
@github-actions

github-actions Bot commented Mar 15, 2026

Copy link
Copy Markdown
Contributor

Benchmarks Rust

  • target: main(df3188d)
  • pr: perf/disable-napi-tracing-feature(009a79f)
group                                                        pr                                     target
-----                                                        --                                     ------
bundle/bundle@multi-duplicated-top-level-symbol              1.00     62.9±1.37ms        ? ?/sec    1.02     64.5±1.36ms        ? ?/sec
bundle/bundle@multi-duplicated-top-level-symbol-sourcemap    1.00     71.2±1.59ms        ? ?/sec    1.02     72.7±1.84ms        ? ?/sec
bundle/bundle@rome_ts                                        1.00    136.7±4.55ms        ? ?/sec    1.03    141.0±5.79ms        ? ?/sec
bundle/bundle@rome_ts-sourcemap                              1.00    154.9±2.36ms        ? ?/sec    1.02    157.8±3.37ms        ? ?/sec
bundle/bundle@threejs                                        1.00     59.2±0.76ms        ? ?/sec    1.01     59.8±1.13ms        ? ?/sec
bundle/bundle@threejs-sourcemap                              1.00     69.5±1.65ms        ? ?/sec    1.01     69.9±1.81ms        ? ?/sec
bundle/bundle@threejs10x                                     1.00    699.0±6.22ms        ? ?/sec    1.00    697.1±5.93ms        ? ?/sec
bundle/bundle@threejs10x-sourcemap                           1.00    799.6±7.04ms        ? ?/sec    1.00    801.2±5.64ms        ? ?/sec

@codspeed-hq

codspeed-hq Bot commented Mar 15, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 6 untouched benchmarks
⏩ 8 skipped benchmarks1

Comparing perf/disable-napi-tracing-feature (009a79f) with main (f8be84a)2

Open in CodSpeed

Footnotes

  1. 8 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

  2. No successful run was found on main (df3188d) during the generation of this report, so f8be84a was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

@hyf0 hyf0 requested a review from sapphi-red March 15, 2026 07:28
@sapphi-red

sapphi-red commented Mar 15, 2026

Copy link
Copy Markdown
Member

Hmm, is it possible to reduce the size without completely removing this feature? If we do this, we'll have to create a build with this feature enabled and give it to users when these kinds of bug were found.

@Boshen

Boshen commented Mar 15, 2026

Copy link
Copy Markdown
Member Author

Hmm, is it possible to reduce the size without completely removing this feature? If we do this, we'll have to create a build with this feature enabled and give it to users when these kinds of bug were found.

What useful tracing information do the N-API crates provide?

@sapphi-red

sapphi-red commented Mar 15, 2026

Copy link
Copy Markdown
Member

It helps debugging deadlock issues like #7289. It will log for each napi function call and we can know which function is pending.

@Boshen

Boshen commented Mar 15, 2026

Copy link
Copy Markdown
Member Author

ahh ok

@Boshen Boshen closed this Mar 15, 2026
@Boshen Boshen deleted the perf/disable-napi-tracing-feature branch March 15, 2026 10:01
@sapphi-red sapphi-red mentioned this pull request Jun 15, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments
@Brooooooklyn Brooooooklyn Awaiting requested review from Brooooooklyn
@sapphi-red sapphi-red Awaiting requested review from sapphi-red

Assignees

@Boshen Boshen

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Boshen @sapphi-red