Skip to content

perf(minify): skip renaming symbols with zero use count#26234

Closed
robobun wants to merge 1 commit intomainfrom
claude/skip-renaming-unused-symbols
Closed

perf(minify): skip renaming symbols with zero use count#26234
robobun wants to merge 1 commit intomainfrom
claude/skip-renaming-unused-symbols

Conversation

@robobun
Copy link
Collaborator

@robobun robobun commented Jan 19, 2026

Summary

  • Filter out zero-count slots before sorting in MinifyRenamer, avoiding unnecessary sorting and iteration

These slots represent declared symbols in nested scopes that were tree-shaken away. By filtering them out before sorting, we avoid unnecessary name generation work.

Ported from oxc: oxc-project/oxc#18183

Test plan

  • Added minify/SkipUnusedSymbolSlots test that verifies the optimization works when nested scopes are tree-shaken
  • Existing bundler minify tests pass

🤖 Generated with Claude Code

@robobun
Copy link
Collaborator Author

robobun commented Jan 19, 2026

Updated 11:23 PM PT - Jan 18th, 2026

❌ Your commit ab9ba5be has 2 failures in Build #35276 (All Failures):


🧪   To try this PR locally:

bunx bun-pr 26234

That installs a local version of the PR into your bun-26234 executable, so you can run:

bun-26234 --bun

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 19, 2026

Walkthrough

Frequency-based renaming in src/renamer.zig now builds candidates by iterating slots and skipping symbols with zero use count; a regression test was added to ensure unused symbol slots are not assigned short minified names.

Changes

Cohort / File(s) Summary
Renaming Logic Updates
src/renamer.zig
MinifyRenamer.assignNamesByFrequency: build the candidate list by iterating each namespace slot and appending only slots with non-zero use counts, then sort by count; removed prior pre-population of zero-count entries.
Tests
test/bundler/bundler_minify.test.ts
Added regression test minify/SkipUnusedSymbolSlots to verify the minifier skips pre-allocated zero-use symbol slots and assigns short names only to used symbols.

Suggested reviewers

  • pfgithub
🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main performance optimization: skipping renaming of symbols with zero use count, which directly aligns with the core change in MinifyRenamer.assignNamesByFrequency.
Description check ✅ Passed The description covers both required sections: explains what the PR does (filtering zero-count slots) and provides a test plan (added regression test and existing tests pass). It includes helpful context about the purpose and oxc source.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

@robobun robobun force-pushed the claude/skip-renaming-unused-symbols branch 2 times, most recently from 6907666 to ef29867 Compare January 19, 2026 05:53
In MinifyRenamer, avoid generating names for slots where the symbol
use count is zero. These represent declared symbols that are never
actually used (e.g., tree-shaken away or declared but not referenced).

We filter out zero-count slots before adding to the sorted array,
avoiding unnecessary sorting and iteration overhead. This is
particularly beneficial when nested scopes are tree-shaken, as their
pre-allocated symbol slots would otherwise still be processed.

Ported from oxc: oxc-project/oxc#18183

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@robobun robobun force-pushed the claude/skip-renaming-unused-symbols branch from ef29867 to ab9ba5b Compare January 19, 2026 06:43
@Jarred-Sumner
Copy link
Collaborator

Could not find a measurable performance difference.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants