Skip to content

ES2020 optimizations#3687

Closed
JoviDeCroock wants to merge 6 commits into
graphql:mainfrom
JoviDeCroock:perf-iterators
Closed

ES2020 optimizations#3687
JoviDeCroock wants to merge 6 commits into
graphql:mainfrom
JoviDeCroock:perf-iterators

Conversation

@JoviDeCroock

@JoviDeCroock JoviDeCroock commented Aug 5, 2022

Copy link
Copy Markdown
Contributor

This PR aims to explain what's going on for #3648 the main issues seem to be:

  • using for..of in hot paths
  • using array destructuring

If we find this to be sufficient we can find a way to add a build step or merge as-is with some lint-rules. This currently touches the following issues in hot-paths only.

There are more issues to be found. Other slowdowns could be due to raw ES6 classes, ....
EDIT: verifies yes, ES5 classes are a lot faster like these but imho it's not worth writing that as opposed to the little change of avoiding for..of in hot paths 😅

I have a suspicion that the custom extension of Map (AccumulatorMap) is also to blame as it lives on the hot-path

Preliminary results where local is ES5 and the other is this branch. (some benches don't run when I use ES5 in tsconfig)

⏱   Recreate a GraphQLSchema
  2 tests completed.

  local                                    x 1,774 ops/sec ±0.21% x 288 KB/op (25 runs sampled)
  a6a240321b7ae96622eb120b7c1fa5eaafc6f16f x 1,386 ops/sec ±0.26% x 453 KB/op (22 runs sampled)

⏱   Build Schema from AST
  2 tests completed.

  local                                    x 145 ops/sec ±0.11% x  3.6 MB/op (9 runs sampled)
  a6a240321b7ae96622eb120b7c1fa5eaafc6f16f x 166 ops/sec ±0.58% x 3.55 MB/op (11 runs sampled)

⏱   Build Schema from Introspection
  2 tests completed.

  local                                    x 169 ops/sec ±0.13% x 3.95 MB/op (15 runs sampled)
  a6a240321b7ae96622eb120b7c1fa5eaafc6f16f x 163 ops/sec ±0.23% x 3.98 MB/op (12 runs sampled)

⏱   Execute Introspection Query
  2 tests completed.

  local                                    x 94,695 ops/sec ±1.99% x 2.85 KB/op (23 runs sampled)
  a6a240321b7ae96622eb120b7c1fa5eaafc6f16f x  74.84 ops/sec ±0.60% x 29.2 MB/op (5 runs sampled)

@netlify

netlify Bot commented Aug 5, 2022

Copy link
Copy Markdown

Deploy Preview for compassionate-pike-271cb3 ready!

Name Link
🔨 Latest commit 730e58b
🔍 Latest deploy log https://app.netlify.com/sites/compassionate-pike-271cb3/deploys/62ed6b5fd7dc290009555830
😎 Deploy Preview https://deploy-preview-3687--compassionate-pike-271cb3.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

Comment thread src/execution/execute.ts
for (const [responseName, fieldNodes] of fields.entries()) {
const fieldsEntries = Array.from(fields.entries());
for (let i = 0; i < fieldsEntries.length; i++) {
const { 0: responseName, 1: fieldNodes } = fieldsEntries[i];

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
const { 0: responseName, 1: fieldNodes } = fieldsEntries[i];
const [responseName, fieldNodes] = fieldsEntries[i];

This version is less wierd

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That version invokes the iterator and is very slow though

IvanGoncharov added a commit to IvanGoncharov/graphql-js that referenced this pull request Aug 10, 2022
Explanation: `Object.create(null)` returns value of `any` type.
So bellow construct is not reported by TS even in "strict" mode:
```ts
const foo = Object.create(null);
```
Fixing this issue in `extendSchema` requires adding more code since we
can't put all extensions nodes into one collection without loosing
typesafety.
IvanGoncharov added a commit that referenced this pull request Aug 10, 2022
Explanation: `Object.create(null)` returns value of `any` type.
So bellow construct is not reported by TS even in "strict" mode:
```ts
const foo = Object.create(null);
```
Fixing this issue in `extendSchema` requires adding more code since we
can't put all extensions nodes into one collection without loosing
typesafety.
IvanGoncharov added a commit to IvanGoncharov/graphql-js that referenced this pull request Aug 11, 2022
Context: I wanted to try more automatic alternative to graphql#3687 that
allows us to continue using ES6 features without perfomance lost.
@IvanGoncharov

Copy link
Copy Markdown
Member

@JoviDeCroock Thanks for PR and research.
Can you please rebase, since I added a few related improvements on main?
I also made an experiment to see, if we can keep modern syntax without a performance drop.
I published it as #3691, what do you think about it?

BTW, your branch is still faster in some tests but slower in others:
image
Maybe rebasing will fix this.

@JoviDeCroock

JoviDeCroock commented Aug 11, 2022

Copy link
Copy Markdown
Contributor Author

@IvanGoncharov #3691 optimizes some problematic ones, you might want to add a plugin like https://babeljs.io/docs/en/babel-plugin-transform-destructuring or the one from react-hooks because they transform const [x, y] = useState to const { 0: x, 1: y } there as well https://github.com/vercel/next.js/blob/canary/packages/next-swc/crates/core/src/hook_optimizer.rs. I think that might be the last performance gap we have between our benches, A last point of optimization might be avoiding using extends on the Map built-in.

If you want I can also close this as your transform approach looks better to me, thank you!

EDIT: if you want I can write that transformer as well

IvanGoncharov added a commit to IvanGoncharov/graphql-js that referenced this pull request Aug 11, 2022
Context: I wanted to try more automatic alternative to graphql#3687 that
allows us to continue using ES6 features without perfomance lost.
@JoviDeCroock
JoviDeCroock deleted the perf-iterators branch August 12, 2022 08:08
yaacovCR pushed a commit to yaacovCR/graphql-js that referenced this pull request Dec 11, 2025
Explanation: `Object.create(null)` returns value of `any` type.
So bellow construct is not reported by TS even in "strict" mode:
```ts
const foo = Object.create(null);
```
Fixing this issue in `extendSchema` requires adding more code since we
can't put all extensions nodes into one collection without loosing
typesafety.
yaacovCR pushed a commit to yaacovCR/graphql-js that referenced this pull request Dec 11, 2025
Explanation: `Object.create(null)` returns value of `any` type.
So bellow construct is not reported by TS even in "strict" mode:
```ts
const foo = Object.create(null);
```
Fixing this issue in `extendSchema` requires adding more code since we
can't put all extensions nodes into one collection without loosing
typesafety.
yaacovCR pushed a commit to yaacovCR/graphql-js that referenced this pull request Dec 11, 2025
Explanation: `Object.create(null)` returns value of `any` type.
So bellow construct is not reported by TS even in "strict" mode:
```ts
const foo = Object.create(null);
```
Fixing this issue in `extendSchema` requires adding more code since we
can't put all extensions nodes into one collection without loosing
typesafety.
yaacovCR pushed a commit to yaacovCR/graphql-js that referenced this pull request Dec 11, 2025
Explanation: `Object.create(null)` returns value of `any` type.
So bellow construct is not reported by TS even in "strict" mode:
```ts
const foo = Object.create(null);
```
Fixing this issue in `extendSchema` requires adding more code since we
can't put all extensions nodes into one collection without loosing
typesafety.
yaacovCR pushed a commit to yaacovCR/graphql-js that referenced this pull request Dec 16, 2025
Explanation: `Object.create(null)` returns value of `any` type.
So bellow construct is not reported by TS even in "strict" mode:
```ts
const foo = Object.create(null);
```
Fixing this issue in `extendSchema` requires adding more code since we
can't put all extensions nodes into one collection without loosing
typesafety.
yaacovCR pushed a commit to yaacovCR/graphql-js that referenced this pull request Dec 17, 2025
Explanation: `Object.create(null)` returns value of `any` type.
So bellow construct is not reported by TS even in "strict" mode:
```ts
const foo = Object.create(null);
```
Fixing this issue in `extendSchema` requires adding more code since we
can't put all extensions nodes into one collection without loosing
typesafety.
yaacovCR pushed a commit to yaacovCR/graphql-js that referenced this pull request Dec 17, 2025
Explanation: `Object.create(null)` returns value of `any` type.
So bellow construct is not reported by TS even in "strict" mode:
```ts
const foo = Object.create(null);
```
Fixing this issue in `extendSchema` requires adding more code since we
can't put all extensions nodes into one collection without loosing
typesafety.
yaacovCR pushed a commit to yaacovCR/graphql-js that referenced this pull request Dec 22, 2025
Explanation: `Object.create(null)` returns value of `any` type.
So bellow construct is not reported by TS even in "strict" mode:
```ts
const foo = Object.create(null);
```
Fixing this issue in `extendSchema` requires adding more code since we
can't put all extensions nodes into one collection without loosing
typesafety.
yaacovCR pushed a commit that referenced this pull request Dec 22, 2025
Explanation: `Object.create(null)` returns value of `any` type.
So bellow construct is not reported by TS even in "strict" mode:
```ts
const foo = Object.create(null);
```
Fixing this issue in `extendSchema` requires adding more code since we
can't put all extensions nodes into one collection without loosing
typesafety.
yaacovCR pushed a commit that referenced this pull request Dec 22, 2025
Explanation: `Object.create(null)` returns value of `any` type.
So bellow construct is not reported by TS even in "strict" mode:
```ts
const foo = Object.create(null);
```
Fixing this issue in `extendSchema` requires adding more code since we
can't put all extensions nodes into one collection without loosing
typesafety.
yaacovCR pushed a commit to yaacovCR/graphql-js that referenced this pull request Jan 27, 2026
Explanation: `Object.create(null)` returns value of `any` type.
So bellow construct is not reported by TS even in "strict" mode:
```ts
const foo = Object.create(null);
```
Fixing this issue in `extendSchema` requires adding more code since we
can't put all extensions nodes into one collection without loosing
typesafety.
yaacovCR pushed a commit that referenced this pull request Feb 22, 2026
Explanation: `Object.create(null)` returns value of `any` type.
So bellow construct is not reported by TS even in "strict" mode:
```ts
const foo = Object.create(null);
```
Fixing this issue in `extendSchema` requires adding more code since we
can't put all extensions nodes into one collection without loosing
typesafety.
yaacovCR pushed a commit that referenced this pull request Feb 23, 2026
Explanation: `Object.create(null)` returns value of `any` type.
So bellow construct is not reported by TS even in "strict" mode:
```ts
const foo = Object.create(null);
```
Fixing this issue in `extendSchema` requires adding more code since we
can't put all extensions nodes into one collection without loosing
typesafety.
yaacovCR pushed a commit that referenced this pull request Feb 24, 2026
Explanation: `Object.create(null)` returns value of `any` type.
So bellow construct is not reported by TS even in "strict" mode:
```ts
const foo = Object.create(null);
```
Fixing this issue in `extendSchema` requires adding more code since we
can't put all extensions nodes into one collection without loosing
typesafety.
yaacovCR pushed a commit that referenced this pull request Feb 24, 2026
Explanation: `Object.create(null)` returns value of `any` type.
So bellow construct is not reported by TS even in "strict" mode:
```ts
const foo = Object.create(null);
```
Fixing this issue in `extendSchema` requires adding more code since we
can't put all extensions nodes into one collection without loosing
typesafety.
yaacovCR pushed a commit that referenced this pull request Feb 24, 2026
Explanation: `Object.create(null)` returns value of `any` type.
So bellow construct is not reported by TS even in "strict" mode:
```ts
const foo = Object.create(null);
```
Fixing this issue in `extendSchema` requires adding more code since we
can't put all extensions nodes into one collection without loosing
typesafety.
yaacovCR pushed a commit that referenced this pull request Feb 24, 2026
Explanation: `Object.create(null)` returns value of `any` type.
So bellow construct is not reported by TS even in "strict" mode:
```ts
const foo = Object.create(null);
```
Fixing this issue in `extendSchema` requires adding more code since we
can't put all extensions nodes into one collection without loosing
typesafety.
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.

2 participants