Skip to content
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

Make gql js faster #4305

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions benchmark/fixtures.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,11 @@ exports.bigSchemaSDL = fs.readFileSync(
'utf8',
);

exports.bigDocument = fs.readFileSync(
path.join(__dirname, 'kitchen-sink.graphql'),
'utf8',
);

exports.bigSchemaIntrospectionResult = JSON.parse(
fs.readFileSync(path.join(__dirname, 'github-schema.json'), 'utf8'),
);
69 changes: 69 additions & 0 deletions benchmark/kitchen-sink.graphql
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
# Copyright (c) 2015-present, Facebook, Inc.
#
# This source code is licensed under the MIT license found in the
# LICENSE file in the root directory of this source tree.

query queryName($foo: ComplexType, $site: Site = MOBILE) @onQuery {
whoever123is: node(id: [123, 456]) {
id
... on User @onInlineFragment {
field2 {
id
alias: field1(first: 10, after: $foo) @include(if: $foo) {
id
...frag @onFragmentSpread
}
}
}
... @skip(unless: $foo) {
id
}
... {
id
}
}
}

mutation likeStory @onMutation {
like(story: 123) @onField {
story {
id @onField
}
}
}

subscription StoryLikeSubscription($input: StoryLikeSubscribeInput)
@onSubscription {
storyLikeSubscribe(input: $input) {
story {
likers {
count
}
likeSentence {
text
}
}
}
}

fragment frag on Friend @onFragmentDefinition {
foo(
size: $site
bar: 12
obj: {
key: "value"
block: """
block string uses \"""
"""
}
)
}

query teeny {
unnamed(truthy: true, falsey: false, nullish: null)
query
}

query tiny {
__typename
}
16 changes: 16 additions & 0 deletions benchmark/printer-benchmark.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
'use strict';

const { parse } = require('graphql/language/parser.js');
const { print } = require('graphql/language/printer.js');

const { bigDocument } = require('./fixtures.js');

const document = parse(bigDocument);

module.exports = {
name: 'Print kitchen-sink query',
count: 1000,
measure() {
print(document);
},
};
1 change: 1 addition & 0 deletions cspell.yml
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ words:
# TODO: contribute upstream
- deno
- codecov
- falsey

