Big performance drop due to switching TS output from ES5 to ES6

[IvanGoncharov]

I investigated the performance drop between v15 and v16 and it was caused by switching transpilation target from es5 to es6. It's easy to reproduce with the following diff:

diff --git a/tsconfig.json b/tsconfig.json
index edb522f1..299b9309 100644
--- a/tsconfig.json
+++ b/tsconfig.json
@@ -7,7 +7,7 @@
   ],
   "compilerOptions": {
     "lib": ["es2020"],
-    "target": "es2020",
+    "target": "es5",
     "module": "commonjs",
     "moduleResolution": "node",
     "strict": true,

Note: some of the functionality stops working due to an error in this file (we can't extend Map in es5): https://github.com/graphql/graphql-js/blob/main/src/jsutils/AccumulatorMap.ts

But even benchmarks that are working showing pretty big performance difference: local is ES5 and HEAD is ES2020

I don't think we need to switch back to ES5 since it has its own drawbacks but instead, we just need to figure out what is slowing us and implement that code changes manually in our code base.

[JoviDeCroock]

As a reference there are a handful of features that are ES6 that are a lot faster when transpiled into ES5, might be worth scanning the code base for some of these and maybe set up some transformations for the published files or adjust the code.

An example here would be for..of what is your preference on how this is tackled, a selective transpilation or hand-coded?

[marvinhagemeister]

Like @JoviDeCroock said my hunch is that ES2015+ features aren't as well optimized as ES5 ones. for..of is a good candidate as that always has to invoke the iterator protocol because the array prototype could've been messed with somewhere.

Another area where the iterator protocol leads to overhead is destructuring:

const arr = [1,2,3];

// invokes iterator protocol
const [a, b] = arr;

// usually faster than destructuring
const a = arr[0];
const b = arr[1];

FYI: The table linked was last run in 2020 and might not represent current engine optimizations well.

[JoviDeCroock]

Quickly did a few manual passes to confirm the assumptions and we seem to be on the right track here

[StefanFeederle]

I saw a noticable performance drop going from graphql-js v15 to v16.8.0. About ~40% depending on the query. @IvanGoncharov is your fix from https://github.com/IvanGoncharov/graphql-js/commit/d1dea90847cfa52d58bc7b3f91829a64fa4d36f5 not yet included in v16.8.0? Any chance you can release this? Thank you :)

[JoviDeCroock]

I don't think it's published, where did you see the perf drop? Parsing, execute,...?

[StefanFeederle]

How do i benchmark parsing vs execute?

My query is quite simple (3 objects, about 25 fields), but has a lot of results (~700kb json). I would guess it's probably the field execution because parsing should be quick in comparison.

query parts {
  parts: recentParts {
    ...partFragment
    __typename
  }
}

fragment partFragment on part {
  id
  pos
  width
  length
  height
  quantity
  quantityNested
  quantityClosed
  partName
  comment
  weight
  costTargetNumber
  drawing
  state
  identifier
  nestingIds
  createdAt
  updatedAt
  closedAt
  image
  firstDateIfUnfinished
  ORDERITEM {
    ORI_ID
    mainDrawing {
      clientPath
      __typename
    }
    mainStep {
      clientPath
      __typename
    }
    __typename
  }
  __typename
}

[diogotorres97]

@IvanGoncharov @JoviDeCroock @StefanFeederle is this fixed? We are considering upgrading from v15 to v16 😄

[JoviDeCroock]

This never made it in, I'm open to reviving my PR but would love to know what the steps are to getting it in then before committing more time to this.

[JoviDeCroock]

This is currently fixed on main - when comparing ES5 to the current main output the performance is equal.