Improving Performances

[benjamindulau]

I try to replace Youshido implementation because of the its catastrophic performance issues. It consumes a lot of CPU because of the way it resolves fields recursively.

So I rewrote a portion of our GraphQL using digiaonline library and out-of-box, without any tweak (I just implemented it by the book following the documentation), it's even worse.

For the exact same GraphQL query:

Youshido:

Digiaonline:

Here are some profiling extracts:

The number of calls just blew my mind :D

Also, I'm not sure the Deferred system is working correctly, Promises are resolved a lot more that they should be. For instance Curl/Guzzle should be called only twice in this example but was called 143 times (like I didn't have deferred calls at all). Maybe that could be related.

Here is the code example related to the deferred data loaders:

public function resolveSelections($rootValue, array $arguments, $context = null, ?ResolveInfo $info = null): Promise
{
    return new Promise($this->findActiveSelectionsForOfferBuffer->add($offerId));
}

Buffer:

<?php
use ST\CQRS\DAO\Elasticsearch\ResultSet;
use ST\CQRS\Projection\Dictionary\Dictionary;

abstract class AbstractSearchableBuffer
{
    /**
     * Offer IDs
     *
     * @var array
     */
    protected $ids = [];

    /**
     * Per Offer Id results.
     *
     * offerId -> ResultSet
     *
     * @var array|ResultSet[]
     */
    protected $results;

    /**
     * @var boolean
     */
    protected $cacheEnabled;

    protected $dictionary;

    public function __construct(Dictionary $dictionary, $cacheEnabled = true)
    {
        $this->dictionary   = $dictionary;
        $this->ids          = [];
        $this->results      = [];

        $this->cacheEnabled = $cacheEnabled;
    }

    public function add(string $id, array $params = []): callable
    {
        $resolver = function (callable $resolve, callable $reject) use ($id) {
            $resultSet = $this->resolveQuery($id);
            $dictionaryResults = $this->dictionary->getMany(array_column($resultSet->items, 'id'));

            $resolve($dictionaryResults);
        };

        // id have been already resolved
        if ($this->cacheEnabled && array_key_exists($id, $this->results)) {
            return $resolver;
        }

        // id will be resolved
        $this->ids[$id] = $id;

        return $resolver;
    }

    protected function resolveQuery($id): ResultSet
    {
        // this id has been already resolved.
        if ($this->cacheEnabled && array_key_exists($id, $this->results)) {
            return $this->results[$id];
        }

        $resolvedIds = $this->findMany($this->ids);

        // map queries resolved value(s)
        $partialResults = array_map(function ($id) use ($resolvedIds) {
            return $resolvedIds[$id];
        }, $this->ids);

        $this->ids = [];
        $this->results = array_merge($this->results, $partialResults);

        return $this->results[$id];
    }

    /**
     * @param array $ids
     * @return ResultSet[] key => id
     */
    abstract protected function findMany(array $ids): array;
}

<?php

use ST\CQRS\Projection\Dictionary\Dictionary;
use ST\Projection\Frontoffice\Searchable\DAO\SelectionDAO;

class FindActiveSelectionsForOfferBuffer extends AbstractSearchableBuffer
{
    private $selectionDAO;

    public function __construct(SelectionDAO $selectionDAO, Dictionary $dictionary, $cacheEnabled = true)
    {
        $this->selectionDAO = $selectionDAO;

        parent::__construct($dictionary, $cacheEnabled);
    }

    protected function findMany(array $ids): array
    {
        return $this->selectionDAO->findActiveSelectionsForOffers($ids, 10);
    }
}

Do you plan on adding some caching strategy and/or recommandations?

[crisu83]

We are aware of this and we plan to rewrite most of the execution logic as soon as we can find some time. @hungneox wrote the execution logic so I’ve assigned this issue to him.

[benjamindulau]

@crisu83 Ok. Right now we really can't use it in production because of that but we'll keep a look on future releases. We really like the approach you took and the fact that's it is much more straightforward to implement than other libraries out there. Maintaining the graphql schema using SDL really makes sense for a lot of reasons and I don't understand why other libraries don't advocate that...

[Jalle19]

Are you trying to use deferred resolvers? I have a feeling you've made a mistake if it doesn't work, we have a couple of tests that (at least they're supposed to) verify that the promise logic works correctly.

[benjamindulau]

@Jalle19 Yeah. I just adapted the code I had for Youshido's deferred resolvers. They were working great before... You can see some part of my code in the issue above, tell me if you need more! Maybe I didn't quite get how the Promise works :) (but maybe I did :p)

[Jalle19]

@benjamindulau did you check the example in the README: https://github.com/digiaonline/graphql-php#the-n1-problem ?

[Jalle19]

I haven't used deferred resolvers in Youshido's implementation so I can't comment on how similar out approach is

[benjamindulau]

@Jalle19 Yes I did check the example and the test file. My Buffer is very similar (but more complexe) to the one in the test file. The only difference with the youshido's implementation here is that you must return a Promise and invoke the resolve function instead of returning their DeferredResolver object.
I'll go deeply into this so I can figure out if I'm missing something. I'll Keep you in touch.

[Jalle19]

@benjamindulau can you try the latest version (v1.0.3), I fixed a couple of performance issues.

[Jalle19]

Especially the 51 754 calls to GraphQL::make() should now be drastically lower

[benjamindulau]

@Jalle19 Sorry I got really busy for the last 2 weeks. I plan on testing again next week. I'll post my results here

[crisu83]

@benjamindulau could you possibly give this a new try after we merge #345?

[hugovk]

Thanks for the report, however we're going to archive this repo soon as we're no longer maintaining it: #359