`DirectiveLocator:associated()` performance

[LastDragon-ru]

We intensively use directives and some of our queries return hundreds and thousands of objects. I've found that each call of DirectiveLocator::associated($node) create new instances of directives and this lead to (for 500 objects)

In PHP 8.0 it is possible to use WeakMap to cache directive instances and we will get:

<?php declare(strict_types = 1);

namespace App\GraphQL;

use App\GraphQL\Extensions\Lighthouse\DirectiveLocator;
use App\GraphQL\Extensions\Lighthouse\Directives\ValidatorDirective;
use Illuminate\Contracts\Events\Dispatcher;
use Illuminate\Support\ServiceProvider;
use Nuwave\Lighthouse\Schema\DirectiveLocator as LighthouseDirectiveLocator;
use Nuwave\Lighthouse\Validation\ValidatorDirective as LighthouseValidatorDirective;

class Provider extends ServiceProvider {
    // <editor-fold desc="Register">
    // =========================================================================
    public function register(): void {
        parent::register();

        $this->app->singleton(LighthouseDirectiveLocator::class, DirectiveLocator::class);
        $this->app->bind(LighthouseValidatorDirective::class, ValidatorDirective::class);

        $this->booting(static function (Dispatcher $dispatcher, LighthouseDirectiveLocator $locator): void {
            $dispatcher->subscribe($locator);
        });
    }
}

<?php declare(strict_types = 1);

namespace App\GraphQL\Extensions\Lighthouse;

use GraphQL\Language\AST\Node;
use Illuminate\Contracts\Events\Dispatcher;
use Illuminate\Support\Collection;
use Nuwave\Lighthouse\Events\ManipulateAST;
use Nuwave\Lighthouse\Events\StartExecution;
use Nuwave\Lighthouse\Schema\DirectiveLocator as LighthouseDirectiveLocator;
use WeakMap;

/**
 * @see https://github.com/nuwave/lighthouse/issues/2041
 */
class DirectiveLocator extends LighthouseDirectiveLocator {
    /**
     * @var \WeakMap<\GraphQL\Language\AST\Node,\Illuminate\Support\Collection<\Nuwave\Lighthouse\Support\Contracts\Directive>
     */
    private WeakMap $directives;

    private bool $execution = false;

    public function __construct(Dispatcher $eventsDispatcher) {
        parent::__construct($eventsDispatcher);

        $this->reset();
    }

    public function subscribe(Dispatcher $dispatcher): void {
        $dispatcher->listen(ManipulateAST::class, function (): void {
            $this->reset();
        });
        $dispatcher->listen(StartExecution::class, function (): void {
            $this->execution = true;
        });
    }

    protected function reset(): void {
        $this->directives = new WeakMap();
        $this->execution  = false;
    }

    public function associated(Node $node): Collection {
        // While AST Manipulation phase the node/directive/args can be changed
        // so we must not cache anything.
        if (!$this->execution) {
            return parent::associated($node);
        }

        // While Execution phase nothing can be changed (?), so
        if (!isset($this->directives[$node])) {
            $this->directives[$node] = parent::associated($node);
        }

        return $this->directives[$node];
    }
}

<?php declare(strict_types = 1);

namespace App\GraphQL\Extensions\Lighthouse\Directives;

use Nuwave\Lighthouse\Execution\Arguments\ArgumentSet;
use Nuwave\Lighthouse\Validation\ValidatorDirective as LighthouseValidatorDirective;

class ValidatorDirective extends LighthouseValidatorDirective {
    /**
     * @inheritDoc
     */
    public function setArgumentValue($argument): self {
        $result = parent::setArgumentValue($argument);

        if ($argument instanceof ArgumentSet) {
            $this->validator()->setArgs($argument);
        }

        return $result;
    }
}

Unfortunately, I have no idea (and time) how to fix it for PHP < 8.0

[LastDragon-ru]

I've found that cache should be disabled while the AST Manipulation phase (because directives/args can change), so the first post updated.

[LastDragon-ru]

I've also found that ValidatorDirective doesn't not call Validator::setArgs() after ValidatorDirective::setArgumentValue() call that is critical when directives cached. The first post updated.

[spawnia]

This change has the potential for a subtle breakage, exactly due to directives such as ValidatorDirective that have lingering state. The same might be true for custom directives.

The PHP compatiblity can be resolved by using SplObjectStorage. I would be happy about a pull request so I can benchmark this for our application.

[LastDragon-ru]

This change has the potential for a subtle breakage, exactly due to directives such as ValidatorDirective

Yep. Also, a bit related issue: BaseDirective::hydrate() doesn't reset cached args.

I would be happy about a pull request so I can benchmark this for our application.

In our real/live application, we have improvements of about ~6%. I will try to create PR next week, but no promises.