Replace generic __clone call by specific methods #13611

JLHwung · 2021-07-28T19:14:33Z

Q	A
License	MIT

In this PR we replace the generic Node#__clone by specific methods. We observe 80% performance boost compared to @babel/parser 7.14.8 in the length-1 named export benchmark cases. In this case the named export specifier is cloned as export values.

baseline 256 length-1 named export: 4_704 ops/sec ±1.59% (0.213ms)
baseline 512 length-1 named export: 2_426 ops/sec ±0.52% (0.412ms)
baseline 1024 length-1 named export: 1_118 ops/sec ±1.23% (0.895ms)
baseline 2048 length-1 named export: 556 ops/sec ±0.77% (1.799ms)
current 256 length-1 named export: 7_073 ops/sec ±33.67% (0.141ms)
current 512 length-1 named export: 4_441 ops/sec ±0.79% (0.225ms)
current 1024 length-1 named export: 2_142 ops/sec ±1.09% (0.467ms)
current 2048 length-1 named export: 943 ops/sec ±2.12% (1.06ms)

JLHwung · 2021-07-28T19:15:27Z

packages/babel-parser/src/parser/node.js

+  return cloneIdentifier(node);
+}
+
+export function cloneIdentifier(node: any): any {


Unfortunately I can not get flow types working on this method. The Placeholder type complicates the typings here.

Does using generics work? Something like

export function cloneIdentifier<T: Identifier | Placeholder<"Identifier">>(node: T): T

I tried generics. However it seems that Flow cannot infer the type from type === "Placeholder". So I fallback to invariant comments, however Flow will throw "unexpected )" at the invariant comment /*:: invariant(node instanceof Placeholder<"Identifier">) */.

Has the parser conversion to TS already started?

Not yet; you can see the wip at #11578

JLHwung · 2021-07-28T19:16:17Z

packages/babel-parser/src/parser/node.js

+  // cloneIdentifier is only used in object shorthand and named import/export.
+  // Neither of them allow type annotations after the identifier or optional identifier
+  const { type, start, end, loc, range, extra, name } = node;
+  const cloned = Object.create(NodePrototype);


We can use plain object after we remove __clone method from the AST node.

why do you need to clone? You will experience more than 80% perf boost if you avoid clone and instead rewrite lexer and parser. And also all the property access you are using kills perf.

babel-bot · 2021-07-28T19:19:16Z

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/47668/

codesandbox-ci · 2021-07-28T19:21:40Z

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 4bbf05d:

Sandbox	Source
babel-repl-custom-plugin	Configuration
babel-plugin-multi-config	Configuration

nicolo-ribaudo · 2021-07-28T19:41:08Z

packages/babel-parser/src/parser/node.js

@@ -27,6 +27,7 @@ class Node implements NodeBase {
  innerComments: Array<Comment>;
  extra: { [key: string]: any };

+  // todo(Babel 8): remove this method in Babel 8


You could remove it from here and do

if (!process.env.BABEL_8_BREAKING) { NodePrototype.__clone = function () { /* ... */ }; }

nicolo-ribaudo · 2021-07-28T19:43:07Z

packages/babel-parser/src/parser/node.js

+  return cloneIdentifier(node);
+}
+
+export function cloneIdentifier(node: any): any {


Does using generics work? Something like

export function cloneIdentifier<T: Identifier | Placeholder<"Identifier">>(node: T): T

packages/babel-parser/src/parser/statement.js

KFlash · 2021-07-29T00:02:52Z

packages/babel-parser/src/parser/node.js

  }
+  const cloned = Object.create(NodePrototype);


'Object.create' is so much slower than a constructor. And overall it's slow.

https://stackoverflow.com/questions/43372746/why-so-bad-the-performance-of-the-nodejs-object-creation-if-num-of-props-greater

We will remove Object.create here in Babel 8.

Can't you just use the 'proto' trick?

const table = { __proto__: null, JLHwung: author }

__proto__ is slower than Object.create:

baseline 256 length-1 named export: 5_650 ops/sec ±45.38% (0.177ms) baseline 512 length-1 named export: 4_129 ops/sec ±1.16% (0.242ms) baseline 1024 length-1 named export: 1_977 ops/sec ±1.31% (0.506ms) baseline 2048 length-1 named export: 894 ops/sec ±2.1% (1.118ms) current 256 length-1 named export: 5_269 ops/sec ±29.84% (0.19ms) current 512 length-1 named export: 3_003 ops/sec ±2.24% (0.333ms) current 1024 length-1 named export: 1_473 ops/sec ±2.04% (0.679ms) current 2048 length-1 named export: 710 ops/sec ±1.6% (1.409ms)

Interesting. I never clone so didn't check that. What I do is to have a create function that creates and return an obj based on the given params. When I need to "clone" I re-use that function and create a new obj based on the given params again. That way I avoid spread, clone and other slow stuff. And it makes GC happy

create a new obj based on the given params again

I agree. But we should get rid of loc and ranges first so the parameters can be limited to start, end, value and type (solely for placeholder support).

or just pack the loc data in a series of binary numbers

Well there are also filename and identifierName in the loc.

baseline 256 length-1 named export: 4_704 ops/sec ±1.59% (0.213ms) baseline 512 length-1 named export: 2_426 ops/sec ±0.52% (0.412ms) baseline 1024 length-1 named export: 1_118 ops/sec ±1.23% (0.895ms) baseline 2048 length-1 named export: 556 ops/sec ±0.77% (1.799ms) current 256 length-1 named export: 7_073 ops/sec ±33.67% (0.141ms) current 512 length-1 named export: 4_441 ops/sec ±0.79% (0.225ms) current 1024 length-1 named export: 2_142 ops/sec ±1.09% (0.467ms) current 2048 length-1 named export: 943 ops/sec ±2.12% (1.06ms)

JLHwung added pkg: parser PR: Performance 🏃‍♀️ A type of pull request used for our changelog categories labels Jul 28, 2021

JLHwung commented Jul 28, 2021

View reviewed changes

nicolo-ribaudo reviewed Jul 28, 2021

View reviewed changes

KFlash reviewed Jul 29, 2021

View reviewed changes

JLHwung added 4 commits July 29, 2021 10:01

update benchmark babel parser version

de0fe04

breaking: remove Node#__clone in Babel 8

fc9a8af

test: use t.cloneNode

4bbf05d

JLHwung force-pushed the avoid--clone-call-in-parser branch from 0dca513 to 4bbf05d Compare July 29, 2021 14:02

nicolo-ribaudo approved these changes Jul 29, 2021

View reviewed changes

fedeci approved these changes Jul 29, 2021

View reviewed changes

nicolo-ribaudo merged commit d3a7cd5 into babel:main Jul 30, 2021

nicolo-ribaudo deleted the avoid--clone-call-in-parser branch July 30, 2021 20:19

sync-by-unito bot mentioned this pull request Aug 2, 2021

chore(deps-dev): bump @babel/eslint-parser from 7.13.14 to 7.14.9 filecoin-project/slate#844

Closed

github-actions bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Oct 30, 2021

github-actions bot locked as resolved and limited conversation to collaborators Oct 30, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace generic __clone call by specific methods #13611

Replace generic __clone call by specific methods #13611

JLHwung commented Jul 28, 2021 •

edited by gitpod-io bot

JLHwung Jul 28, 2021

nicolo-ribaudo Jul 28, 2021

JLHwung Jul 28, 2021

fedeci Jul 28, 2021

nicolo-ribaudo Jul 29, 2021

JLHwung Jul 28, 2021

KFlash Jul 28, 2021 •

edited

babel-bot commented Jul 28, 2021 •

edited

codesandbox-ci bot commented Jul 28, 2021 •

edited

nicolo-ribaudo Jul 28, 2021

nicolo-ribaudo Jul 28, 2021

KFlash Jul 29, 2021

JLHwung Jul 29, 2021

KFlash Jul 29, 2021 •

edited

JLHwung Jul 29, 2021

KFlash Jul 29, 2021

JLHwung Jul 29, 2021

KFlash Jul 29, 2021

JLHwung Jul 29, 2021

Replace generic __clone call by specific methods #13611

Replace generic __clone call by specific methods #13611

Conversation

JLHwung commented Jul 28, 2021 • edited by gitpod-io bot

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KFlash Jul 28, 2021 • edited

Choose a reason for hiding this comment

babel-bot commented Jul 28, 2021 • edited

codesandbox-ci bot commented Jul 28, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KFlash Jul 29, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JLHwung commented Jul 28, 2021 •

edited by gitpod-io bot

KFlash Jul 28, 2021 •

edited

babel-bot commented Jul 28, 2021 •

edited

codesandbox-ci bot commented Jul 28, 2021 •

edited

KFlash Jul 29, 2021 •

edited