# Website tech
- Nextra
Expand Down
85 changes: 61 additions & 24 deletions src/language/printer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,10 @@ const printDocASTReducer: ASTReducer<string> = {
OperationDefinition: {
leave(node) {
const varDefs = wrap('(', join(node.variableDefinitions, ', '), ')');
const prefix = join(
const prefix = maybeJoin(
[
node.operation,
join([node.name, varDefs]),
maybeJoin([node.name, varDefs]),
join(node.directives, ' '),
],
' ',
Expand Down Expand Up @@ -63,7 +63,7 @@ const printDocASTReducer: ASTReducer<string> = {
argsLine = prefix + wrap('(\n', indent(join(args, '\n')), '\n)');
}

return join([argsLine, join(directives, ' '), selectionSet], ' ');
return maybeJoin([argsLine, join(directives, ' '), selectionSet], ' ');
},
},

Expand All @@ -78,7 +78,7 @@ const printDocASTReducer: ASTReducer<string> = {

InlineFragment: {
leave: ({ typeCondition, directives, selectionSet }) =>
join(
maybeJoin(
[
'...',
wrap('on ', typeCondition),
Expand Down Expand Up @@ -137,7 +137,7 @@ const printDocASTReducer: ASTReducer<string> = {
SchemaDefinition: {
leave: ({ description, directives, operationTypes }) =>
wrap('', description, '\n') +
join(['schema', join(directives, ' '), block(operationTypes)], ' '),
maybeJoin(['schema', join(directives, ' '), block(operationTypes)], ' '),
},

OperationTypeDefinition: {
Expand All @@ -147,13 +147,13 @@ const printDocASTReducer: ASTReducer<string> = {
ScalarTypeDefinition: {
leave: ({ description, name, directives }) =>
wrap('', description, '\n') +
join(['scalar', name, join(directives, ' ')], ' '),
maybeJoin(['scalar', name, join(directives, ' ')], ' '),
},

ObjectTypeDefinition: {
leave: ({ description, name, interfaces, directives, fields }) =>
wrap('', description, '\n') +
join(
maybeJoin(
[
'type',
name,
Expand All @@ -180,7 +180,7 @@ const printDocASTReducer: ASTReducer<string> = {
InputValueDefinition: {
leave: ({ description, name, type, defaultValue, directives }) =>
wrap('', description, '\n') +
join(
maybeJoin(
[name + ': ' + type, wrap('= ', defaultValue), join(directives, ' ')],
' ',
),
Expand All @@ -189,7 +189,7 @@ const printDocASTReducer: ASTReducer<string> = {
InterfaceTypeDefinition: {
leave: ({ description, name, interfaces, directives, fields }) =>
wrap('', description, '\n') +
join(
maybeJoin(
[
'interface',
name,
Expand All @@ -204,7 +204,7 @@ const printDocASTReducer: ASTReducer<string> = {
UnionTypeDefinition: {
leave: ({ description, name, directives, types }) =>
wrap('', description, '\n') +
join(
maybeJoin(
['union', name, join(directives, ' '), wrap('= ', join(types, ' | '))],
' ',
),
Expand All @@ -213,18 +213,19 @@ const printDocASTReducer: ASTReducer<string> = {
EnumTypeDefinition: {
leave: ({ description, name, directives, values }) =>
wrap('', description, '\n') +
join(['enum', name, join(directives, ' '), block(values)], ' '),
maybeJoin(['enum', name, join(directives, ' '), block(values)], ' '),
},

EnumValueDefinition: {
leave: ({ description, name, directives }) =>
wrap('', description, '\n') + join([name, join(directives, ' ')], ' '),
wrap('', description, '\n') +
maybeJoin([name, join(directives, ' ')], ' '),
},

InputObjectTypeDefinition: {
leave: ({ description, name, directives, fields }) =>
wrap('', description, '\n') +
join(['input', name, join(directives, ' '), block(fields)], ' '),
maybeJoin(['input', name, join(directives, ' '), block(fields)], ' '),
},

DirectiveDefinition: {
Expand All @@ -242,20 +243,20 @@ const printDocASTReducer: ASTReducer<string> = {

SchemaExtension: {
leave: ({ directives, operationTypes }) =>
join(
maybeJoin(
['extend schema', join(directives, ' '), block(operationTypes)],
' ',
),
},

ScalarTypeExtension: {
leave: ({ name, directives }) =>
join(['extend scalar', name, join(directives, ' ')], ' '),
maybeJoin(['extend scalar', name, join(directives, ' ')], ' '),
},

ObjectTypeExtension: {
leave: ({ name, interfaces, directives, fields }) =>
join(
maybeJoin(
[
'extend type',
name,
Expand All @@ -269,7 +270,7 @@ const printDocASTReducer: ASTReducer<string> = {

InterfaceTypeExtension: {
leave: ({ name, interfaces, directives, fields }) =>
join(
maybeJoin(
[
'extend interface',
name,
Expand All @@ -283,7 +284,7 @@ const printDocASTReducer: ASTReducer<string> = {

UnionTypeExtension: {
leave: ({ name, directives, types }) =>
join(
maybeJoin(
[
'extend union',
name,
Expand All @@ -296,30 +297,66 @@ const printDocASTReducer: ASTReducer<string> = {

EnumTypeExtension: {
leave: ({ name, directives, values }) =>
join(['extend enum', name, join(directives, ' '), block(values)], ' '),
maybeJoin(
['extend enum', name, join(directives, ' '), block(values)],
' ',
),
},

InputObjectTypeExtension: {
leave: ({ name, directives, fields }) =>
join(['extend input', name, join(directives, ' '), block(fields)], ' '),
maybeJoin(
['extend input', name, join(directives, ' '), block(fields)],
' ',
),
},
};

/**
* Given maybeArray, print an empty string if it is null or empty, otherwise
* print all items together separated by separator if provided
*/
function join(
maybeArray: Maybe<ReadonlyArray<string | undefined>>,
function maybeJoin(
maybeArray: ReadonlyArray<string | undefined>,
separator = '',
): string {
return maybeArray?.filter((x) => x).join(separator) ?? '';
const list = maybeArray.filter((x) => x);
const listLength = list.length;
let result = '';
for (let i = 0; i < listLength; i++) {
if (i === listLength - 1) {
result += list[i];
} else {
result += list[i] + separator;
}
}
return result;
}

function join(
list: ReadonlyArray<string> | undefined,
separator: string,
): string {
if (!list) {
return '';
}
const listLength = list.length;
let result = '';
for (let i = 0; i < listLength; i++) {
if (i === listLength - 1) {
result += list[i];
} else {
result += list[i] + separator;
}
}

return result;
}

/**
* Given array, print each item on its own line, wrapped in an indented `{ }` block.
*/
function block(array: Maybe<ReadonlyArray<string | undefined>>): string {
function block(array: ReadonlyArray<string> | undefined): string {
return wrap('{\n', indent(join(array, '\n')), '\n}');
}

Expand Down
13 changes: 6 additions & 7 deletions src/language/visitor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ type KindVisitor = {
| EnterLeaveVisitor<NodeT>;
};

const ALL_KINDS = Object.values(Kind);

interface EnterLeaveVisitor<TVisitedNode extends ASTNode> {
readonly enter?: ASTVisitFn<TVisitedNode>;
readonly leave?: ASTVisitFn<TVisitedNode>;
Expand Down Expand Up @@ -182,7 +184,7 @@ export function visit(
visitorKeys: ASTVisitorKeyMap = QueryDocumentKeys,
): any {
const enterLeaveMap = new Map<Kind, EnterLeaveVisitor<ASTNode>>();
for (const kind of Object.values(Kind)) {
for (const kind of ALL_KINDS) {
enterLeaveMap.set(kind, getEnterLeaveForKind(visitor, kind));
}

Expand All @@ -202,7 +204,7 @@ export function visit(
do {
index++;
const isLeaving = index === keys.length;
const isEdited = isLeaving && edits.length !== 0;
const isEdited = edits.length !== 0;
if (isLeaving) {
key = ancestors.length === 0 ? undefined : path[path.length - 1];
node = parent;
Expand All @@ -222,10 +224,7 @@ export function visit(
}
}
} else {
node = Object.defineProperties(
Copy link
Member Author

@JoviDeCroock JoviDeCroock Nov 28, 2024

Choose a reason for hiding this comment

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

This is a massive deopt, maybe we could keep this in by i.e. rather than copying the node just returning the string if that's what the edit is symbolising

{},
Object.getOwnPropertyDescriptors(node),
);
node = { ...node };
for (const [editKey, editValue] of edits) {
node[editKey] = editValue;
}
Expand Down Expand Up @@ -267,7 +266,7 @@ export function visit(
} else if (result !== undefined) {
edits.push([key, result]);
if (!isLeaving) {
if (isNode(result)) {
if (isNode(result) && result !== node) {
node = result;
} else {
path.pop();
Expand Down