Skip to content

Conversation

@ulrichstark
Copy link
Contributor

@ulrichstark ulrichstark commented Sep 26, 2025

  • Avoid unnecessary Option wrapping and checks by splitting parse_normal_list function into a breakable and non-breakable variant
  • Avoid unnecessary Option wrapping and checks in parse_variable_declaration_for_statement function
  • Break up parse_break_or_continue_statement function into function for break and continue
  • Remove unnecessary checks in is_using_declaration and use cheaper lexer.peek_token()
  • Make parse_variable_declaration take declare: bool instead of &Modifiers avoid constructing empty Modifiers
  • some other minor code improvements

I hope these changes are not too much and therefore difficult to review. It's now less code with better performance. I was in the flow of refactoring and couldn't stop :)

Edit:

I don't see the improvements on Codspeed I'm seeing locally when running cargo benchmark --bench parser against main... Feel free to close my PR if there are no verifiable performance improvements. My local output:

image

Copilot AI review requested due to automatic review settings September 26, 2025 14:02
@github-actions github-actions bot added A-parser Area - Parser C-performance Category - Solution not expected to change functional behavior, only performance labels Sep 26, 2025
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 optimizes various parsing functions by reducing unnecessary Option wrapping, eliminating redundant checks, and splitting functions for better performance. The changes focus on streamlining control flow and leveraging more efficient parsing patterns.

  • Split parse_normal_list into breakable and non-breakable variants to avoid unnecessary Option handling
  • Refactored parse_variable_declaration to accept bool instead of &Modifiers for better performance
  • Separated break/continue statement parsing into dedicated functions and optimized various parsing helpers

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
crates/oxc_parser/src/ts/types.rs Simplified type literal parsing by removing Option wrapper
crates/oxc_parser/src/ts/statement.rs Updated interface body parsing and variable declaration calls
crates/oxc_parser/src/modifiers.rs Streamlined modifier construction and early return logic
crates/oxc_parser/src/js/statement.rs Major refactoring of statement parsing with function splitting and optimization
crates/oxc_parser/src/js/declaration.rs Simplified variable declaration parameter handling
crates/oxc_parser/src/js/class.rs Updated class body parsing to use new breakable variant
crates/oxc_parser/src/cursor.rs Added separate breakable/non-breakable list parsing functions
Comments suppressed due to low confidence (1)

crates/oxc_parser/src/js/statement.rs:1

  • This modifier verification logic has been moved here but appears to duplicate the verification that was previously done inside parse_variable_declaration. Consider whether this verification should be consolidated in one location to avoid potential inconsistencies.
use oxc_allocator::{Box, Vec};

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@codspeed-hq
Copy link

codspeed-hq bot commented Sep 26, 2025

CodSpeed Instrumentation Performance Report

Merging #14160 will not alter performance

Comparing ulrichstark:cleanup-and-optimize-various-parsing-functions (e3473b6) with main (c0e461f)1

Summary

✅ 37 untouched

Footnotes

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

@Boshen Boshen self-assigned this Sep 26, 2025
@Boshen
Copy link
Member

Boshen commented Sep 27, 2025

Thank you! Less code, more performant!

@Boshen Boshen merged commit af88e94 into oxc-project:main Sep 27, 2025
28 checks passed
@ulrichstark ulrichstark deleted the cleanup-and-optimize-various-parsing-functions branch September 28, 2025 02:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-parser Area - Parser 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.

2 participants