fix: avoid string copy when processing input source-map#10885
fix: avoid string copy when processing input source-map#10885existentialism merged 3 commits intobabel:masterfrom
Conversation
| // eslint-disable-next-line max-len | ||
| const INLINE_SOURCEMAP_REGEX = /^[@#]\s+sourceMappingURL=data:(?:application|text)\/json;(?:charset[:=]\S+?;)?base64,(?:.*)$/; | ||
| const EXTERNAL_SOURCEMAP_REGEX = /^[@#][ \t]+sourceMappingURL=(?:[^\s'"`]+?)[ \t]*$/; | ||
| const EXTERNAL_SOURCEMAP_REGEX = /^[@#][ \t]+sourceMappingURL=(?<url>[^\s'"`]+?)[ \t]*$/; |
There was a problem hiding this comment.
Dogfooding named-capturing-regex here.
The file size is bloated by helpers (6678 B -> 11276 B) but I think it is okay since named-capturing-regex is supported on Node.js 10+, which means we can get rid of the helpers in v8.
I have changed my mind.
nicolo-ribaudo
left a comment
There was a problem hiding this comment.
I agree with @jridgewell suggestion. Apart from that, ✔️
Co-Authored-By: Justin Ridgewell <justin@ridgewell.name>
|
Derp, missed the flow errors... will push a PR fix in the morning if no one gets to it first! |
|
Hello, thanks for such a quick response to this issue @JLHwung ! However I tried to replace the file you provided and it did not seem to do anything. Not sure if I'm doing it wrong, but I'm just supposed to replace the file at the path right? I'm using the same repo I linked in the issue. Also tried rolling back before ejection and then replacing - same result. |
|
Ops, I didn't see your comment 😅 |
|
@artisb45 # Clear babel-loader cache
rm -rf node_modules/.cache/babel-loader
cp normalize-file.js node_modules/@babel/core/lib/transformation/normalize-file.js
npm run build
Update: #10875 is not fixed: #10875 (comment) |
This PR fixes a performance regression introduced in #10623.
babel/packages/babel-core/src/transformation/normalize-file.js
Lines 68 to 69 in c35ba3d
The
lastCommentwas copied into a new string, prefixed by//and later unwrapped by convert-source-map.https://github.com/thlorenz/convert-source-map/blob/9e9a7a9b652c30878c9c9aa591d861a9fdf61a7e/index.js#L31
This approach introduced significant memory footprint when there are many files with input source-map, i.e. the test repo mentioned in #10875. Here we use
convertSourceMap.fromJSONto avoid unnecessary string copy.@artisb45 Thank you for the reproduction repo, it helps me a lot! You can test flight this fix by downloading and overwrite
node_modules/@babel/core/lib/transformation/normalize-file.jswith this file. We are expected to land it in a patch release if review is good.