Skip to content

JS: Use StringBuilder when building up type name in JSDoc#19069

Merged
asgerf merged 1 commit intogithub:mainfrom
asgerf:js/jsdoc-parser
Mar 20, 2025
Merged

JS: Use StringBuilder when building up type name in JSDoc#19069
asgerf merged 1 commit intogithub:mainfrom
asgerf:js/jsdoc-parser

Conversation

@asgerf
Copy link
Copy Markdown
Contributor

@asgerf asgerf commented Mar 20, 2025

This code was a bit of a performance cringe. It copied every character into a temporary array, copied that into a String, and slow-appended that onto another String.

Note that the call to Characters.toChars is redundant here as advance() doesn't return a code point; it returns -1 or a UTF-16 char. The -1 case is checked for before reaching the call, so we can just cast it to a char and use it directly.

We use a StringBuilder to accumulate the string. Normally it's faster to track the start/end indices and do a substring(), but that won't work in the JSDoc extractor because of the star-skipping logic in advance().

This code was a bit of a performance cringe. It copied every character
into a temporary array, copied that into a String, and slow-appended
that onto another String.

Note that the call to Characters.toChars is redundant here as advance()
doesn't return a code point; it returns -1 or a UTF-16 char. The -1 case
is checked for before reaching the call, so we can just cast it to
a char and use it directly.

We use a StringBuilder to accumulate the string. Normally it's faster
to track the start/end indices and do a substring(), but that won't
work in the JSDoc extractor because of the star-skipping logic in
advance().
@github-actions github-actions Bot added the JS label Mar 20, 2025
@asgerf asgerf added the no-change-note-required This PR does not need a change note label Mar 20, 2025
@asgerf asgerf marked this pull request as ready for review March 20, 2025 13:37
Copilot AI review requested due to automatic review settings March 20, 2025 13:37
@asgerf asgerf requested a review from a team as a code owner March 20, 2025 13:37
Copy link
Copy Markdown
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 improves the performance of type name accumulation in JSDoc extraction by replacing inefficient string concatenation with a StringBuilder.

  • Replaces the use of new String(Character.toChars(advance())) with StringBuilder's append((char)advance()).
  • Removes redundant conversion of characters via Characters.toChars, directly casting the char returned by advance().

Tip: Copilot only keeps its highest confidence comments to reduce noise and keep you focused. Learn more

Copy link
Copy Markdown
Contributor

@erik-krogh erik-krogh left a comment

Choose a reason for hiding this comment

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

👍

@asgerf asgerf merged commit d9c1589 into github:main Mar 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

JS no-change-note-required This PR does not need a change note

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants