Memoize per-callable spec via PSR-16 and add sound fast-path to resolve()

[henriquemoody]

Deep object-graph construction through the resolver makes repeated ReflectionParameter introspection (getName/getType/isVariadic etc.) a bottleneck. Cache the per-callable spec -- name, non-builtin type, variadic flag, default-available flag -- in a PSR-16 cache supplied as an optional second constructor argument. The key is derived from the callable's stable identity (FQCN::method or fn:name); closures and invocable objects bypass the cache. Default values are deliberately excluded from the spec and fetched lazily only when the slow-path default branch executes, so PHP 8.1+ new Foo() defaults do not run prematurely.

A sound fast-path returns the arguments unchanged when the resolution would be a no-op: no named args, no variadic, positional count equals parameter count, and each positional already satisfies its parameter's type (or the container lacks that type). This avoids the loop and container lookups in the common DI case where the caller supplies all args in order. A naive count-only fast path is unsound because container injection shifts positionals.

The commit adds psr/simple-cache: ^3.0 as a runtime dependency and bundles Respect\Parameter\InMemoryCache, a zero-dependency array- backed PSR-16 implementation whose entries live for the cache instance's lifetime. Users get spec memoization out of the box by passing new InMemoryCache() as the second constructor argument, with no external cache package needed.

New test fixtures (TwoRequiredConsumer, ConsumerWithExpensiveDefault, ExpensiveDefaultService) and an ArrayCache fixture subclass exercise both fast-path branches, cache-sharing across reflection instances, lazy-default behavior, and the full PSR-16 contract.

[Copilot]

Pull request overview

This PR optimizes Respect\Parameter\Resolver::resolve() for DI-heavy workloads by caching reflection-derived parameter metadata per reflection (via WeakMap) and adding a “sound” fast path that returns arguments unchanged when resolution can be proven to be a no-op.

Changes:

• Cache per-parameter introspection (name, non-builtin type, variadic flag, default metadata) per ReflectionFunctionAbstract in a WeakMap.
• Add a tiered fast path that short-circuits resolution when all provided positionals are already aligned with the container/type expectations.
• Add unit tests and a new fixture to validate both the container-shift case and the aligned fast-path case.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File	Description
src/Resolver.php	Adds WeakMap caching of parameter specs and a sound fast path for no-op resolutions.
tests/unit/ResolverTest.php	Adds tests to ensure the fast path is sound (doesn’t break container injection shifting) and triggers when alignment is safe.
tests/fixtures/TwoRequiredConsumer.php	Introduces a fixture with two required parameters to exercise the new fast-path conditions.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 6 changed files in this pull request and generated 1 comment.

[henriquemoody]

This won't scale with union types. Maybe we should also do something for intersection types too, because it could be that only one service with a specific interface is in the container

[alganet]

The idea is sound. However, for the variadic case, which is the path that Validation regresses by using Parameter (allOf, oneOf, etc), this implementation is roughly 10% slower.

It shows gains of ~20% for bare container inject and 10% for the fast path, which is nice!

I believe the getDeclaringClass and md5 are potential issues (sometimes, computing the cache key is more expensive than just resolving the parameters).

If it were a clear win in all scenarios, this would be a no-brainer and I would accept it as it is. Considering that performance is the only deliverable of this PR, I see the variadic being slower as a thing we must improve upon before merging.

Here's a benchmark you can run to replicate my results:

bench.php

<?php

/*
 * SPDX-License-Identifier: ISC
 * SPDX-FileCopyrightText: (c) Respect Project Contributors
 */

declare(strict_types=1);

/**
 * Micro-benchmark for Respect\Parameter\Resolver.
 *
 * Times resolve() across a few representative scenarios. Can compare the
 * working-tree Resolver against any git ref so the effect of a change
 * (e.g. spec memoization) is visible in one run.
 *
 *   php bench.php                 # working tree vs HEAD~1
 *   php bench.php HEAD~1 HEAD~3   # compare those refs against the working tree
 *   php bench.php --iterations=500000
 *
 * Each git ref's src/ is materialized via `git show` into a temp dir under
 * the OS temp directory and removed when the run finishes; nothing is left
 * behind and no path outside the repo is hard-coded.
 */

namespace Respect\Parameter\Bench;

const SCENARIO_ITERATIONS = 200_000;
const TIMED_RUNS = 5;
const WARMUP = 1_000;

// ---------------------------------------------------------------------------
// Argument parsing
// ---------------------------------------------------------------------------

/** @return array{src: ?string, label: string, iterations: int, refs: list<string>} */
function parseArgs(array $argv): array
{
    $opts = ['src' => null, 'label' => 'working-tree', 'iterations' => SCENARIO_ITERATIONS, 'refs' => []];
    foreach (array_slice($argv, 1) as $arg) {
        if (str_starts_with($arg, '--src=')) {
            $opts['src'] = substr($arg, 6);
        } elseif (str_starts_with($arg, '--label=')) {
            $opts['label'] = substr($arg, 8);
        } elseif (str_starts_with($arg, '--iterations=')) {
            $opts['iterations'] = max(1, (int) substr($arg, 13));
        } elseif (!str_starts_with($arg, '--')) {
            $opts['refs'][] = $arg;
        }
    }

    return $opts;
}

// ---------------------------------------------------------------------------
// Child mode: --src=<dir> benchmarks the Resolver loaded from <dir> and emits
// JSON. The driver (default mode) spawns one child per target so the two
// versions of the Resolver class never collide in a single process.
// ---------------------------------------------------------------------------

/**
 * Register a scoped autoloader so the Resolver and its bundled collaborators
 * are loaded from $srcDir, while fixtures and PSR interfaces still come from
 * Composer. Prepended so it wins over Composer's src/ mapping; falls through
 * for files absent from this ref (e.g. InMemoryCache on older revisions).
 */
function registerScopedResolver(string $srcDir): void
{
    spl_autoload_register(static function (string $class) use ($srcDir): void {
        if (!str_starts_with($class, 'Respect\\Parameter\\')) {
            return;
        }
        $rel = substr($class, strlen('Respect\\Parameter\\'));
        if (str_contains($rel, '\\')) {
            return; // sub-namespaces (Test\Fixtures) stay with Composer
        }
        $file = $srcDir . '/' . $rel . '.php';
        if (is_file($file)) {
            require $file;
        }
    }, prepend: true);
}

/**
 * @return array<string, float> ns-per-op keyed by scenario
 */
function runScenarios(int $iterations): array
{
    $resolver = makeResolver();

    $service = new \Respect\Parameter\Test\Fixtures\SampleService();
    $reflInject = new \ReflectionMethod(\Respect\Parameter\Test\Fixtures\ServiceConsumer::class, '__construct');
    $reflFast = new \ReflectionMethod(\Respect\Parameter\Test\Fixtures\TwoRequiredConsumer::class, '__construct');
    $reflVariadic = new \ReflectionMethod(\Respect\Parameter\Test\Fixtures\VariadicConsumer::class, '__construct');

    $scenarios = [
        // Common DI case: service comes from the container, string supplied positionally.
        'container-inject' => [$reflInject, ['hello']],
        // Fast-path candidate: every positional already supplied in type-correct order.
        'fast-path' => [$reflFast, [$service, 'hello']],
        // Variadic expansion: service + trailing ints.
        'variadic' => [$reflVariadic, [$service, 1, 2, 3, 4]],
    ];

    foreach ($scenarios as [$refl, $args]) {
        for ($i = 0; $i < WARMUP; $i++) {
            $resolver->resolve($refl, $args);
        }
    }

    $results = [];
    foreach ($scenarios as $name => [$refl, $args]) {
        $best = PHP_FLOAT_MAX;
        for ($run = 0; $run < TIMED_RUNS; $run++) {
            $start = hrtime(true);
            for ($i = 0; $i < $iterations; $i++) {
                $resolver->resolve($refl, $args);
            }
            $best = min($best, hrtime(true) - $start);
        }
        $results[$name] = $best / $iterations;
    }

    return $results;
}

/** The cached Resolver takes an optional cache arg; the older one takes only the container. */
function makeResolver(): object
{
    $container = new \Respect\Parameter\Test\Fixtures\ArrayContainer([
        \Respect\Parameter\Test\Fixtures\SampleService::class => new \Respect\Parameter\Test\Fixtures\SampleService(),
    ]);

    return new \Respect\Parameter\Resolver($container);
}

// ---------------------------------------------------------------------------
// Driver helpers
// ---------------------------------------------------------------------------

