Skip to content

Commit

Permalink
Improve performance of printer
Browse files Browse the repository at this point in the history
  • Loading branch information
JoviDeCroock committed Nov 28, 2024
1 parent c079ae3 commit 84689d1
Show file tree
Hide file tree
Showing 6 changed files with 165 additions and 52 deletions.
2 changes: 1 addition & 1 deletion benchmark/benchmark.js
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ function maxBy(array, fn) {

// Prepare all revisions and run benchmarks matching a pattern against them.
async function runBenchmarks(benchmarks, benchmarkProjects) {
for (const benchmark of benchmarks) {
for (const benchmark of benchmarks.filter(x => x.includes('visit-') || x.includes('printer-'))) {
const results = [];
for (let i = 0; i < benchmarkProjects.length; ++i) {
const { revision, projectPath } = benchmarkProjects[i];
Expand Down
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
}
15 changes: 15 additions & 0 deletions benchmark/printer-benchmark.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
'use strict';

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

Check failure on line 4 in benchmark/printer-benchmark.js

View workflow job for this annotation

GitHub Actions / ci / Lint source files

There should be at least one empty line between import groups
const { bigDocument } = require('./fixtures')

Check failure on line 5 in benchmark/printer-benchmark.js

View workflow job for this annotation

GitHub Actions / ci / Lint source files

Missing file extension "js" for "./fixtures"

const document = parse(bigDocument)

module.exports = {
name: 'Print ktichen-sink query',
count: 1000,
measure() {
print(document);
},
};
106 changes: 65 additions & 41 deletions src/language/printer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,17 +23,18 @@ const printDocASTReducer: ASTReducer<string> = {
// Document

Document: {
leave: (node) => join(node.definitions, '\n\n'),
leave: (node) => truthyJoin(node.definitions, '\n\n'),
},

OperationDefinition: {
leave(node) {
const varDefs = wrap('(', join(node.variableDefinitions, ', '), ')');
const varDefs = wrap('(', truthyJoin(node.variableDefinitions, ', '), ')');
const prefix = join(
[
node.operation,
// TODO: optimize
join([node.name, varDefs]),
join(node.directives, ' '),
truthyJoin(node.directives, ' '),
],
' ',
);
Expand All @@ -50,20 +51,20 @@ const printDocASTReducer: ASTReducer<string> = {
': ' +
type +
wrap(' = ', defaultValue) +
wrap(' ', join(directives, ' ')),
wrap(' ', truthyJoin(directives, ' ')),
},
SelectionSet: { leave: ({ selections }) => block(selections) },

Field: {
leave({ alias, name, arguments: args, directives, selectionSet }) {
const prefix = wrap('', alias, ': ') + name;
let argsLine = prefix + wrap('(', join(args, ', '), ')');
let argsLine = prefix + wrap('(', truthyJoin(args, ', '), ')');

if (argsLine.length > MAX_LINE_LENGTH) {
argsLine = prefix + wrap('(\n', indent(join(args, '\n')), '\n)');
argsLine = prefix + wrap('(\n', indent(truthyJoin(args, '\n')), '\n)');
}

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

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

FragmentSpread: {
leave: ({ name, directives }) =>
'...' + name + wrap(' ', join(directives, ' ')),
'...' + name + wrap(' ', truthyJoin(directives, ' ')),
},

InlineFragment: {
Expand All @@ -82,7 +83,7 @@ const printDocASTReducer: ASTReducer<string> = {
[
'...',
wrap('on ', typeCondition),
join(directives, ' '),
truthyJoin(directives, ' '),
selectionSet,
],
' ',
Expand All @@ -99,8 +100,8 @@ const printDocASTReducer: ASTReducer<string> = {
}) =>
// Note: fragment variable definitions are experimental and may be changed
// or removed in the future.
`fragment ${name}${wrap('(', join(variableDefinitions, ', '), ')')} ` +
`on ${typeCondition} ${wrap('', join(directives, ' '), ' ')}` +
`fragment ${name}${wrap('(', truthyJoin(variableDefinitions, ', '), ')')} ` +
`on ${typeCondition} ${wrap('', truthyJoin(directives, ' '), ' ')}` +
selectionSet,
},

Expand All @@ -115,15 +116,15 @@ const printDocASTReducer: ASTReducer<string> = {
BooleanValue: { leave: ({ value }) => (value ? 'true' : 'false') },
NullValue: { leave: () => 'null' },
EnumValue: { leave: ({ value }) => value },
ListValue: { leave: ({ values }) => '[' + join(values, ', ') + ']' },
ObjectValue: { leave: ({ fields }) => '{' + join(fields, ', ') + '}' },
ListValue: { leave: ({ values }) => '[' + truthyJoin(values, ', ') + ']' },
ObjectValue: { leave: ({ fields }) => '{' + truthyJoin(fields, ', ') + '}' },
ObjectField: { leave: ({ name, value }) => name + ': ' + value },

// Directive

Directive: {
leave: ({ name, arguments: args }) =>
'@' + name + wrap('(', join(args, ', '), ')'),
'@' + name + wrap('(', truthyJoin(args, ', '), ')'),
},

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

ObjectTypeDefinition: {
Expand All @@ -157,8 +158,8 @@ const printDocASTReducer: ASTReducer<string> = {
[
'type',
name,
wrap('implements ', join(interfaces, ' & ')),
join(directives, ' '),
wrap('implements ', truthyJoin(interfaces, ' & ')),
truthyJoin(directives, ' '),
block(fields),
],
' ',
Expand All @@ -170,18 +171,18 @@ const printDocASTReducer: ASTReducer<string> = {
wrap('', description, '\n') +
name +
(hasMultilineItems(args)
? wrap('(\n', indent(join(args, '\n')), '\n)')
: wrap('(', join(args, ', '), ')')) +
? wrap('(\n', indent(truthyJoin(args, '\n')), '\n)')
: wrap('(', truthyJoin(args, ', '), ')')) +
': ' +
type +
wrap(' ', join(directives, ' ')),
wrap(' ', truthyJoin(directives, ' ')),
},

InputValueDefinition: {
leave: ({ description, name, type, defaultValue, directives }) =>
wrap('', description, '\n') +
join(
[name + ': ' + type, wrap('= ', defaultValue), join(directives, ' ')],
[name + ': ' + type, wrap('= ', defaultValue), truthyJoin(directives, ' ')],
' ',
),
},
Expand All @@ -193,8 +194,8 @@ const printDocASTReducer: ASTReducer<string> = {
[
'interface',
name,
wrap('implements ', join(interfaces, ' & ')),
join(directives, ' '),
wrap('implements ', truthyJoin(interfaces, ' & ')),
truthyJoin(directives, ' '),
block(fields),
],
' ',
Expand All @@ -205,26 +206,26 @@ const printDocASTReducer: ASTReducer<string> = {
leave: ({ description, name, directives, types }) =>
wrap('', description, '\n') +
join(
['union', name, join(directives, ' '), wrap('= ', join(types, ' | '))],
['union', name, truthyJoin(directives, ' '), wrap('= ', truthyJoin(types, ' | '))],
' ',
),
},

EnumTypeDefinition: {
leave: ({ description, name, directives, values }) =>
wrap('', description, '\n') +
join(['enum', name, join(directives, ' '), block(values)], ' '),
join(['enum', name, truthyJoin(directives, ' '), block(values)], ' '),
},

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

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

DirectiveDefinition: {
Expand All @@ -233,24 +234,24 @@ const printDocASTReducer: ASTReducer<string> = {
'directive @' +
name +
(hasMultilineItems(args)
? wrap('(\n', indent(join(args, '\n')), '\n)')
: wrap('(', join(args, ', '), ')')) +
? wrap('(\n', indent(truthyJoin(args, '\n')), '\n)')
: wrap('(', truthyJoin(args, ', '), ')')) +
(repeatable ? ' repeatable' : '') +
' on ' +
join(locations, ' | '),
truthyJoin(locations, ' | '),
},

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

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

ObjectTypeExtension: {
Expand All @@ -259,8 +260,8 @@ const printDocASTReducer: ASTReducer<string> = {
[
'extend type',
name,
wrap('implements ', join(interfaces, ' & ')),
join(directives, ' '),
wrap('implements ', truthyJoin(interfaces, ' & ')),
truthyJoin(directives, ' '),
block(fields),
],
' ',
Expand All @@ -273,8 +274,8 @@ const printDocASTReducer: ASTReducer<string> = {
[
'extend interface',
name,
wrap('implements ', join(interfaces, ' & ')),
join(directives, ' '),
wrap('implements ', truthyJoin(interfaces, ' & ')),
truthyJoin(directives, ' '),
block(fields),
],
' ',
Expand All @@ -287,21 +288,21 @@ const printDocASTReducer: ASTReducer<string> = {
[
'extend union',
name,
join(directives, ' '),
wrap('= ', join(types, ' | ')),
truthyJoin(directives, ' '),
wrap('= ', truthyJoin(types, ' | ')),
],
' ',
),
},

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

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

Expand All @@ -313,7 +314,30 @@ function join(
maybeArray: Maybe<ReadonlyArray<string | undefined>>,
separator = '',
): string {
return maybeArray?.filter((x) => x).join(separator) ?? '';
if (!maybeArray) return ''

Check failure on line 317 in src/language/printer.ts

View workflow job for this annotation

GitHub Actions / ci / Lint source files

Expected { after 'if' condition

const list = maybeArray.filter((x) => x);
const listLength = list.length;
let result = '';
for (let i = 0; i < listLength; i++) {
if (i === listLength - 1) return result + list[i];

Check failure on line 323 in src/language/printer.ts

View workflow job for this annotation

GitHub Actions / ci / Lint source files

Expected { after 'if' condition
else result += list[i] + separator;

Check failure on line 324 in src/language/printer.ts

View workflow job for this annotation

GitHub Actions / ci / Lint source files

Expected { after 'else'

Check failure on line 324 in src/language/printer.ts

View workflow job for this annotation

GitHub Actions / ci / Lint source files

Unnecessary 'else' after 'return'
}
return result
}

function truthyJoin(
list: ReadonlyArray<string> | undefined,
separator: string,
): string {
if (!list) return ''

Check failure on line 333 in src/language/printer.ts

View workflow job for this annotation

GitHub Actions / ci / Lint source files

Expected { after 'if' condition
const listLength = list.length;
let result = '';
for (let i = 0; i < listLength; i++) {
if (i === listLength - 1) return result + list[i];

Check failure on line 337 in src/language/printer.ts

View workflow job for this annotation

GitHub Actions / ci / Lint source files

Expected { after 'if' condition
else result += list[i] + separator;

Check failure on line 338 in src/language/printer.ts

View workflow job for this annotation

GitHub Actions / ci / Lint source files

Expected { after 'else'

Check failure on line 338 in src/language/printer.ts

View workflow job for this annotation

GitHub Actions / ci / Lint source files

Unnecessary 'else' after 'return'
}
return result
}

/**
Expand Down
Loading

0 comments on commit 84689d1

Please sign in to comment.