From ae96377d1ff55e46bc2d5985c258f0e218ffe1b5 Mon Sep 17 00:00:00 2001 From: Steven Chaitoff Date: Mon, 6 Feb 2017 14:14:58 -0500 Subject: [PATCH 1/3] Improve perf of separateOperations --- src/utilities/separateOperations.js | 32 +++++++++++++++++++++-------- 1 file changed, 23 insertions(+), 9 deletions(-) diff --git a/src/utilities/separateOperations.js b/src/utilities/separateOperations.js index 9ccfd5ff12..6b2208602f 100644 --- a/src/utilities/separateOperations.js +++ b/src/utilities/separateOperations.js @@ -11,6 +11,7 @@ import { visit } from '../language/visitor'; import type { DocumentNode, + FragmentDefinitionNode, OperationDefinitionNode, } from '../language/ast'; @@ -24,18 +25,22 @@ export function separateOperations( documentAST: DocumentNode ): { [operationName: string]: DocumentNode } { - const operations = []; + const definitions: DefinitionMap = Object.create(null); const depGraph: DepGraph = Object.create(null); let fromName; + let idx = 0; - // Populate the list of operations and build a dependency graph. + // Populate the list of definitions and build a dependency graph. visit(documentAST, { OperationDefinition(node) { - operations.push(node); fromName = opName(node); + definitions[fromName] = { idx, node }; + ++idx; }, FragmentDefinition(node) { fromName = node.name.value; + definitions[fromName] = { idx, node }; + ++idx; }, FragmentSpread(node) { const toName = node.name.value; @@ -47,23 +52,32 @@ export function separateOperations( // For each operation, produce a new synthesized AST which includes only what // is necessary for completing that operation. const separatedDocumentASTs = Object.create(null); - operations.forEach(operation => { - const operationName = opName(operation); + const operationNames = Object.keys(definitions).filter(defName => + definitions[defName].node.kind === 'OperationDefinition' + ); + operationNames.forEach(operationName => { const dependencies = Object.create(null); collectTransitiveDependencies(dependencies, depGraph, operationName); + dependencies[operationName] = true; separatedDocumentASTs[operationName] = { kind: 'Document', - definitions: documentAST.definitions.filter(def => - def === operation || - def.kind === 'FragmentDefinition' && dependencies[def.name.value] - ) + definitions: Object.keys(dependencies) + .map(defName => definitions[defName]) + .sort((def1, def2) => def1.idx - def2.idx) + .map(def => def.node) }; }); return separatedDocumentASTs; } +type DefinitionMap = { + [defName: string]: { + idx: number, + node: OperationDefinitionNode | FragmentDefinitionNode + } +}; type DepGraph = {[from: string]: {[to: string]: boolean}}; // Provides the empty string for anonymous operations. From 36ea2babac8bcee1be68f1481668d52926000b5d Mon Sep 17 00:00:00 2001 From: Lee Byron Date: Mon, 6 Feb 2017 20:06:09 -0800 Subject: [PATCH 2/3] Use a Map for original indices --- src/utilities/separateOperations.js | 43 ++++++++++++++--------------- 1 file changed, 21 insertions(+), 22 deletions(-) diff --git a/src/utilities/separateOperations.js b/src/utilities/separateOperations.js index 6b2208602f..087d9c2f3e 100644 --- a/src/utilities/separateOperations.js +++ b/src/utilities/separateOperations.js @@ -11,7 +11,6 @@ import { visit } from '../language/visitor'; import type { DocumentNode, - FragmentDefinitionNode, OperationDefinitionNode, } from '../language/ast'; @@ -25,22 +24,24 @@ export function separateOperations( documentAST: DocumentNode ): { [operationName: string]: DocumentNode } { - const definitions: DefinitionMap = Object.create(null); + const operations = []; + const fragments = Object.create(null); + const positions = new Map(); const depGraph: DepGraph = Object.create(null); let fromName; let idx = 0; - // Populate the list of definitions and build a dependency graph. + // Populate metadata and build a dependency graph. visit(documentAST, { OperationDefinition(node) { fromName = opName(node); - definitions[fromName] = { idx, node }; - ++idx; + operations.push(node); + positions.set(node, idx++); }, FragmentDefinition(node) { fromName = node.name.value; - definitions[fromName] = { idx, node }; - ++idx; + fragments[fromName] = node; + positions.set(node, idx++); }, FragmentSpread(node) { const toName = node.name.value; @@ -52,32 +53,30 @@ export function separateOperations( // For each operation, produce a new synthesized AST which includes only what // is necessary for completing that operation. const separatedDocumentASTs = Object.create(null); - const operationNames = Object.keys(definitions).filter(defName => - definitions[defName].node.kind === 'OperationDefinition' - ); - operationNames.forEach(operationName => { + operations.forEach(operation => { + const operationName = opName(operation); const dependencies = Object.create(null); collectTransitiveDependencies(dependencies, depGraph, operationName); - dependencies[operationName] = true; + + // The list of definition nodes to be included for this operation, sorted + // to retain the same order as the original document. + const definitions = [ operation ]; + Object.keys(dependencies).forEach(name => { + definitions.push(fragments[name]) + }); + definitions.sort( + (n1, n2) => (positions.get(n1) || 0) - (positions.get(n2) || 0) + ); separatedDocumentASTs[operationName] = { kind: 'Document', - definitions: Object.keys(dependencies) - .map(defName => definitions[defName]) - .sort((def1, def2) => def1.idx - def2.idx) - .map(def => def.node) + definitions }; }); return separatedDocumentASTs; } -type DefinitionMap = { - [defName: string]: { - idx: number, - node: OperationDefinitionNode | FragmentDefinitionNode - } -}; type DepGraph = {[from: string]: {[to: string]: boolean}}; // Provides the empty string for anonymous operations. From f8010893ea49efa400e3332fc591f7e4d6aa5840 Mon Sep 17 00:00:00 2001 From: Lee Byron Date: Mon, 6 Feb 2017 20:16:27 -0800 Subject: [PATCH 3/3] semi --- src/utilities/separateOperations.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utilities/separateOperations.js b/src/utilities/separateOperations.js index 087d9c2f3e..e103e8dd09 100644 --- a/src/utilities/separateOperations.js +++ b/src/utilities/separateOperations.js @@ -62,7 +62,7 @@ export function separateOperations( // to retain the same order as the original document. const definitions = [ operation ]; Object.keys(dependencies).forEach(name => { - definitions.push(fragments[name]) + definitions.push(fragments[name]); }); definitions.sort( (n1, n2) => (positions.get(n1) || 0) - (positions.get(n2) || 0)