/** Materialize a ref's src/ into a fresh temp dir; returns the dir path. */
function extractRef(string $ref): string
{
    $repo = __DIR__;
    $slug = preg_replace('/[^A-Za-z0-9]/', '_', $ref);
    $dir = sys_get_temp_dir() . '/respect-parameter-bench-' . $slug . '-' . getmypid();
    @mkdir($dir, 0700, true);

    $list = shell_exec(sprintf('git -C %s ls-tree --name-only %s:src 2>/dev/null', escapeshellarg($repo), escapeshellarg($ref . '')));
    foreach (array_filter(explode("\n", (string) $list)) as $file) {
        if (!str_ends_with($file, '.php')) {
            continue;
        }
        $contents = shell_exec(sprintf('git -C %s show %s:src/%s', escapeshellarg($repo), escapeshellarg($ref), escapeshellarg($file)));
        file_put_contents($dir . '/' . $file, (string) $contents);
    }

    return $dir;
}

function removeDir(string $dir): void
{
    foreach (glob($dir . '/*') ?: [] as $file) {
        @unlink($file);
    }
    @rmdir($dir);
}

/**
 * Spawn a child process to benchmark $srcDir; returns ns-per-op by scenario.
 *
 * @return array<string, float>
 */
function benchmarkSrc(string $srcDir, string $label, int $iterations): array
{
    $cmd = sprintf(
        '%s %s --src=%s --label=%s --iterations=%d',
        escapeshellarg(PHP_BINARY),
        escapeshellarg(__FILE__),
        escapeshellarg($srcDir),
        escapeshellarg($label),
        $iterations,
    );
    $decoded = json_decode((string) shell_exec($cmd), true);

    return is_array($decoded) ? $decoded['ns_per_op'] : [];
}

/** @param array<string, array<string, float>> $byTarget */
function printTable(array $byTarget): void
{
    $labels = array_keys($byTarget);
    $scenarios = array_keys($byTarget[$labels[0]]);
    $baseline = $labels[count($labels) - 1]; // last target is the comparison baseline
    $pairwise = count($labels) === 2;

    $w = max(array_map('strlen', $scenarios)) + 2;
    printf("%-{$w}s", 'scenario');
    foreach ($labels as $label) {
        printf('%18s', $label);
    }
    if ($pairwise) {
        printf('%10s', 'delta');
    }
    echo "\n", str_repeat('-', $w + 18 * count($labels) + ($pairwise ? 10 : 0)), "\n";

    foreach ($scenarios as $scenario) {
        printf("%-{$w}s", $scenario);
        foreach ($labels as $label) {
            printf('%15.0f ns', $byTarget[$label][$scenario]);
        }
        if ($pairwise) {
            $a = $byTarget[$labels[0]][$scenario];
            $b = $byTarget[$baseline][$scenario];
            printf('%9.0f%%', ($a - $b) / $b * 100);
        }
        echo "\n";
    }
}

// ---------------------------------------------------------------------------
// Entry point
// ---------------------------------------------------------------------------

require __DIR__ . '/vendor/autoload.php';

$args = parseArgs($argv);

if ($args['src'] !== null) {
    // Child mode: benchmark a single source tree, emit JSON.
    registerScopedResolver($args['src']);
    echo json_encode(['label' => $args['label'], 'ns_per_op' => runScenarios($args['iterations'])]), "\n";

    return;
}

// Driver mode: working tree first, then each comparison ref (default HEAD~1).
$refs = $args['refs'] !== [] ? $args['refs'] : ['HEAD~1'];

$byTarget = ['working-tree' => benchmarkSrc(__DIR__ . '/src', 'working-tree', $args['iterations'])];
foreach ($refs as $ref) {
    $dir = extractRef($ref);
    try {
        $byTarget[$ref] = benchmarkSrc($dir, $ref, $args['iterations']);
    } finally {
        removeDir($dir);
    }
}

printf("Resolver benchmark — %d iterations, best of %d runs (ns per resolve)\n\n", $args['iterations'], TIMED_RUNS);
printTable($byTarget);

[alganet]

Not sure about md5(), this now computes a hash at each lookup.

[henriquemoody]

The MD5 is necessary because of the constraints of the cache key:

• The length cannot be greater than 64 chars
• It needs to match [a-zA-Z0-9_.]

We could do some transformations, but I need to test it

[alganet]

getDeclaringClass creates a new ReflectionClass object for every call, this is an overhead the previous implementation didn't had.

[henriquemoody]

I didn't realise that, this is a major hurdle. Perhaps we could use the filename and line, but I'm not sure if that's better or worst. Need to test it