Type-graphql: FieldResolver is too slow

[sixmen]

Is your feature request related to a problem? Please describe.
This is an advanced topic of #254

If you use FieldResolver, execution needs more time.

import 'reflect-metadata';
import { buildSchemaSync, Field, FieldResolver, ObjectType, Query, Resolver, Root } from 'type-graphql';
import { graphql } from 'graphql';

@ObjectType({ simpleResolvers: true })
class Item {
  @Field()
  a: number;
  @Field()
  b: number;
  @Field()
  c: number;
  @Field()
  d: number;
  @Field()
  e: number;
}

@Resolver(Item)
class ItemResolver {
  @Query(() => [Item])
  items() {
    return new Array(5000).fill(0).map((v, i) => ({ a: i, b: i, c: i, d: i, e: i }))
  }

  @FieldResolver()
  a(@Root() source) {
    return source.a;
  }

  @FieldResolver()
  b(@Root() source) {
    return source.b;
  }

  @FieldResolver()
  c(@Root() source) {
    return source.c;
  }

  @FieldResolver()
  d(@Root() source) {
    return source.d;
  }

  @FieldResolver()
  e(@Root() source) {
    return source.e;
  }
}

const schema = buildSchemaSync({
  resolvers: [ItemResolver]
});

const start = Date.now();
graphql(schema, '{ items { a b c d e } }')
  .then((r) => {
    console.log(r.data!.items.length);
    console.log(r.data!.items[30].c);
    console.log('time =', Date.now() - start);
  });

Describe the solution you'd like
I didn't think whether some option likes simple is a solution or not.

But in the end, some optimization technique applied in graphql-js may be needed, I think.

For example,

    if (isPromise(result)) {
      return result.then(undefined, error => {
        exeContext.errors.push(error);
        return Promise.resolve(null);
      });
    }
    return result;

(from https://github.com/graphql/graphql-js/blob/master/src/execution/execute.js)

In graphql-js, those two declarations become difference result.

const Item = new GraphQLObjectType({
  name: 'Item',
  fields: {
    a: { type: GraphQLString, resolve: (s) => s.a },
  }
});

const Item = new GraphQLObjectType({
  name: 'Item',
  fields: {
    a: { type: GraphQLString, resolve: async (s) => s.a },
  }
});

[MichalLytek]

The problem here is not about promises - it's a really small overhead that's hard to measure.

The real cause is that field resolvers are a normal resolvers - they have injected transformed args and other params, you can have custom decorators, etc. So they are not generated as root[field] but a normal class method call, so we also need resolve the dependencies, get an instance from container, etc.

@FieldResolver are designed to work with "rich resolvers" that e.g. resolve relation between tables in database. If you need something like this:

  @FieldResolver()
  a(@Root() source) {
    return source.a;
  }

It is better to use a getter that is called just like the simple resolver:

  @Field({ name: "a" })
  get getA() {
    return this.a;
  }

[sixmen]

It is a good idea to use getter instead of FieldResolver.
But I reminded the original issue of mine, and it was an async resolver. So it it not helpful.

Anyway, getParams for '@Root' may be the cause of the performance issue.
So, I think it is still useful to optimize Promises.

I tried to optimize by myself, and make a PR #488. Please check it.
In my test, the performance is improved dramatically. (maybe no need for simple option?)

[MichalLytek]

From code readability and performance perspective, I don't like to mix the sync and async stuff just for some percent of use cases that use field resolver for simple sync access.

I think that all we can think of now is to add a { raw: true } support for full resolvers like @Query or @FieldResolver that will skip all the TypeGraphQL things and executes using a standard (root, args, ctx, info) signature.
This will allow to "fallback" into almost the plain graphql-js execution when someone needs that.

[benawad]

I would love to do { raw: true } at a global level

[MichalLytek]

{ raw: true } at a global level

@benawad
So what do you need TypeGraphQL for? Maybe #339 (https://github.com/BramKaashoek/typescript-typedefs) fits you better?

[benawad]

@MichalLytek I still like writing the resolvers in the TypeGraphQL style, I'd just like to have a raw option if I don't need to use middleware/validation/etc

[MichalLytek]

For now we will just merge #488 - further optimization will be applied on vNext that will aim to have as minimal overhead as possible:
https://github.com/MichalLytek/type-graphql/tree/vNext
https://github.com/MichalLytek/type-graphql/projects/2

[benawad]

@MichalLytek Awesome, I'm looking forward to that!

Are you open to { simpleResolvers: true } at a global level?

[MichalLytek]

Are you open to { simpleResolvers: true } at a global level?

With #488 if you don't use middlewares nor auth checker, there's no performance penalty - see the PR and the benchmarks results.