Skip to content

Conversation

@camc314
Copy link
Contributor

@camc314 camc314 commented Dec 26, 2025

Before:

Screenshot 2025-12-26 at 21 22 30

After:

Screenshot 2025-12-26 at 21 22 22

Before checking each declarator for potential inlining, collect all identifier names referenced in the target expression. This allows skipping declarators whose names are not present, avoiding expensive expression tree traversals.

This fixes a performance regression where files with many interspersed unused variable declarations (e.g., bundled modules with duplicate imports) would cause O(n*m) traversals where n is the number of statements and m is the number of declarators checked per statement.

Closes #17369

@github-actions github-actions bot added the A-minifier Area - Minifier label Dec 26, 2025
Copy link
Contributor Author

camc314 commented Dec 26, 2025


How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • 0-merge - adds this PR to the back of the merge queue
  • hotfix - for urgent hot fixes, skip the queue and merge this PR next

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

@github-actions github-actions bot added the C-performance Category - Solution not expected to change functional behavior, only performance label Dec 26, 2025
@codspeed-hq
Copy link

codspeed-hq bot commented Dec 26, 2025

CodSpeed Performance Report

Merging #17377 will degrade performance by 7.43%

Comparing c/12-26-perf_minifier_speed_up_single-use_symbol_substitution_by_collecting_referenced_identifiers_upfront (ca77d24) with main (d2abc78)

Summary

❌ 1 regression
✅ 37 untouched
⏩ 7 skipped1

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Mode Benchmark BASE HEAD Efficiency
Simulation minifier[cal.com.tsx] 36.6 ms 39.5 ms -7.43%

Footnotes

  1. 7 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.

@camc314 camc314 force-pushed the c/12-26-perf_minifier_speed_up_single-use_symbol_substitution_by_collecting_referenced_identifiers_upfront branch 3 times, most recently from 9b34a71 to b743666 Compare December 26, 2025 21:36
// in the target expression. This allows us to quickly skip declarators whose names
// aren't even present, avoiding expensive tree traversals.
// For small numbers of declarators, the overhead of creating the HashSet isn't worth it.
const IDENTIFIER_CACHE_THRESHOLD: usize = 4;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is a bit of a hack, but if we're minifying a small file e.g. the files in our benchmarks, creating the ident cache introduces a slowdown and has a negative 5+% perf hit. But if we've got a ton of idents like the one in the linked issue, it's worth it

… referenced identifiers upfront

Before checking each declarator for potential inlining, collect all
identifier names referenced in the target expression. This allows
skipping declarators whose names are not present, avoiding expensive
expression tree traversals.

This fixes a performance regression where files with many interspersed
unused variable declarations (e.g., bundled modules with duplicate
imports) would cause O(n*m) traversals where n is the number of
statements and m is the number of declarators checked per statement.

Closes #17369
@camc314 camc314 force-pushed the c/12-26-perf_minifier_speed_up_single-use_symbol_substitution_by_collecting_referenced_identifiers_upfront branch from b743666 to ca77d24 Compare December 26, 2025 21:42
@armano2
Copy link
Contributor

armano2 commented Dec 26, 2025

q: what do you think about changing symbol_value.read_references_count > 1 to symbol_value.read_references_count != 1 as if variable is not used at all, we do not have to do anything?

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

Labels

A-minifier Area - Minifier C-performance Category - Solution not expected to change functional behavior, only performance

Projects

None yet

Development

Successfully merging this pull request may close these issues.

minifier: Performance regression: 2.5x slower minification with minor input size increase

3 participants