Skip to content

Conversation

@tiye
Copy link
Member

@tiye tiye commented May 10, 2025

No description provided.

@tiye tiye requested review from a team and Copilot May 10, 2025 15:47
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 introduces performance improvements and refactoring in the parsing logic by reusing allocated buffers and preallocating vector capacities.

  • Introduces DEFAULT_EXPR_CAPACITY and DEFAULT_BUFFER_CAPACITY constants for preallocation.
  • Replaces several reallocation patterns with buffer clearing for improved string reuse.
  • Updates Cargo.toml to bump the version to 0.1.32 and adds VS Code Rust Analyzer settings.

Reviewed Changes

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

File Description
src/parser.rs Refactors buffer initialization and vector allocation for parsing performance improvements.
Cargo.toml Updates the package version to 0.1.32 and enables the 2024 edition.
.vscode/settings.json Adds configuration for Rust Analyzer linked to Cargo.toml.

_ => {
state = CirruLexState::Token;
buffer = String::from(c)
buffer.clear();
Copy link

Copilot AI May 10, 2025

Choose a reason for hiding this comment

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

[nitpick] Consider standardizing the buffer reinitialization technique across the lexing states. Either consistently use clear() on an already allocated buffer or reallocate with the preset capacity to improve code readability.

Copilot uses AI. Check for mistakes.
}

acc.insert(0, CirruLexItem::Open);
// More efficient way to wrap acc
Copy link

Copilot AI May 10, 2025

Choose a reason for hiding this comment

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

Verify that the preallocation size calculation (1 + acc.len() + level + 1) consistently covers all edge cases. The new vector wrapping approach is efficient, but a brief comment noting the rationale could enhance maintainability.

Suggested change
// More efficient way to wrap acc
// More efficient way to wrap acc
// Preallocate capacity for the new vector:
// - `1` for the initial `Open` item.
// - `acc.len()` for the existing items in `acc`.
// - `level` (converted to `usize`) for the additional `Close` items corresponding to the current indentation level.
// - `1` for the final `Close` item.

Copilot uses AI. Check for mistakes.
@NoEgAm NoEgAm merged commit a9d27a4 into main May 28, 2025
1 check passed
@NoEgAm NoEgAm deleted the perf branch May 28, 2025 07:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants