Move argumentSet/argumentSetFactory to a middleware

[k0ka]

**Are you willing to provide a PR for this issue or aid in developing it? ** yes

What problem does this feature proposal attempt to solve? I've pushed https://github.com/nuwave/lighthouse/pull/1697

I'm optimizing my GraphQL API server. The biggest bottlenecks I've found in lighthouse are global field middlewares. Here are test results with them on:

\Benchmarks\HugeResponseBench

    benchmark1..............................I9 [μ Mo]/r: 0.082 0.081 (s) [μSD μRSD]/r: 0.002s 2.44%
    benchmark100............................I9 [μ Mo]/r: 0.099 0.100 (s) [μSD μRSD]/r: 0.002s 2.16%
    benchmark10k............................I9 [μ Mo]/r: 0.856 0.853 (s) [μSD μRSD]/r: 0.019s 2.17%

and off (they are off in the PR):

\Benchmarks\HugeResponseBench

    benchmark1..............................I9 [μ Mo]/r: 0.070 0.069 (s) [μSD μRSD]/r: 0.001s 1.98%
    benchmark100............................I9 [μ Mo]/r: 0.078 0.078 (s) [μSD μRSD]/r: 0.001s 1.63%
    benchmark10k............................I9 [μ Mo]/r: 0.431 0.428 (s) [μSD μRSD]/r: 0.008s 1.85%

There is 2x difference for 10k fields and even for the simplest query which gains 15% performance boost. However, it's not a real problem as they can be easily turned off in the config.

The next evident bottleneck I've found is argumentSetFactory - https://github.com/nuwave/lighthouse/blob/master/src/Schema/Factories/FieldFactory.php#L90. Commenting this line increases speed by 20% in 10k scenario:

    benchmark1..............................I9 [μ Mo]/r: 0.068 0.068 (s) [μSD μRSD]/r: 0.002s 2.73%
    benchmark100............................I9 [μ Mo]/r: 0.077 0.076 (s) [μSD μRSD]/r: 0.001s 1.70%
    benchmark10k............................I9 [μ Mo]/r: 0.360 0.357 (s) [μSD μRSD]/r: 0.009s 2.43%

Which possible solutions should be considered?

I suggest rewriting this code as a field middleware that is enabled globally by default but can be disabled.

Most of fields are simple data that don't need any middlewares to be resolved. So it's a good point in the ability to disable them globally and fine tune for individual fields.

[spawnia]

We could add a method to field middleware directives to determine if it is required to be added to a given field and run this during schema compilation.

[k0ka]

Yes, that's a good idea.

SanitizeDirective and TransformArgsDirective can include themselves only into fields that have an arg with the ArgSanitizerDirective/ArgTransformerDirective directive.

We might also create ArgumentSetDirective which creates argumentSet and includes itself only when there are any other directives present in the field.

[spawnia]

I have come up with an optimization that should be both simple and efficient: Only run certain field middleware once for every usage of the field in the query. That makes it so the amount of executed field middleware only grow with the complexity and depth of the query and no longer with the amount of returned result.

[k0ka]

Yes, that would be great.

However, I suggest also offloading as many calculation to the AST creation time as we can. So I want SanitizeDirective to include itself only if ArgSanitizerDirective is attached to any of field arguments. It can be resolved during AST creation and won't change whichever query is.

There are some restrictions, that should be fulfilled to make this change non-BC breaking:

• SanitizeDirective must be on the field_middleware list.
• All directives from the field_middleware list which implement FieldMiddleware must be applied during execution time.

So I see two options how to implement it:

1. Rename SanitizeDirective to SanitizeDirectiveApplied. Create the new SanitizeDirective which will implement TypeManipulator. It will add SanitizeDirectiveApplied to required fields.
2. Add this logic to SanitizeDirective, but also add some interface or variable like ShouldntExecutedGlobally. So that we can skip this middleware on the field_middleware list during execution phase.

What do you think is better?