From 98d4d932195c1cfd60ef053154ac2d926248af65 Mon Sep 17 00:00:00 2001 From: Ivan Goncharov Date: Wed, 24 Mar 2021 14:19:21 +0200 Subject: [PATCH] Replace 'Array.reduce' with 'for of' Results in measurable perfomance increase and significantly lower memory usage in a few benchmarks. --- src/error/GraphQLError.js | 16 +++++++-------- src/jsutils/keyMap.js | 9 +++++---- src/jsutils/keyValMap.js | 9 +++++---- src/jsutils/promiseForObject.js | 15 +++++++------- .../experimentalOnlineParser/onlineParser.js | 14 +++---------- src/validation/ValidationContext.js | 20 ++++++++----------- .../rules/OverlappingFieldsCanBeMergedRule.js | 9 ++------- 7 files changed, 38 insertions(+), 54 deletions(-) diff --git a/src/error/GraphQLError.js b/src/error/GraphQLError.js index f4d955e8a0..a8cb027bd4 100644 --- a/src/error/GraphQLError.js +++ b/src/error/GraphQLError.js @@ -102,12 +102,12 @@ export class GraphQLError extends Error { let _positions = positions; if (!_positions && _nodes) { - _positions = _nodes.reduce((list, node) => { + _positions = []; + for (const node of _nodes) { if (node.loc) { - list.push(node.loc.start); + _positions.push(node.loc.start); } - return list; - }, []); + } } if (_positions && _positions.length === 0) { _positions = undefined; @@ -117,12 +117,12 @@ export class GraphQLError extends Error { if (positions && source) { _locations = positions.map((pos) => getLocation(source, pos)); } else if (_nodes) { - _locations = _nodes.reduce((list, node) => { + _locations = []; + for (const node of _nodes) { if (node.loc) { - list.push(getLocation(node.loc.source, node.loc.start)); + _locations.push(getLocation(node.loc.source, node.loc.start)); } - return list; - }, []); + } } let _extensions = extensions; diff --git a/src/jsutils/keyMap.js b/src/jsutils/keyMap.js index 21c1ec3e3b..8bfaf2727c 100644 --- a/src/jsutils/keyMap.js +++ b/src/jsutils/keyMap.js @@ -27,8 +27,9 @@ export function keyMap( list: $ReadOnlyArray, keyFn: (item: T) => string, ): ObjMap { - return list.reduce((map, item) => { - map[keyFn(item)] = item; - return map; - }, Object.create(null)); + const result = Object.create(null); + for (const item of list) { + result[keyFn(item)] = item; + } + return result; } diff --git a/src/jsutils/keyValMap.js b/src/jsutils/keyValMap.js index 1981a520cf..d046cbd5e3 100644 --- a/src/jsutils/keyValMap.js +++ b/src/jsutils/keyValMap.js @@ -22,8 +22,9 @@ export function keyValMap( keyFn: (item: T) => string, valFn: (item: T) => V, ): ObjMap { - return list.reduce((map, item) => { - map[keyFn(item)] = valFn(item); - return map; - }, Object.create(null)); + const result = Object.create(null); + for (const item of list) { + result[keyFn(item)] = valFn(item); + } + return result; } diff --git a/src/jsutils/promiseForObject.js b/src/jsutils/promiseForObject.js index 879d732d1f..1074676030 100644 --- a/src/jsutils/promiseForObject.js +++ b/src/jsutils/promiseForObject.js @@ -10,12 +10,11 @@ import type { ObjMap } from './ObjMap'; export function promiseForObject( object: ObjMap>, ): Promise> { - const keys = Object.keys(object); - const valuesAndPromises = keys.map((name) => object[name]); - return Promise.all(valuesAndPromises).then((values) => - values.reduce((resolvedObject, value, i) => { - resolvedObject[keys[i]] = value; - return resolvedObject; - }, Object.create(null)), - ); + return Promise.all(Object.values(object)).then((resolvedValues) => { + const resolvedObject = Object.create(null); + for (const [i, key] of Object.keys(object).entries()) { + resolvedObject[key] = resolvedValues[i]; + } + return resolvedObject; + }); } diff --git a/src/language/experimentalOnlineParser/onlineParser.js b/src/language/experimentalOnlineParser/onlineParser.js index 3418bada9d..92a9c667d0 100644 --- a/src/language/experimentalOnlineParser/onlineParser.js +++ b/src/language/experimentalOnlineParser/onlineParser.js @@ -389,17 +389,9 @@ export class OnlineParser { ): boolean { if (rule.butNot) { if (Array.isArray(rule.butNot)) { - if ( - rule.butNot.reduce( - (matched, constraint) => - matched || this._matchToken(token, constraint), - false, - ) - ) { - return false; - } - - return true; + return !rule.butNot.some((constraint) => + this._matchToken(token, constraint), + ); } return !this._matchToken(token, rule.butNot); diff --git a/src/validation/ValidationContext.js b/src/validation/ValidationContext.js index 2a0d8aef1a..c2f602a8ca 100644 --- a/src/validation/ValidationContext.js +++ b/src/validation/ValidationContext.js @@ -67,19 +67,15 @@ export class ASTValidationContext { } getFragment(name: string): ?FragmentDefinitionNode { - let fragments = this._fragments; - if (!fragments) { - this._fragments = fragments = this.getDocument().definitions.reduce( - (frags, statement) => { - if (statement.kind === Kind.FRAGMENT_DEFINITION) { - frags[statement.name.value] = statement; - } - return frags; - }, - Object.create(null), - ); + if (!this._fragments) { + const fragments = (this._fragments = Object.create(null)); + for (const defNode of this.getDocument().definitions) { + if (defNode.kind === Kind.FRAGMENT_DEFINITION) { + fragments[defNode.name.value] = defNode; + } + } } - return fragments[name]; + return this._fragments[name]; } getFragmentSpreads( diff --git a/src/validation/rules/OverlappingFieldsCanBeMergedRule.js b/src/validation/rules/OverlappingFieldsCanBeMergedRule.js index 830c4098a6..3468738427 100644 --- a/src/validation/rules/OverlappingFieldsCanBeMergedRule.js +++ b/src/validation/rules/OverlappingFieldsCanBeMergedRule.js @@ -775,13 +775,8 @@ function subfieldConflicts( if (conflicts.length > 0) { return [ [responseName, conflicts.map(([reason]) => reason)], - conflicts.reduce((allFields, [, fields1]) => allFields.concat(fields1), [ - node1, - ]), - conflicts.reduce( - (allFields, [, , fields2]) => allFields.concat(fields2), - [node2], - ), + [node1, ...conflicts.map(([, fields1]) => fields1).flat()], + [node2, ...conflicts.map(([, , fields2]) => fields2).flat()], ]; } }