Skip to content

Conversation

@Boshen
Copy link
Member

@Boshen Boshen commented Aug 11, 2025

No description provided.

@graphite-app
Copy link
Contributor

graphite-app bot commented Aug 11, 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.

@github-actions github-actions bot added the A-minifier Area - Minifier label Aug 11, 2025
@Boshen Boshen force-pushed the 08-11-refactor_minfier_single_match_expression branch from a828c92 to 1ecabde Compare August 11, 2025 16:58
@Boshen Boshen changed the title 08 11 refactor minfier single match expression refactor(minfier): single match expression Aug 11, 2025
@github-actions github-actions bot added the C-cleanup Category - technical debt or refactoring. Solution not expected to change behavior label Aug 11, 2025
@codspeed-hq
Copy link

codspeed-hq bot commented Aug 11, 2025

CodSpeed Instrumentation Performance Report

Merging #13000 will improve performances by 13.97%

Comparing 08-11-refactor_minfier_single_match_expression (ca91a26) with main (2bf9347)

Summary

⚡ 4 improvements
✅ 30 untouched benchmarks

Benchmarks breakdown

Benchmark BASE HEAD Change
minifier[RadixUIAdoptionSection.jsx] 74.1 µs 65.2 µs +13.77%
minifier[binder.ts] 4.6 ms 4.2 ms +10.07%
minifier[cal.com.tsx] 37.3 ms 32.7 ms +13.97%
minifier[react.development.js] 2.6 ms 2.3 ms +11.54%

@Boshen Boshen marked this pull request as ready for review August 11, 2025 17:05
@Boshen Boshen requested review from Copilot and sapphi-red August 11, 2025 17:05
Copy link
Contributor

Copilot AI left a comment

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 refactors the peephole optimizer's exit expression handling by eliminating individual exit expression methods in favor of a single consolidated match expression. The changes move from separate method calls per optimization type to a unified approach where all optimizations are applied within one comprehensive match statement.

Key changes:

  • Removes individual *_exit_expression methods and consolidates logic into main exit_expression match
  • Makes previously private optimization methods public for direct usage
  • Renames some methods for consistency (e.g., fold_call_expression instead of remove_call_expression)

Reviewed Changes

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

Show a summary per file
File Description
substitute_alternate_syntax.rs Removes substitute_exit_expression method and makes optimization methods public
replace_known_methods.rs Removes replace_known_methods_exit_expression and makes methods public
remove_unused_expression.rs Renames fold_call_expression to remove_unused_call_expression
remove_dead_code.rs Removes remove_dead_code_exit_expression and makes methods public
mod.rs Consolidates all optimization calls into single match expression in exit_expression
minimize_statements.rs Updates method calls to use new consolidated approach
minimize_conditions.rs Removes minimize_conditions_exit_expression and makes methods public
fold_constants.rs Removes fold_constants_exit_expression and makes methods public

@Boshen
Copy link
Member Author

Boshen commented Aug 11, 2025

Oh wow look at that perf!

@sapphi-red I believe this makes things easier to debug, but harder to get the function call ordering right.

I'll rewrite the method names and a documentation to make sure the orderings are consistent, in a follow up PR.

@sapphi-red
Copy link
Member

Wow!

@Boshen Boshen added the 0-merge Merge with Graphite Merge Queue label Aug 11, 2025
Copy link
Member Author

Boshen commented Aug 11, 2025

Merge activity

@graphite-app graphite-app bot force-pushed the 08-11-refactor_minfier_single_match_expression branch from 1ecabde to ca91a26 Compare August 11, 2025 17:24
@graphite-app graphite-app bot merged commit ca91a26 into main Aug 11, 2025
30 checks passed
@graphite-app graphite-app bot deleted the 08-11-refactor_minfier_single_match_expression branch August 11, 2025 17:30
@graphite-app graphite-app bot removed the 0-merge Merge with Graphite Merge Queue label Aug 11, 2025
taearls pushed a commit to taearls/oxc that referenced this pull request Aug 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-minifier Area - Minifier C-cleanup Category - technical debt or refactoring. Solution not expected to change behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants