perf(transformer/jsx): speed up decoding `JSXText` strings #9741

overlookmotel · 2025-03-13T08:57:52Z

close: #10249

Speed up transforming JSXText nodes by:

Avoiding allocations (construct strings directly in arena, with no intermediate Strings).
Where no HTML entity decoding or string concatenation is required, reuse a slice of source text, rather than generating a new Atom and copying string data.

overlookmotel · 2025-03-13T08:58:07Z

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.

codspeed-hq · 2025-03-13T09:14:46Z

CodSpeed Instrumentation Performance Report

Merging #9741 will degrade performances by 3.69%

_{Comparing 03-13-perf_transformer_jsx_speed_up_decoding_jsxtext_strings (267922b) with main (b54fb3e)}

Summary

⚡ 2 improvements
❌ 1 regressions
✅ 33 untouched benchmarks

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

Benchmarks breakdown

	Benchmark	`BASE`	`HEAD`	Change
❌	`formatter[antd.js]`	7.6 ms	7.9 ms	-3.69%
⚡	`formatter[typescript.js]`	7.4 ms	7.2 ms	+3.3%
⚡	`transformer[RadixUIAdoptionSection.jsx]`	144.3 µs	137.1 µs	+5.29%

overlookmotel · 2025-03-13T09:21:01Z

Alright!

overlookmotel · 2025-03-13T09:24:22Z

@Dunqing Do you know how good test coverage we have for transforming JSXText? I was surprised that when I ran transform conformance for first time after making these changes that no tests failed. Usually, I make at least one mistake somewhere. I'm suspicious that maybe I have made mistakes, but we just don't have enough test coverage to catch them.

Obviously I can read through Babel's tests, but I know you will have battled through passing all the conformance tests for this transform last year, so I figured you probably know the answer off top of your head.

Dunqing · 2025-03-13T09:32:39Z

@Dunqing Do you know how good test coverage we have for transforming JSXText? I was surprised that when I ran transform conformance for first time after making these changes that no tests failed. Usually, I make at least one mistake somewhere. I'm suspicious that maybe I have made mistakes, but we just don't have enough test coverage to catch them.

Obviously I can read through Babel's tests, but I know you will have battled through passing all the conformance tests for this transform last year, so I figured you probably know the answer off top of your head.

I am sorry that I don't know, as #9667 (comment) said, the current logic is the same as TypeScript, we may need to look at TypeScript to find tests? Or you have rewritten the code 100% correctly while improving the performance!

Boshen · 2025-03-20T09:25:01Z

Moving decoding logic to the lexer: #9667

overlookmotel · 2025-04-08T12:46:14Z

As discussed in #10249, it'll be easier to move the logic for decoding & etc into the lexer once this is merged. The optimizations in this PR will still apply, as removing whitespace still has to happen in transformer.

All that's needed is for me to check Babel's tests cover all possible permutations of line breaks/whitespace in JSXText elements, and add a new test to cover any which are missing - then we can be sure this change doesn't break anything.

overlookmotel · 2025-04-08T23:24:12Z

All the tests added in #10324 pass. I think this is good to go.

Dunqing · 2025-04-09T09:15:56Z

Merge activity

Apr 9, 5:15 AM EDT: The merge label '0-merge' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
Apr 9, 5:16 AM EDT: Dunqing added this pull request to the Graphite merge queue.
Apr 9, 5:25 AM EDT: Merged by the Graphite merge queue.

close: #10249 Speed up transforming `JSXText` nodes by: 1. Avoiding allocations (construct strings directly in arena, with no intermediate `String`s). 2. Where no HTML entity decoding or string concatenation is required, reuse a slice of source text, rather than generating a new `Atom` and copying string data.

overlookmotel · 2025-04-09T10:05:18Z

Thanks for reviewing.

@camchenry

commit e419aaf Author: overlookmotel <[email protected]> Date: Wed Apr 9 09:54:18 2025 +0000 refactor(ast_tools): move limit for inlining to a const in `Visit` generator (#10291) Pure refactor. `Visit` generator adds `#[inline]` attr to visitor methods containing 5 or less statements. Make this limit a `const`, rather than having it inline in multiple places. commit a605247 Author: overlookmotel <[email protected]> Date: Wed Apr 9 09:54:17 2025 +0000 refactor(ast_tools): make visit call generator functions generic (#10290) Pure refactor. Make functions in `Visit` generator which generate `visitor.visit_*(...)` statements generic over `VisitorOutputs`. `VisitorOutputs` trait represents a set of outputs. In generator for `Visit` and `VisitMut`, there are 2 outputs for the 2 traits. But other generators need to be able to just generate `Visit` alone. Preparatory work for #10292. commit 411610f Author: overlookmotel <[email protected]> Date: Wed Apr 9 09:54:17 2025 +0000 refactor(ast_tools): change methods to free functions in `Visit` generator (#10289) Pure refactor. Move methods of `VisitBuilder` which generate `visitor.visit_*(...)` statements to be free functions, so that they can be exported and used by other generators. Preparatory work for #10292. commit 267922b Author: overlookmotel <[email protected]> Date: Wed Apr 9 09:16:59 2025 +0000 perf(transformer/jsx): speed up decoding `JSXText` strings (#9741) close: #10249 Speed up transforming `JSXText` nodes by: 1. Avoiding allocations (construct strings directly in arena, with no intermediate `String`s). 2. Where no HTML entity decoding or string concatenation is required, reuse a slice of source text, rather than generating a new `Atom` and copying string data. commit 11647e8 Author: overlookmotel <[email protected]> Date: Wed Apr 9 09:16:59 2025 +0000 test(transformer/jsx): tests for `JSXText` strings (#10324) Add tests for transforming `JSXText` and `JSXAttributeValue`. Tests cover: * Whitespace and line breaks. * HTML entities ([`HTMLCharacterReference`](https://facebook.github.io/jsx/#sec-HTMLCharacterReference)). commit b54fb3e Author: Yuji Sugiura <[email protected]> Date: Wed Apr 9 17:13:24 2025 +0900 fix(estree): Rename `TSInstantiationExpression`.`type_parameters` to `type_arguments` (#10327) Part of #9705 commit 9734152 Author: Yuji Sugiura <[email protected]> Date: Wed Apr 9 16:14:25 2025 +0900 fix(ast): Handle `TSThisType` in `TSTypePredicate` (#10328) Part of #9705 When given this code: ```ts interface X { y(): this is { z: 1 } } ``` Currently the parser treats `this` inside the type annotation as `Identifier` named `this`, but it should be `TSThisType`. commit 81867c4 Author: camc314 <[email protected]> Date: Wed Apr 9 04:39:13 2025 +0000 fix(linter): fix stack overflow in react/exhaustive deps (#10322) fixes #10319 commit a95ba40 Author: Sysix <[email protected]> Date: Tue Apr 8 22:33:17 2025 +0000 refactor(language_server): make server more error resistance by falling back to default config (#10257) When the client does provide us with an invalid config path / file, the server should not crash. Related oxc-project/oxc-zed#10 #10123 commit e0b6c8c Author: overlookmotel <[email protected]> Date: Tue Apr 8 22:05:34 2025 +0000 fix(transformer/react): correct comment (#10323) Fix comment in function for decoding HTML entities in `JSXText` elements. Hex code escape is `ሴ` not `&x1234;`. Also reformat comments in this function. commit 294d24b Author: overlookmotel <[email protected]> Date: Tue Apr 8 16:15:13 2025 +0000 refactor(ast/estree): simplify serialization for `JSXOpeningFragment` (#10316) Follow-on after #10208. We can remove the `JSXOpeningFragmentAttributes` serializer, and use `TsEmptyArray` instead. This results in `attributes?: []` in the TS type def, which seems reasonable since a JSX fragment can't have attributes. Really this field shouldn't exist at all! It can probably be considered a mistake in `acorn-jsx` that it does. commit cc07efd Author: therewillbecode <[email protected]> Date: Tue Apr 8 16:55:36 2025 +0100 fix(ast/estree): Fix `JSXOpeningFragment` (#10208) Fix `JSXOpeningFragment` node estree TS serialization by skipping `selfClosing` and `attributes` fields when AST is TS. Part of our broader work to align our AST's ESTree output with that of TS-ESLint's. Relates to #9705 --------- Co-authored-by: overlookmotel <[email protected]> commit 48ed6a1 Author: overlookmotel <[email protected]> Date: Tue Apr 8 13:07:27 2025 +0000 fix(ast/estree): fix span for `TemplateElement` in TS AST (#10315) Part of #9705. TS-ESLint differs from Acorn in the `span` of `TemplateElement`. TS-ESLint includes the preceding `` ` `` or `}` and following `${` or `` ` `` in the span. ```js const template = `abc${x}def${x}ghi`; // Acorn: ^^^ ^^^ ^^^ // TS-ESLint: ^^^^^^ ^^^^^^ ^^^^^ ``` Make the span follow TS-ESLint in the TS AST. commit 48c711a Author: 翠 / green <[email protected]> Date: Tue Apr 8 19:53:14 2025 +0900 fix(minifier): panic when compressing `a ? b() : b()` (#10311) found in vitejs/rolldown-vite#104 commit 4268b23 Author: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Date: Tue Apr 8 17:13:17 2025 +0800 chore(deps): lock file maintenance rust crates (#10300) commit eba5fcf Author: oxc-bot <[email protected]> Date: Tue Apr 8 13:52:15 2025 +0800 release(crates): v0.63.0 (#10309) commit 52ea978 Author: Ulrich Stark <[email protected]> Date: Tue Apr 8 06:07:48 2025 +0200 refactor(linter): update comments, improve tests, add variant All to LintFilterKind (#10259) I split this PR into easily reviewable commits. While working on this PR, I noticed two possible follow up changes: `LintFilterKind::Rule` and `LintFilterKind::Generic` behave exactly the same in `ConfigStoreBuilder::with_filter` because `LintFilterKind::Rule` never actually uses or checks for the plugin it stores. Should I include the check for plugin? The proposed fifth `LintFilterKind` isn't implemented yet (see: `// TODO: plugin + category? e.g -A react:correctness)`). Should I add it? commit 9aaba69 Author: Sub <[email protected]> Date: Tue Apr 8 08:07:31 2025 +0400 fix(linter): nested configuration directory resolution (#10157) Fixes #10156 I'm not very familiar with the inner workings of this project - this was a fix based on what I could understand after exploring the codebase for the first time. @camchenry It would be great to get your eyes on this since you are familiar with the nested config implementation. commit e0057c3 Author: Sysix <[email protected]> Date: Tue Apr 8 01:49:06 2025 +0000 perf(language_server): only restart internal linter once when multiple config changes detected (#10256)

github-actions bot added A-transformer Area - Transformer / Transpiler C-performance Category - Solution not expected to change functional behavior, only performance labels Mar 13, 2025

overlookmotel force-pushed the 03-13-perf_transformer_jsx_speed_up_decoding_jsxtext_strings branch from 75c9162 to 3db4be4 Compare March 13, 2025 09:00

Boshen closed this Mar 20, 2025

overlookmotel mentioned this pull request Apr 7, 2025

perf(transformer/jsx): use ArenaString to store decoded JSX text #10249

Closed

overlookmotel reopened this Apr 8, 2025

overlookmotel force-pushed the 03-13-perf_transformer_jsx_speed_up_decoding_jsxtext_strings branch 2 times, most recently from 98714f1 to 9a750d3 Compare April 8, 2025 17:48

overlookmotel changed the base branch from main to graphite-base/9741 April 8, 2025 23:18

overlookmotel force-pushed the 03-13-perf_transformer_jsx_speed_up_decoding_jsxtext_strings branch from 9a750d3 to 9af9e50 Compare April 8, 2025 23:18

overlookmotel changed the base branch from graphite-base/9741 to 04-09-test_transformer_jsx_tests_for_jsxtext_strings April 8, 2025 23:19

overlookmotel mentioned this pull request Apr 8, 2025

test(transformer/jsx): tests for JSXText strings #10324

Merged

overlookmotel force-pushed the 04-09-test_transformer_jsx_tests_for_jsxtext_strings branch from 03c6efd to da35ef9 Compare April 8, 2025 23:23

overlookmotel force-pushed the 03-13-perf_transformer_jsx_speed_up_decoding_jsxtext_strings branch from 9af9e50 to c1fbc1b Compare April 8, 2025 23:23

overlookmotel marked this pull request as ready for review April 8, 2025 23:24

overlookmotel requested a review from Dunqing as a code owner April 8, 2025 23:24

overlookmotel force-pushed the 03-13-perf_transformer_jsx_speed_up_decoding_jsxtext_strings branch from c1fbc1b to 13725c9 Compare April 8, 2025 23:56

overlookmotel force-pushed the 04-09-test_transformer_jsx_tests_for_jsxtext_strings branch from da35ef9 to 7a3a29e Compare April 8, 2025 23:56

Dunqing approved these changes Apr 9, 2025

View reviewed changes

Dunqing added the 0-merge Merge with Graphite Merge Queue label Apr 9, 2025

graphite-app bot force-pushed the 04-09-test_transformer_jsx_tests_for_jsxtext_strings branch from 7a3a29e to 11647e8 Compare April 9, 2025 09:17

graphite-app bot force-pushed the 03-13-perf_transformer_jsx_speed_up_decoding_jsxtext_strings branch from 13725c9 to 267922b Compare April 9, 2025 09:17

Base automatically changed from 04-09-test_transformer_jsx_tests_for_jsxtext_strings to main April 9, 2025 09:23

graphite-app bot removed the 0-merge Merge with Graphite Merge Queue label Apr 9, 2025

graphite-app bot merged commit 267922b into main Apr 9, 2025
27 checks passed

graphite-app bot deleted the 03-13-perf_transformer_jsx_speed_up_decoding_jsxtext_strings branch April 9, 2025 09:25

oxc-bot mentioned this pull request Apr 17, 2025

release(crates): v0.64.0 #10459

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

perf(transformer/jsx): speed up decoding `JSXText` strings #9741

perf(transformer/jsx): speed up decoding `JSXText` strings #9741

Uh oh!

overlookmotel commented Mar 13, 2025 •

edited by Dunqing

Loading

Uh oh!

overlookmotel commented Mar 13, 2025 •

edited

Loading

Uh oh!

codspeed-hq bot commented Mar 13, 2025 •

edited

Loading

Uh oh!

overlookmotel commented Mar 13, 2025

Uh oh!

overlookmotel commented Mar 13, 2025 •

edited

Loading

Uh oh!

Dunqing commented Mar 13, 2025

Uh oh!

Boshen commented Mar 20, 2025

Uh oh!

overlookmotel commented Apr 8, 2025

Uh oh!

overlookmotel commented Apr 8, 2025

Uh oh!

Dunqing commented Apr 9, 2025 •

edited by graphite-app bot

Loading

Uh oh!

Uh oh!

overlookmotel commented Apr 9, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

perf(transformer/jsx): speed up decoding JSXText strings #9741

perf(transformer/jsx): speed up decoding JSXText strings #9741

Uh oh!

Conversation

overlookmotel commented Mar 13, 2025 • edited by Dunqing Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

overlookmotel commented Mar 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

How to use the Graphite Merge Queue

Uh oh!

codspeed-hq bot commented Mar 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CodSpeed Instrumentation Performance Report

Merging #9741 will degrade performances by 3.69%

Summary

Benchmarks breakdown

Uh oh!

overlookmotel commented Mar 13, 2025

Uh oh!

overlookmotel commented Mar 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Dunqing commented Mar 13, 2025

Uh oh!

Boshen commented Mar 20, 2025

Uh oh!

overlookmotel commented Apr 8, 2025

Uh oh!

overlookmotel commented Apr 8, 2025

Uh oh!

Dunqing commented Apr 9, 2025 • edited by graphite-app bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merge activity

Uh oh!

Uh oh!

overlookmotel commented Apr 9, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

perf(transformer/jsx): speed up decoding `JSXText` strings #9741

perf(transformer/jsx): speed up decoding `JSXText` strings #9741

overlookmotel commented Mar 13, 2025 •

edited by Dunqing

Loading

overlookmotel commented Mar 13, 2025 •

edited

Loading

codspeed-hq bot commented Mar 13, 2025 •

edited

Loading

overlookmotel commented Mar 13, 2025 •

edited

Loading

Dunqing commented Apr 9, 2025 •

edited by graphite-app bot

Loading