-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Big performance drop due to switching TS output from ES5 to ES6 #3648
Comments
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 |
Like @JoviDeCroock said my hunch is that ES2015+ features aren't as well optimized as ES5 ones. 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. |
I saw a noticable performance drop going from graphql-js v15 to v16.8.0. About ~40% depending on the query. |
I don't think it's published, where did you see the perf drop? Parsing, execute,...? |
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).
|
@IvanGoncharov @JoviDeCroock @StefanFeederle is this fixed? We are considering upgrading from v15 to v16 😄 |
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. |
This is currently fixed on |
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:
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 andHEAD
is ES2020I 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.
The text was updated successfully, but these errors were encountered: