From 3c0a90820af3d2966c2278b161941afdcd719859 Mon Sep 17 00:00:00 2001 From: "ondrej@mirtes.cz" Date: Sat, 14 Nov 2026 13:02:10 +0000 Subject: [PATCH 1/2] Let tally accept replace a preset complexity class that the growth no longer fits A benchmark compared by its growth had nothing for tally accept to write, so a slower case meant editing the attribute by hand and guessing the class. Accept now fits the measured instructions of a method's data sets against the presets and writes the lowest one they grow within, as tally run compares them: up when they grew more than the class allows, such as #[Constant] to #[Logarithmic], down when a lower preset bounds them too, such as #[Linear] to #[Logarithmic]. The size argument stays as written, and the import of the replaced class goes when nothing else uses it. Classes are compared at the measured sizes only, so #[Exponential] comes before a preset that allows more growth at the largest data set; otherwise any growth would fit some high-degree polynomial. A #[Complexity] with its own formula is left alone, and a method no preset bounds is reported. Amp-Thread-ID: https://ampcode.com/threads/T-01a1d9f0-e845-746f-86dc-7977af8ab66d --- CLAUDE.md | 4 +- README.md | 18 +- bench/Expectation/ExpectationChangesBench.php | 2 +- src/Attribute/Polynomial.php | 5 + src/Cli/AcceptCommand.php | 103 ++++++++- src/Cli/ApplicationFactory.php | 7 +- src/Cli/ReportOutput.php | 2 +- src/Comparison/Comparator.php | 5 +- src/Complexity/GrowthPoint.php | 7 +- src/Complexity/PresetFit.php | 155 +++++++++++++ src/Discovery/CaseExpander.php | 10 +- src/Discovery/GrowthPoints.php | 7 +- src/Expectation/AttributeIndex.php | 9 +- src/Expectation/ComplexityAttribute.php | 50 +++++ src/Expectation/ExpectationChanges.php | 26 ++- src/Expectation/ExpectationFiles.php | 10 +- src/Expectation/ExpectedAttributeRewriter.php | 204 +++++++++++++++--- src/Report/ConsoleReport.php | 2 +- src/Report/MarkdownReport.php | 2 +- src/Report/Report.php | 9 +- src/Report/Section.php | 6 +- .../AcceptGrowth/AcceptGrowthBench.php | 64 ++++++ .../accept-growth/benchDown-forty.json | 8 + .../Results/accept-growth/benchDown-ten.json | 8 + .../accept-growth/benchDown-twenty.json | 8 + .../accept-growth/benchFormula-forty.json | 8 + .../accept-growth/benchFormula-ten.json | 8 + .../accept-growth/benchFormula-twenty.json | 8 + .../accept-growth/benchKept-forty.json | 8 + .../Results/accept-growth/benchKept-ten.json | 8 + .../accept-growth/benchKept-twenty.json | 8 + .../accept-growth/benchNotMeasured-ten.json | 8 + .../benchNotMeasured-twenty.json | 8 + .../accept-growth/benchUnbounded-one.json | 8 + .../accept-growth/benchUnbounded-two.json | 8 + .../accept-growth/benchUnbounded-zero.json | 8 + .../Results/accept-growth/benchUp-forty.json | 8 + .../Results/accept-growth/benchUp-ten.json | 8 + .../Results/accept-growth/benchUp-twenty.json | 8 + .../Cli/CompareAndAcceptCommandTest.php | 52 +++++ tests/Integration/Cli/RunCommandTest.php | 10 +- tests/LinearGrowth.php | 27 +++ tests/Unit/Comparison/ComparatorTest.php | 10 +- tests/Unit/Complexity/PresetFitTest.php | 179 +++++++++++++++ tests/Unit/Discovery/CaseExpanderTest.php | 2 +- tests/Unit/Discovery/GrowthPointsTest.php | 2 +- .../Expectation/ExpectationChangesTest.php | 37 +++- .../ExpectedAttributeRewriterTest.php | 161 ++++++++++++++ tests/Unit/Report/Comparisons.php | 4 +- tests/Unit/Report/ReportTest.php | 16 +- tests/Unit/Selection/Cases.php | 4 +- 51 files changed, 1240 insertions(+), 107 deletions(-) create mode 100644 src/Complexity/PresetFit.php create mode 100644 src/Expectation/ComplexityAttribute.php create mode 100644 tests/Fixtures/AcceptGrowth/AcceptGrowthBench.php create mode 100644 tests/Fixtures/Results/accept-growth/benchDown-forty.json create mode 100644 tests/Fixtures/Results/accept-growth/benchDown-ten.json create mode 100644 tests/Fixtures/Results/accept-growth/benchDown-twenty.json create mode 100644 tests/Fixtures/Results/accept-growth/benchFormula-forty.json create mode 100644 tests/Fixtures/Results/accept-growth/benchFormula-ten.json create mode 100644 tests/Fixtures/Results/accept-growth/benchFormula-twenty.json create mode 100644 tests/Fixtures/Results/accept-growth/benchKept-forty.json create mode 100644 tests/Fixtures/Results/accept-growth/benchKept-ten.json create mode 100644 tests/Fixtures/Results/accept-growth/benchKept-twenty.json create mode 100644 tests/Fixtures/Results/accept-growth/benchNotMeasured-ten.json create mode 100644 tests/Fixtures/Results/accept-growth/benchNotMeasured-twenty.json create mode 100644 tests/Fixtures/Results/accept-growth/benchUnbounded-one.json create mode 100644 tests/Fixtures/Results/accept-growth/benchUnbounded-two.json create mode 100644 tests/Fixtures/Results/accept-growth/benchUnbounded-zero.json create mode 100644 tests/Fixtures/Results/accept-growth/benchUp-forty.json create mode 100644 tests/Fixtures/Results/accept-growth/benchUp-ten.json create mode 100644 tests/Fixtures/Results/accept-growth/benchUp-twenty.json create mode 100644 tests/LinearGrowth.php create mode 100644 tests/Unit/Complexity/PresetFitTest.php diff --git a/CLAUDE.md b/CLAUDE.md index 05a77bd..e2c4bf2 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -6,9 +6,9 @@ Tally benchmarks PHP code by counting instructions, cycles, and memory under Cod - `src/Attribute` – the attributes benchmark classes use: `Expected` (a method without a data provider), `ExpectedFor` (one data set), `Complexity` and its presets (how the instructions grow), `Warmup`, `Before` and `After` (hook methods), `DataProvider`, and the tolerance policies `RelativeTolerance`, `MetricTolerance`, and `DurationTolerance`. - `src/Expression` – the expression language of complexity classes, all pure: `Lexer` (tokens with offsets), `Parser` (recursive descent into the `Node` tree, sealed), `Printer` (the canonical form, parsing it gives the same tree), `TypeChecker` (finds what the parameter types reveal, through `ValueType`), `Evaluator`, and the `MathFunction`s of the `FunctionLibrary`. Every mistake is an `ExpressionException` that quotes the expression and underlines the part that is wrong; `ExpressionErrors` words them the same for the checker and the evaluator. -- `src/Complexity` – `ComplexityClass`, the interface of `#[Complexity]` and its presets (`#[Linear]` and the others, which compose a `Complexity`), and `GrowthPoint`, where a case is on its growth curve. `Discovery\GrowthPoints` computes them at discovery, `Comparator::compareGrowth()` compares a case with its base. These cases have no numbers to accept. +- `src/Complexity` – `ComplexityClass`, the interface of `#[Complexity]` and its presets (`#[Linear]` and the others, which compose a `Complexity`), and `GrowthPoint`, where a case is on its growth curve. `Discovery\GrowthPoints` computes them at discovery, `Comparator::compareGrowth()` compares a case with its base. These cases have no numbers to accept: `PresetFit` (pure) finds the lowest preset the measured instructions of a method grow within, which `tally accept` writes instead of the method's preset through the `ComplexityAttribute` of its `GrowthPoint`. - `src/Quantity` – `QuantityFormat` formats and parses numbers with units (`3.119B`, `44.49MiB`). -- `src/Expectation` – where expected metrics are stored. `ExpectationSource` is the interface: `read()` the expectation, `file()` the file it lives in, the pure `edits(code, metrics)` that returns the `CodeEdit`s that write new expectations into the file's code, and `location()`, which throws `ExpectationException` when the source cannot be rewritten. `AttributeExpectations` implements it for `#[Expected]` and `#[ExpectedFor]` through `ExpectedAttributeRewriter`, which reads a file once into an `AttributeIndex` of every method's attributes, with `Tokens`, the token list it walks, and locates and edits each case in the index; `DirectorySuiteLoader` indexes each file once for all its cases, because tokenizing it for every case made locating and rewriting quadratic in the cases of a file. `ExpectationChanges` plans the rewrites of many cases (pure): it applies the edits of all cases of a file in one pass, one `FileChange` per file with every case in it, in a `ChangePlan` together with the cases that failed. `ExpectationFiles` reads and writes the files. +- `src/Expectation` – where expected metrics are stored. `ExpectationSource` is the interface: `read()` the expectation, `file()` the file it lives in, the pure `edits(code, metrics)` that returns the `CodeEdit`s that write new expectations into the file's code, and `location()`, which throws `ExpectationException` when the source cannot be rewritten. `AttributeExpectations` implements it for `#[Expected]` and `#[ExpectedFor]` through `ExpectedAttributeRewriter`, which reads a file once into an `AttributeIndex` of every method's attributes, with `Tokens`, the token list it walks, and locates and edits each case in the index; `ComplexityAttribute` replaces the complexity class of a method through `ExpectedAttributeRewriter::complexityEdits()`, in the same index; `DirectorySuiteLoader` indexes each file once for all its cases, because tokenizing it for every case made locating and rewriting quadratic in the cases of a file. `ExpectationChanges` plans the rewrites of many cases and complexity classes (pure): it applies the edits of all of them in a file in one pass, one `FileChange` per file with every change in it, in a `ChangePlan` together with the cases that failed. `ExpectationFiles` reads and writes the files. - `src/Discovery` – `DirectorySuiteLoader` finds and loads `*Bench.php` files (I/O), `CaseExpander` turns one method into its cases (pure, works on reflection); `hooks()` finds a class's `#[Before]` and `#[After]` methods once per class, because scanning them for every benchmark method made loading a class quadratic in its size. `AttributeFactory` creates every attribute Tally reads: it checks the arguments against the attribute's constructor first (names, count, types through `ValueTypeCheck`, as under strict_types), so a mistake is an `InvalidBenchmarkException` naming the argument, not a PHP error. Everything a benchmark author can get wrong, including a file that does not load and a data provider that throws, ends as an `InvalidBenchmarkException` that names the benchmark or file; PHP errors with stack traces are bugs. `ArgumentResolver` checks that a method takes the arguments of a data set (a hook the leading ones) and `ConstructorArguments` that a constructor takes only `Meter` and `ScratchDirectory`; discovery checks both, so a mismatch fails `tally list` instead of the worker. - `src/Selection` – `CaseSelector` implementations: `FilterSelector`, `ShardSelector` (longest-processing-time sharding by expected instructions). Both keep a case compared by its growth with its base: the filter adds the base, the shards keep a method's cases together. - `src/Runner` – running one case in this process. `CaseRunner` calls the hooks uncounted, prepares the counter, calls the warm-up uncounted, then counts a `#warmup` span, an empty `#baseline` call, and one call of the benchmark, and subtracts the baseline's memory. The `#warmup` span exists because the first counted span costs more than later ones. `CodSpeedCounter` starts Callgrind's instrumentation once, when it is prepared, stops it when it is finished after the benchmark's span, so the after methods and the end of the process run at Valgrind's quickest, and only toggles collecting for each span: starting the instrumentation empties the simulated caches, and doing it for every span made the cycles of small benchmarks depend on the cache lines the hooks touched. `MeterSession` is the `Meter` the benchmark object gets in its constructor, one per case: every call `CaseRunner` makes is a span of it with its own `Counter` (`CodSpeedCounter` over `InstrumentHooks` for the counted ones, `IdleCounter` for hooks and warm-up), and it measures memory above the start of the span through `Memory`. `ExtensionInstrumentHooks` calls the `tally_hooks` extension, the only C that Tally calls. diff --git a/README.md b/README.md index 342b2c3..39374c0 100644 --- a/README.md +++ b/README.md @@ -290,7 +290,7 @@ Benchmarks failed: 2 slower, 1 unchanged Allowed difference: instructions grow at most as O(n) from the smallest data set, ±10 % of each metric Slower than expected -These benchmarks do more work than expected. Find out why before merging. If it is the price of a fix, loosen the complexity class. +These benchmarks do more work than expected. Find out why before merging. If it is the price of a fix, accept the new complexity class. Benchmark Metric Expected Measured Change Allowed App\PairsBench::benchPairs@size-20 instructions 93.44K 178.7K +91.2 % ±10 % @@ -298,9 +298,21 @@ These benchmarks do more work than expected. Find out why before merging. If it Reproduce locally vendor/bin/tally run --filter='App\PairsBench::benchPairs@size-40' + +Accept the new numbers + vendor/bin/tally run --output=tmp/tally + vendor/bin/tally accept tmp/tally +``` + +`tally accept` replaces a preset with the lowest one, in the order of the table above, that the measured instructions grow within as `tally run` compares them, keeping the size as written: + +``` +slower App\PairsBench::benchPairs: O(n) to O(n^2) + +Accepted the complexity class of 1 benchmark. ``` -`tally accept` does not write anything for these cases; change the attribute instead. A method needs a data provider with at least two data sets of different sizes, and cannot also have `#[ExpectedFor]` or expectations from its data provider. `--filter` adds the base of each selected case, and `--shard` keeps the cases of a method in one shard, because each is compared with its base. +This also tightens a preset the instructions grow well within, such as `#[Linear]` to `#[Logarithmic]`, listed as faster. The presets are compared at the sizes of the data sets only, so `#[Exponential]` replaces a preset that allows more growth at the largest one, as every polynomial of a high enough degree does. Accept needs the instructions of every data set of the method. It leaves `#[Complexity]` with a formula of its own as it is, and fails for a method no preset bounds. A method needs a data provider with at least two data sets of different sizes, and cannot also have `#[ExpectedFor]` or expectations from its data provider. `--filter` adds the base of each selected case, and `--shard` keeps the cases of a method in one shard, because each is compared with its base. Mistakes in the expressions are found when the benchmarks are loaded, before anything is counted: @@ -395,7 +407,7 @@ Compares the results of `tally run --output` with the expected numbers again, wi ### `tally accept ` -Makes the results of `tally run --output` the expected numbers of the cases that are new, slower, or faster. It rewrites the `#[Expected]` and `#[ExpectedFor]` attributes, or adds them, with the imports they need, and leaves the rest of the file as it is. +Makes the results of `tally run --output` the expected numbers of the cases that are new, slower, or faster. It rewrites the `#[Expected]` and `#[ExpectedFor]` attributes, or adds them, with the imports they need, and leaves the rest of the file as it is. It replaces the preset [complexity class](#complexity) of a method whose instructions grew more than it allows, or well within a lower preset, and removes the import of the replaced class when nothing else uses it. ``` slower App\SortBench::benchSort@size-10 diff --git a/bench/Expectation/ExpectationChangesBench.php b/bench/Expectation/ExpectationChangesBench.php index 5575aec..c8c98a5 100644 --- a/bench/Expectation/ExpectationChangesBench.php +++ b/bench/Expectation/ExpectationChangesBench.php @@ -43,7 +43,7 @@ public static function newCases(): iterable #[ExpectedFor('cases-200', instructions: '21.63M', cycles: '26.19M', memory: '291.0KiB')] public function benchPlan(array $accepted, array $code): void { - (new ExpectationChanges())->plan($accepted, $code); + (new ExpectationChanges())->plan($accepted, [], $code); } } diff --git a/src/Attribute/Polynomial.php b/src/Attribute/Polynomial.php index 0bf25b6..d22ad73 100644 --- a/src/Attribute/Polynomial.php +++ b/src/Attribute/Polynomial.php @@ -32,6 +32,11 @@ public function __construct(string $size, private int $degree) $this->complexity = new Complexity($size, 'n^' . $degree); } + public function degree(): int + { + return $this->degree; + } + #[Override] public function size(): string { diff --git a/src/Cli/AcceptCommand.php b/src/Cli/AcceptCommand.php index 4cccad4..c8e82bc 100644 --- a/src/Cli/AcceptCommand.php +++ b/src/Cli/AcceptCommand.php @@ -5,10 +5,17 @@ use Override; use RuntimeException; use Tally\BenchmarkCase; +use Tally\CaseId; use Tally\Comparison\Comparator; use Tally\Comparison\Status; +use Tally\Complexity\ComplexityClass; +use Tally\Complexity\GrowthPoint; +use Tally\Complexity\PresetFit; +use Tally\Expectation\ComplexityAttribute; use Tally\Expectation\ExpectationChanges; use Tally\Expectation\ExpectationFiles; +use Tally\Measurement; +use Tally\Metric; use function array_filter; use function array_map; use function count; @@ -20,7 +27,8 @@ /** * Writes the measured numbers of the cases that changed as their expectations: into #[Expected] * attributes, or wherever a data provider keeps them. Cases within their tolerance keep their - * numbers, so accepting does not churn them. + * numbers, so accepting does not churn them. A preset complexity class, like #[Linear], is + * replaced by the preset that bounds the measured growth most tightly. */ final class AcceptCommand implements Command { @@ -33,7 +41,8 @@ public function __construct( private ResultsArguments $resultsArguments, private Comparator $comparator, private ExpectationFiles $files, - private ExpectationChanges $changes + private ExpectationChanges $changes, + private PresetFit $fit ) { } @@ -69,22 +78,47 @@ public function execute(Input $input, Output $output): int $cases = $this->selectedCases->load($input, $this->selectedCases->configuration($input), $this->resultsArguments->paths($input)); $changed = []; + $growing = []; foreach ($cases as $case) { $measurement = $measurements[$case->id->toString()] ?? null; $comparison = $this->comparator->compareCase($case, $measurements); + if ($case->growth !== null) { + $growing[$case->growth->base->toString()][] = [$case, $case->growth, $comparison->status, $measurement]; + continue; + } $status = $comparison->status; - if ($measurement === null || !$comparison->acceptable || $status === Status::Ok || $status === Status::NotMeasured) { + if ($measurement === null || $status === Status::Ok || $status === Status::NotMeasured) { continue; } $changed[] = [$case, $measurement->metrics, $status]; } + $failed = false; + $complexities = []; + foreach ($growing as $methodCases) { + $change = $this->complexityChange($methodCases); + if ($change === null) { + continue; + } + if ($change[2] === null) { + $output->error(sprintf('Cannot accept %s: no preset complexity class bounds how its instructions grow, write a #[Complexity] that does.', $change[0]->toString())); + $failed = true; + continue; + } + + $complexities[] = $change; + } + $plan = $this->changes->plan( array_map(static fn (array $change): array => [$change[0], $change[1]], $changed), - $this->files->read(array_map(static fn (array $change): BenchmarkCase => $change[0], $changed)), + array_map(static fn (array $change): array => [$change[0], $change[1], $change[2]], $complexities), + $this->files->read([ + ...array_map(static fn (array $change): string => $change[0]->expectations->file(), $changed), + ...array_map(static fn (array $change): string => $change[1]->file(), $complexities), + ]), ); - $failed = $plan->failures !== []; + $failed = $failed || $plan->failures !== []; foreach ($plan->failures as $id => $failure) { $output->error(sprintf('Cannot accept %s: %s', $id, $failure)); } @@ -104,14 +138,61 @@ public function execute(Input $input, Output $output): int foreach ($accepted as [$case, , $status]) { $output->write(sprintf("%s %s\n", str_pad($status->value, self::STATUS_WIDTH), $case->id->toString())); } - $output->write(sprintf( - "%sAccepted the numbers of %d %s.\n", - $accepted === [] ? '' : "\n", - count($accepted), - count($accepted) === 1 ? 'case' : 'cases', - )); + $acceptedComplexities = array_filter($complexities, static fn (array $change): bool => in_array($change[0]->toString(), $written, true)); + foreach ($acceptedComplexities as [$id, $attribute, $replacement, $status]) { + $output->write(sprintf( + "%s %s: %s to %s\n", + str_pad($status->value, self::STATUS_WIDTH), + $id->toString(), + $attribute->complexity->describe(), + $replacement->describe(), + )); + } + $what = []; + if ($accepted !== [] || $acceptedComplexities === []) { + $what[] = sprintf('the numbers of %d %s', count($accepted), count($accepted) === 1 ? 'case' : 'cases'); + } + if ($acceptedComplexities !== []) { + $what[] = count($acceptedComplexities) === 1 ? 'the complexity class of 1 benchmark' : sprintf('the complexity classes of %d benchmarks', count($acceptedComplexities)); + } + $output->write(sprintf("%sAccepted %s.\n", $accepted === [] && $acceptedComplexities === [] ? '' : "\n", implode(' and ', $what))); return $failed ? Application::EXIT_ERROR : Application::EXIT_SUCCESS; } + /** + * The preset that replaces the complexity class of a benchmark method, when the instructions of + * all its cases were counted: slower when its class allows less than they grew, faster when a + * tighter preset bounds them. + * + * @param non-empty-list $cases + * @return array{CaseId, ComplexityAttribute, ComplexityClass|null, Status}|null null when it keeps its class, the class null when no preset bounds them + */ + private function complexityChange(array $cases): ?array + { + $points = []; + $status = Status::Faster; + foreach ($cases as [, $growth, $caseStatus, $measurement]) { + $instructions = $measurement?->metrics->get(Metric::Instructions); + if ($instructions === null) { + return null; + } + $points[] = [$growth->size, $instructions]; + if ($caseStatus !== Status::Slower) { + continue; + } + + $status = Status::Slower; + } + + [$case, $growth] = $cases[0]; + $attribute = $growth->attribute; + $fitted = $this->fit->fit($attribute->complexity, $points, $case->tolerance); + if ($fitted === $attribute->complexity) { + return null; + } + + return [new CaseId($case->id->class, $case->id->method, null), $attribute, $fitted, $status]; + } + } diff --git a/src/Cli/ApplicationFactory.php b/src/Cli/ApplicationFactory.php index c83d06f..46e5417 100644 --- a/src/Cli/ApplicationFactory.php +++ b/src/Cli/ApplicationFactory.php @@ -3,6 +3,7 @@ namespace Tally\Cli; use Tally\Comparison\Comparator; +use Tally\Complexity\PresetFit; use Tally\Discovery\ArgumentResolver; use Tally\Discovery\AttributeFactory; use Tally\Discovery\CaseExpander; @@ -77,7 +78,9 @@ public function create($stdout, $stderr, string $workingDirectory, array $enviro $rewriter = new ExpectedAttributeRewriter(); $functions = new FunctionLibrary([new Log(), new SquareRoot(), new Exponential(), new Factorial()]); $expressionErrors = new ExpressionErrors($closestName); - $growthPoints = new GrowthPoints(new Parser(new Lexer()), new TypeChecker($functions, $expressionErrors), new Evaluator($functions, $expressionErrors)); + $parser = new Parser(new Lexer()); + $evaluator = new Evaluator($functions, $expressionErrors); + $growthPoints = new GrowthPoints($parser, new TypeChecker($functions, $expressionErrors), $evaluator); $selectedCases = new SelectedCases( new DirectorySuiteLoader(new CaseExpander($rewriter, new AttributeFactory(new ValueTypeCheck()), $argumentResolver, $growthPoints), $constructorArguments, $rewriter, $relativePath), $workingDirectory, @@ -119,7 +122,7 @@ public function create($stdout, $stderr, string $workingDirectory, array $enviro $workingDirectory, ), new CompareCommand($selectedCases, $resultsArguments, $comparator, $reportOutput, $environment, $workingDirectory), - new AcceptCommand($selectedCases, $resultsArguments, $comparator, $expectationFiles, $expectationChanges), + new AcceptCommand($selectedCases, $resultsArguments, $comparator, $expectationFiles, $expectationChanges, new PresetFit($parser, $evaluator, $comparator)), ], [new WorkerCommand( $selectedCases, diff --git a/src/Cli/ReportOutput.php b/src/Cli/ReportOutput.php index 78a6e62..e6cbdd9 100644 --- a/src/Cli/ReportOutput.php +++ b/src/Cli/ReportOutput.php @@ -92,7 +92,7 @@ private function newExpectations(array $cases, array $measurements, string $work $accepted[] = [$case, $measurement->metrics]; } - $plan = $this->changes->plan($accepted, $this->files->read(array_map(static fn (array $accepted): BenchmarkCase => $accepted[0], $accepted))); + $plan = $this->changes->plan($accepted, [], $this->files->read(array_map(static fn (array $accepted): string => $accepted[0]->expectations->file(), $accepted))); return new NewExpectations($this->patch->render($plan, $workingDirectory), $plan->failures); } diff --git a/src/Comparison/Comparator.php b/src/Comparison/Comparator.php index baf78bd..4a36421 100644 --- a/src/Comparison/Comparator.php +++ b/src/Comparison/Comparator.php @@ -59,7 +59,7 @@ public function compareLocated(BenchmarkCase $case, SourceLocation $location, ar */ public function compareGrowth(CaseId $id, SourceLocation $location, GrowthPoint $growth, TolerancePolicy $tolerance, ?Measurement $measurement, ?Measurement $base): CaseComparison { - $tolerance = new GrowthTolerance($growth->bound, $tolerance); + $tolerance = new GrowthTolerance($growth->attribute->complexity->describe(), $tolerance); if ($measurement === null || $measurement->failure !== null) { return new CaseComparison($id, $location, $tolerance, [], $measurement->failure ?? self::NOT_RUN, Status::NotMeasured, false); } @@ -112,7 +112,8 @@ public function compare(CaseId $id, SourceLocation $location, ?Metrics $expected ); } - private function compareMetric(Metric $metric, ?int $expected, int $measured, TolerancePolicy $tolerance): MetricComparison + /** Also tells whether instructions grow within a complexity class, for tally accept */ + public function compareMetric(Metric $metric, ?int $expected, int $measured, TolerancePolicy $tolerance): MetricComparison { if ($expected === null) { return new MetricComparison($metric, null, $measured, null, Status::New); diff --git a/src/Complexity/GrowthPoint.php b/src/Complexity/GrowthPoint.php index 2655180..82b0bae 100644 --- a/src/Complexity/GrowthPoint.php +++ b/src/Complexity/GrowthPoint.php @@ -3,6 +3,7 @@ namespace Tally\Complexity; use Tally\CaseId; +use Tally\Expectation\ComplexityAttribute; /** * Where a case of a benchmark with a complexity class is on its growth curve: how many times its @@ -11,9 +12,13 @@ final class GrowthPoint { + /** + * @param float $size the size of the case's data set + */ public function __construct( - public readonly string $bound, + public readonly ComplexityAttribute $attribute, public readonly CaseId $base, + public readonly float $size, public readonly float $growth ) { diff --git a/src/Complexity/PresetFit.php b/src/Complexity/PresetFit.php new file mode 100644 index 0000000..4a8c9eb --- /dev/null +++ b/src/Complexity/PresetFit.php @@ -0,0 +1,155 @@ + $points the size and the measured instructions of each data set + * @return ComplexityClass|null the current class when it is not a preset, null when no preset fits + */ + public function fit(ComplexityClass $current, array $points, TolerancePolicy $tolerance): ?ComplexityClass + { + if (!in_array($current::class, self::PRESETS, true)) { + return $current; + } + + usort($points, static fn (array $a, array $b): int => $a[0] <=> $b[0]); + $size = $current->size(); + $exponential = new Exponential($size); + $exponentialGrowth = $this->growth($exponential, $points); + $exponentialFits = $exponentialGrowth !== null && $this->fits($exponentialGrowth, $tolerance); + for ($presets = $this->presets($size); true; $presets->next()) { + $preset = $presets->current(); + $growth = $this->growth($preset, $points); + // like log(n) at size 1; a polynomial is not defined at the higher degrees either + if ($growth === null && !$preset instanceof Polynomial) { + continue; + } + if ($growth === null || $exponentialFits && $this->largest($growth) >= $this->largest($exponentialGrowth)) { + return $this->kept($current, $exponentialFits ? $exponential : null, $points); + } + if ($this->fits($growth, $tolerance)) { + return $this->kept($current, $preset, $points); + } + } + } + + /** + * The presets below #[Exponential] in the order of complexity, the polynomials without end. + * #[Linear] and #[Quadratic] are the polynomials of degree 1 and 2. + * + * @return Generator + */ + private function presets(string $size): Generator + { + yield new Constant($size); + yield new Logarithmic($size); + for ($degree = 1;; $degree++) { + yield from match ($degree) { + 1 => [new Linear($size), new Linearithmic($size)], + 2 => [new Quadratic($size)], + default => [new Polynomial($size, $degree)], + }; + } + } + + /** + * @param non-empty-list $points sorted by size + */ + private function kept(ComplexityClass $current, ?ComplexityClass $fitted, array $points): ?ComplexityClass + { + return $fitted !== null && $this->growth($fitted, $points) === $this->growth($current, $points) ? $current : $fitted; + } + + /** + * How many times the formula is larger at each size than at the smallest one, as discovery + * computes it, with the measured instructions of that size. + * + * @param non-empty-list $points sorted by size + * @return non-empty-list|null the growth and the instructions of each point, null when the formula is not above 0 and finite at every size + */ + private function growth(ComplexityClass $class, array $points): ?array + { + try { + $formula = $this->parser->parse($class->formula()); + $costs = array_map(fn (array $point): float => $this->evaluator->evaluateNumber($formula, ['n' => $point[0]]), $points); + } catch (ExpressionException) { + return null; + } + foreach ($costs as $cost) { + if (!is_finite($cost) || $cost <= 0) { + return null; + } + } + + return array_map(static fn (float $cost, array $point): array => [$cost / $costs[0], $point[1]], $costs, $points); + } + + /** + * The largest growth of the points, which a tighter class allows less of + * + * @param non-empty-list $growth + */ + private function largest(array $growth): float + { + return max(array_column($growth, 0)); + } + + /** + * Whether no data set is slower than the smallest one's instructions times the growth, as + * Comparator::compareGrowth() decides. + * + * @param non-empty-list $growth the growth and the instructions of each point + */ + private function fits(array $growth, TolerancePolicy $tolerance): bool + { + foreach ($growth as [$times, $instructions]) { + $expected = (int) round($growth[0][1] * $times); + if ($this->comparator->compareMetric(Metric::Instructions, $expected, $instructions, $tolerance)->status === Status::Slower) { + return false; + } + } + + return true; + } + +} diff --git a/src/Discovery/CaseExpander.php b/src/Discovery/CaseExpander.php index 1160fa5..9dc5ccf 100644 --- a/src/Discovery/CaseExpander.php +++ b/src/Discovery/CaseExpander.php @@ -20,6 +20,7 @@ use Tally\DataSet; use Tally\Expectation\AttributeExpectations; use Tally\Expectation\AttributeIndex; +use Tally\Expectation\ComplexityAttribute; use Tally\Expectation\ExpectationSource; use Tally\Expectation\ExpectedAttributeRewriter; use Tally\Metrics; @@ -160,7 +161,14 @@ public function expand(ReflectionClass $class, ReflectionMethod $method, array $ return array_map( static fn (BenchmarkCase $case, GrowthPoint $growth): BenchmarkCase => $case->withGrowth($growth), $cases, - $this->growthPoints->points($class->getName(), $method, $complexity[0], $complexity[1], $dataSets, $where), + $this->growthPoints->points( + $class->getName(), + $method, + new ComplexityAttribute($this->rewriter, $file, $attributes, $declaringClass, $method->getName(), $complexity[0]), + $complexity[1], + $dataSets, + $where, + ), ); } diff --git a/src/Discovery/GrowthPoints.php b/src/Discovery/GrowthPoints.php index cd76317..1864d34 100644 --- a/src/Discovery/GrowthPoints.php +++ b/src/Discovery/GrowthPoints.php @@ -4,9 +4,9 @@ use ReflectionMethod; use Tally\CaseId; -use Tally\Complexity\ComplexityClass; use Tally\Complexity\GrowthPoint; use Tally\DataSet; +use Tally\Expectation\ComplexityAttribute; use Tally\Expression\Evaluator; use Tally\Expression\Expression; use Tally\Expression\ExpressionException; @@ -44,8 +44,9 @@ public function __construct(private Parser $parser, private TypeChecker $checker * @return list in the order of the data sets * @throws InvalidBenchmarkException */ - public function points(string $className, ReflectionMethod $method, ComplexityClass $complexity, string $name, array $dataSets, string $where): array + public function points(string $className, ReflectionMethod $method, ComplexityAttribute $attribute, string $name, array $dataSets, string $where): array { + $complexity = $attribute->complexity; $size = $this->parse($complexity->size(), $name . ' size', $where); $formula = $this->parse($complexity->formula(), $name . ' formula', $where); $this->checkTypes($method, $size, $formula, $name, $where); @@ -71,7 +72,7 @@ public function points(string $className, ReflectionMethod $method, ComplexityCl $baseId = new CaseId($className, $method->getName(), $baseName); return array_map( - fn (DataSet $dataSet, float $n): GrowthPoint => new GrowthPoint($complexity->describe(), $baseId, $this->cost($formula, $n, $dataSet->name, $name, $where) / $baseCost), + fn (DataSet $dataSet, float $n): GrowthPoint => new GrowthPoint($attribute, $baseId, $n, $this->cost($formula, $n, $dataSet->name, $name, $where) / $baseCost), $dataSets, array_values($sizes), ); diff --git a/src/Expectation/AttributeIndex.php b/src/Expectation/AttributeIndex.php index 856d852..7d97f56 100644 --- a/src/Expectation/AttributeIndex.php +++ b/src/Expectation/AttributeIndex.php @@ -3,11 +3,11 @@ namespace Tally\Expectation; /** - * Where the #[Expected] and #[ExpectedFor] attributes of every method of a file are, and where + * Where the attributes of every method of a file are, such as #[Expected] and #[Linear], and where * missing ones and their imports go, found by one pass of ExpectedAttributeRewriter over the * code. Offsets are in bytes of the code. * - * @phpstan-type Attribute array{start: int, end: int, name: string, dataSet: string|null, line: int, literal: bool} + * @phpstan-type Attribute array{start: int, end: int, name: string, class: string, dataSet: string|null, size: string|null, line: int, literal: bool} * @phpstan-type Member array{attributes: list, insertAt: int, line: int, indent: string} * @phpstan-type Import array{name: string, alias: string, start: int, end: int} */ @@ -21,6 +21,8 @@ final class AttributeIndex * @param int|null $otherUse the first import of a function or a constant * @param array $members the methods by lowercase Class::method, the attributes * before each, and where an attribute goes after them, with its line and indentation + * @param array $names how often each lowercase name is used outside the imports, + * so that an import nothing uses any more can go */ public function __construct( public readonly string $code, @@ -28,7 +30,8 @@ public function __construct( public readonly ?int $namespaceEnd, public readonly array $imports, public readonly ?int $otherUse, - public readonly array $members + public readonly array $members, + public readonly array $names ) { } diff --git a/src/Expectation/ComplexityAttribute.php b/src/Expectation/ComplexityAttribute.php new file mode 100644 index 0000000..d7b3166 --- /dev/null +++ b/src/Expectation/ComplexityAttribute.php @@ -0,0 +1,50 @@ +file; + } + + /** + * The edits of the file's code that write the replacement instead of this complexity class. + * + * @return list + * @throws ExpectationException + */ + public function edits(string $code, ComplexityClass $replacement): array + { + if ($code !== $this->index->code) { + throw new ExpectationException(sprintf('%s changed after Tally loaded it, run the command again.', $this->file)); + } + + return $this->rewriter->complexityEdits($this->index, $this->class, $this->method, $this->complexity::class, $replacement); + } + +} diff --git a/src/Expectation/ExpectationChanges.php b/src/Expectation/ExpectationChanges.php index 8e4c0b5..c9c657d 100644 --- a/src/Expectation/ExpectationChanges.php +++ b/src/Expectation/ExpectationChanges.php @@ -3,6 +3,8 @@ namespace Tally\Expectation; use Tally\BenchmarkCase; +use Tally\CaseId; +use Tally\Complexity\ComplexityClass; use Tally\Metrics; use function array_diff_key; use function array_keys; @@ -12,25 +14,33 @@ use function usort; /** - * Plans the new code of the files that keep the expectations of the accepted cases. The edits of - * all cases of a file are applied together, in one pass over its code. ExpectationFiles reads and - * writes the files. + * Plans the new code of the files that keep the expectations of the accepted cases, and the + * complexity classes of the accepted benchmark methods. The edits of all of them in a file are + * applied together, in one pass over its code. ExpectationFiles reads and writes the files. */ final class ExpectationChanges { /** * @param list $accepted the cases with their measured metrics + * @param list $complexities the benchmark methods with the classes that replace theirs * @param array $code the code of the files by path, false for a file that cannot be read */ - public function plan(array $accepted, array $code): ChangePlan + public function plan(array $accepted, array $complexities, array $code): ChangePlan { /** @var array>}> $editsByFile the code and the edits by case */ $editsByFile = []; - $failures = []; + $changes = []; foreach ($accepted as [$case, $measured]) { - $id = $case->id->toString(); - $file = $case->expectations->file(); + $changes[] = [$case->id, $case->expectations->file(), static fn (string $code): array => $case->expectations->edits($code, $measured)]; + } + foreach ($complexities as [$id, $attribute, $replacement]) { + $changes[] = [$id, $attribute->file(), static fn (string $code): array => $attribute->edits($code, $replacement)]; + } + + $failures = []; + foreach ($changes as [$caseId, $file, $edits]) { + $id = $caseId->toString(); $before = $code[$file] ?? false; if ($before === false) { $failures[$id] = sprintf('Cannot read %s.', $file); @@ -38,7 +48,7 @@ public function plan(array $accepted, array $code): ChangePlan } try { - $edits = $case->expectations->edits($before, $measured); + $edits = $edits($before); } catch (ExpectationException $e) { $failures[$id] = $e->getMessage(); continue; diff --git a/src/Expectation/ExpectationFiles.php b/src/Expectation/ExpectationFiles.php index 827c721..361ab46 100644 --- a/src/Expectation/ExpectationFiles.php +++ b/src/Expectation/ExpectationFiles.php @@ -3,7 +3,6 @@ namespace Tally\Expectation; use RuntimeException; -use Tally\BenchmarkCase; use function file_get_contents; use function file_put_contents; use function sprintf; @@ -15,14 +14,13 @@ final class ExpectationFiles { /** - * @param list $cases - * @return array the code of their files by path, false for a file that cannot be read + * @param list $files + * @return array their code by path, false for a file that cannot be read */ - public function read(array $cases): array + public function read(array $files): array { $code = []; - foreach ($cases as $case) { - $file = $case->expectations->file(); + foreach ($files as $file) { $code[$file] ??= @file_get_contents($file); } diff --git a/src/Expectation/ExpectedAttributeRewriter.php b/src/Expectation/ExpectedAttributeRewriter.php index 36a2385..0b2386f 100644 --- a/src/Expectation/ExpectedAttributeRewriter.php +++ b/src/Expectation/ExpectedAttributeRewriter.php @@ -5,8 +5,11 @@ use InvalidArgumentException; use Tally\Attribute\Expected; use Tally\Attribute\ExpectedFor; +use Tally\Attribute\Polynomial; +use Tally\Complexity\ComplexityClass; use Tally\Metric; use Tally\Metrics; +use function array_values; use function count; use function explode; use function implode; @@ -38,10 +41,10 @@ /** * Finds the #[Expected] attribute of each benchmark method, or its #[ExpectedFor] of each data set, - * in PHP code, using PHP's tokenizer, and the edits that rewrite one of them. index() reads the - * whole code once, so a file with many cases is tokenized once, not once per case. Everything else - * in the code stays byte for byte the same. A missing attribute is added after the method's other - * attributes, and imported when the file does not import it yet. + * in PHP code, using PHP's tokenizer, and the edits that rewrite one of them or replace the method's + * complexity class. index() reads the whole code once, so a file with many cases is tokenized once, + * not once per case. Everything else in the code stays byte for byte the same. A missing attribute + * is added after the method's other attributes, and imported when the file does not import it yet. * * @phpstan-import-type Attribute from AttributeIndex * @phpstan-import-type Member from AttributeIndex @@ -61,7 +64,7 @@ final class ExpectedAttributeRewriter public function locate(AttributeIndex $index, string $class, string $method, ?string $dataSet): int { $member = $this->member($index, $class, $method); - $attribute = $this->findAttribute($member, $dataSet); + $attribute = $this->findAttribute($member, $dataSet === null ? Expected::class : ExpectedFor::class, $dataSet); return $attribute !== null ? $attribute['line'] : $member['line']; } @@ -81,22 +84,113 @@ public function edits(AttributeIndex $index, string $class, string $method, ?str { $arguments = $this->arguments($dataSet, $metrics, $namedArguments); $member = $this->member($index, $class, $method); - $attribute = $this->findAttribute($member, $dataSet); + $attributeClass = $dataSet === null ? Expected::class : ExpectedFor::class; + $attribute = $this->findAttribute($member, $attributeClass, $dataSet); if ($attribute !== null) { return [new CodeEdit($attribute['start'], $attribute['end'] - $attribute['start'], $attribute['name'] . '(' . $arguments . ')')]; } - $attributeClass = $dataSet === null ? Expected::class : ExpectedFor::class; $name = $this->localName($index, $attributeClass); $imports = []; if ($name === null) { $name = $this->shortName($attributeClass); - $imports = $this->importEdits($index, $attributeClass); + $imports = $this->importEdits($index, $index->imports, $attributeClass); } return [...$imports, new CodeEdit($member['insertAt'], 0, '#[' . $name . '(' . $arguments . ')]' . "\n" . $member['indent'])]; } + /** + * The edits of the indexed code that replace the complexity class of a benchmark method with + * another, with the same size argument as written. The import of the replaced class goes when + * nothing else uses it; the import of the new one takes its place when it sorts there. + * + * @param class-string $current the class of the attribute to replace + * @return non-empty-list + * @throws MethodNotFoundException + * @throws ExpectationException + */ + public function complexityEdits(AttributeIndex $index, string $class, string $method, string $current, ComplexityClass $replacement): array + { + $attribute = $this->findAttribute($this->member($index, $class, $method), $current, null) + ?? throw new ExpectationException(sprintf('Method %s::%s() has no #[%s] attribute.', $class, $method, $this->shortName($current))); + $arguments = $attribute['size'] ?? ''; + if ($replacement instanceof Polynomial) { + $arguments .= ', degree: ' . $replacement->degree(); + } + + $edits = []; + // a polynomial of another degree keeps the import + $removed = $replacement::class === $current ? null : $this->unusedImport($index, $attribute); + $imports = $index->imports; + if ($removed !== null) { + unset($imports[$removed[0]]); + } + $name = $this->localName($index, $replacement::class); + if ($name === null) { + $name = $this->shortName($replacement::class); + if ($removed !== null && $this->sortsInPlace($index->imports, $removed[0], $replacement::class)) { + $edits[] = new CodeEdit($removed[1]['start'], $removed[1]['end'] - $removed[1]['start'], 'use ' . $replacement::class . ';'); + $removed = null; + } else { + $edits = $this->importEdits($index, array_values($imports), $replacement::class); + } + } + if ($removed !== null) { + [, $old] = $removed; + $edits[] = new CodeEdit($old['start'], $old['end'] - $old['start'] + (substr($index->code, $old['end'], 1) === "\n" ? 1 : 0), ''); + } + + return [...$edits, new CodeEdit($attribute['start'], $attribute['end'] - $attribute['start'], $name . '(' . $arguments . ')')]; + } + + /** + * The import of the attribute's class, when the attribute uses its alias and nothing else in + * the code does. An import among others in one use statement stays. + * + * @param Attribute $attribute + * @return array{int, Import}|null its key in the imports of the index, and the import + */ + private function unusedImport(AttributeIndex $index, array $attribute): ?array + { + foreach ($index->imports as $key => $import) { + if (strcasecmp($import['name'], $attribute['class']) !== 0) { + continue; + } + foreach ($index->imports as $other) { + if ($other['start'] === $import['start'] && $other !== $import) { + return null; + } + } + + if (strcasecmp($attribute['name'], $import['alias']) !== 0) { + return null; + } + + // the attribute's own name is one of the uses + $alias = strtolower($import['alias']); + + return isset($index->names[$alias]) && $index->names[$alias] > 1 ? null : [$key, $import]; + } + + return null; + } + + /** + * Whether the class sorts between the imports before and after the removed one. + * + * @param list $imports + * @param class-string $class + */ + private function sortsInPlace(array $imports, int $removed, string $class): bool + { + $previous = $imports[$removed - 1] ?? null; + $next = $imports[$removed + 1] ?? null; + + return ($previous === null || (strtolower($previous['name']) <=> strtolower($class)) === -1) + && ($next === null || (strtolower($next['name']) <=> strtolower($class)) === 1); + } + /** * Reads every class of the code once. */ @@ -138,12 +232,13 @@ private function arguments(?string $dataSet, Metrics $metrics, bool $namedArgume /** * @param Member $member + * @param class-string $class * @return Attribute|null */ - private function findAttribute(array $member, ?string $dataSet): ?array + private function findAttribute(array $member, string $class, ?string $dataSet): ?array { foreach ($member['attributes'] as $attribute) { - if ($attribute['dataSet'] === $dataSet) { + if (strcasecmp($attribute['class'], $class) === 0 && $attribute['dataSet'] === $dataSet) { return $attribute; } } @@ -186,20 +281,20 @@ private function shortName(string $class): string * #[Expected] and #[ExpectedFor] can go to the same place; in the natural order of their text, * which is how ExpectationChanges applies edits at one place, they stay sorted too. * + * @param list $imports the imports of the index to place it among * @param class-string $attributeClass * @return non-empty-list */ - private function importEdits(AttributeIndex $index, string $attributeClass): array + private function importEdits(AttributeIndex $index, array $imports, string $attributeClass): array { $statement = 'use ' . $attributeClass . ';'; - foreach ($index->imports as $import) { + foreach ($imports as $import) { // 0 would be the attribute itself, which needs no import if ((strtolower($import['name']) <=> strtolower($attributeClass)) === 1) { return [new CodeEdit($import['start'], 0, $statement . "\n")]; } } - $imports = $index->imports; if ($imports !== []) { return [new CodeEdit($imports[count($imports) - 1]['end'], 0, "\n" . $statement)]; } @@ -226,6 +321,7 @@ private function scan(Tokens $tokens, string $code): AttributeIndex $attributes = []; $insertAt = null; $members = []; + $names = []; $count = $tokens->count(); for ($i = 0; $i < $count; $i++) { @@ -233,6 +329,9 @@ private function scan(Tokens $tokens, string $code): AttributeIndex if ($token->isIgnorable()) { continue; } + if ($token->is(T_STRING)) { + $names[strtolower($token->text)] = ($names[strtolower($token->text)] ?? 0) + 1; + } if ($token->text === '{' || $token->is(T_DOLLAR_OPEN_CURLY_BRACES)) { $depth++; @@ -302,6 +401,13 @@ private function scan(Tokens $tokens, string $code): AttributeIndex if ($token->is(T_ATTRIBUTE)) { [$groupAttributes, $groupEnd] = $this->parseAttributeGroup($tokens, $i, $namespace, $imports); $attributes = [...$attributes, ...$groupAttributes]; + for ($j = $i; $j < $groupEnd; $j++) { + if (!$tokens->at($j)->is(T_STRING)) { + continue; + } + + $names[strtolower($tokens->at($j)->text)] = ($names[strtolower($tokens->at($j)->text)] ?? 0) + 1; + } $insertAt = null; $i = $groupEnd; continue; @@ -330,12 +436,12 @@ private function scan(Tokens $tokens, string $code): AttributeIndex $insertAt = null; } - return new AttributeIndex($code, $namespace, $namespaceEnd, $imports, $otherUse, $members); + return new AttributeIndex($code, $namespace, $namespaceEnd, $imports, $otherUse, $members, $names); } /** * @param list $imports - * @return array{list, int} the #[Expected] and #[ExpectedFor] attributes of the group, and the index of its closing bracket + * @return array{list, int} the attributes of the group, and the index of its closing bracket */ private function parseAttributeGroup(Tokens $tokens, int $start, ?string $namespace, array $imports): array { @@ -364,9 +470,9 @@ private function parseAttributeGroup(Tokens $tokens, int $start, ?string $namesp continue; } - $attribute = $this->parseAttribute($tokens, $segmentStart, $i, $namespace, $aliases); - if ($attribute !== null) { - $attributes[] = $attribute; + // nothing after a trailing comma + if ($tokens->next($segmentStart - 1) !== $i) { + $attributes[] = $this->parseAttribute($tokens, $segmentStart, $i, $namespace, $aliases); } $segmentStart = $i + 1; if ($text === ']') { @@ -377,23 +483,19 @@ private function parseAttributeGroup(Tokens $tokens, int $start, ?string $namesp /** * @param array $aliases - * @return Attribute|null + * @return Attribute */ - private function parseAttribute(Tokens $tokens, int $start, int $end, ?string $namespace, array $aliases): ?array + private function parseAttribute(Tokens $tokens, int $start, int $end, ?string $namespace, array $aliases): array { - // the closing bracket after a trailing comma, which is not the attribute either $name = $tokens->next($start - 1); $resolved = $this->resolve($tokens->at($name)->text, $namespace, $aliases); - $isExpected = strcasecmp($resolved, Expected::class) === 0; - if (!$isExpected && strcasecmp($resolved, ExpectedFor::class) !== 0) { - return null; - } + $isExpectedFor = strcasecmp($resolved, ExpectedFor::class) === 0; $open = $tokens->next($name); $hasArguments = $tokens->at($open)->text === '('; $attributeEnd = $hasArguments ? $tokens->previous($end) : $name; $dataSet = null; - if (!$isExpected) { + if ($isExpectedFor) { $value = $hasArguments ? $this->dataSetArgument($tokens, $open, $attributeEnd) : null; $dataSet = $value === null ? null : $this->stringValue($tokens->at($value)->text); } @@ -402,13 +504,59 @@ private function parseAttribute(Tokens $tokens, int $start, int $end, ?string $n 'start' => $tokens->at($name)->pos, 'end' => $tokens->at($attributeEnd)->pos + strlen($tokens->at($attributeEnd)->text), 'name' => $tokens->at($name)->text, + 'class' => $resolved, 'dataSet' => $dataSet, + 'size' => $hasArguments ? $this->sizeArgument($tokens, $open, $attributeEnd) : null, 'line' => $tokens->at($name)->line, - // scan() rejects it only on the method it looks for - 'literal' => $isExpected || $dataSet !== null, + // member() rejects it only on the method it looks for + 'literal' => !$isExpectedFor || $dataSet !== null, ]; } + /** + * The size argument of a complexity class as written: the first argument, unless it is named, + * else the one named size. + * + * @param int $open the opening parenthesis + * @param int $close the closing parenthesis + * @return string|null null when the attribute has no such argument + */ + private function sizeArgument(Tokens $tokens, int $open, int $close): ?string + { + $nesting = 0; + $argumentStart = $open + 1; + for ($i = $argumentStart; $i <= $close; $i++) { + $text = $tokens->at($i)->text; + if (in_array($text, ['(', '['], true)) { + $nesting++; + continue; + } + if ($nesting > 0) { + if (in_array($text, [')', ']'], true)) { + $nesting--; + } + continue; + } + if ($text !== ',' && $i !== $close) { + continue; + } + + $first = $tokens->next($argumentStart - 1); + $named = $tokens->at($first)->is(T_STRING) && $tokens->at($tokens->next($first))->text === ':'; + if ($first !== $i && (!$named || $tokens->at($first)->text === 'size')) { + $argument = ''; + for ($j = $first; $j <= $tokens->previous($i); $j++) { + $argument .= $tokens->at($j)->text; + } + + return $argument; + } + $argumentStart = $i + 1; + } + + return null; + } + /** * The string literal of the data set: the named argument dataSet, or the first argument. * diff --git a/src/Report/ConsoleReport.php b/src/Report/ConsoleReport.php index 41aecb5..1d1efd2 100644 --- a/src/Report/ConsoleReport.php +++ b/src/Report/ConsoleReport.php @@ -68,7 +68,7 @@ public function summary(Report $report, string $reproduceCommand, string $accept $output .= "\n" . $this->style($colors, '1', 'Reproduce locally') . "\n"; $output .= ' ' . $reproduceCommand . "\n"; - if ($report->hasNumbersToAccept()) { + if ($report->hasChangesToAccept()) { $output .= "\n" . $this->style($colors, '1', 'Accept the new numbers') . "\n"; $output .= ' ' . str_replace("\n", "\n ", $acceptCommand) . "\n"; } diff --git a/src/Report/MarkdownReport.php b/src/Report/MarkdownReport.php index 39f0e0c..56180e5 100644 --- a/src/Report/MarkdownReport.php +++ b/src/Report/MarkdownReport.php @@ -49,7 +49,7 @@ public function render(Report $report, string $reproduceCommand, string $acceptC } if ($sections !== []) { $output .= "### Reproduce locally\n\n```\n" . $reproduceCommand . "\n```\n\n"; - if ($report->hasNumbersToAccept()) { + if ($report->hasChangesToAccept()) { $output .= "### Accept the new numbers\n\n```\n" . $acceptCommand . "\n```\n\n"; } } diff --git a/src/Report/Report.php b/src/Report/Report.php index 2b9b22e..79524d1 100644 --- a/src/Report/Report.php +++ b/src/Report/Report.php @@ -58,11 +58,14 @@ public function fails(): bool return false; } - /** Whether tally accept has numbers to write: of a slower, faster, or new case it can write */ - public function hasNumbersToAccept(): bool + /** + * Whether tally accept has something to write: the numbers of a slower, faster, or new case, or + * the complexity class of a case that grew more than its class allows + */ + public function hasChangesToAccept(): bool { foreach ($this->cases as $case) { - if ($case->acceptable && $case->status !== Status::Ok && $case->status !== Status::NotMeasured) { + if ($case->status === Status::Slower || $case->acceptable && $case->status !== Status::Ok && $case->status !== Status::NotMeasured) { return true; } } diff --git a/src/Report/Section.php b/src/Report/Section.php index 9850977..61a168f 100644 --- a/src/Report/Section.php +++ b/src/Report/Section.php @@ -40,7 +40,7 @@ public function text(): string }; } - /** A case compared by its growth has no numbers to accept, its complexity class is what changes */ + /** A case compared by its growth has no numbers to accept, tally accept changes its complexity class */ private function slowerText(): string { $growing = 0; @@ -50,8 +50,8 @@ private function slowerText(): string return 'These benchmarks do more work than expected. Find out why before merging. If it is the price of a fix, ' . match ($growing) { 0 => 'accept the new numbers.', - count($this->cases) => 'loosen the complexity class.', - default => 'accept the new numbers or loosen the complexity class.', + count($this->cases) => 'accept the new complexity class.', + default => 'accept the new numbers and complexity classes.', }; } diff --git a/tests/Fixtures/AcceptGrowth/AcceptGrowthBench.php b/tests/Fixtures/AcceptGrowth/AcceptGrowthBench.php new file mode 100644 index 0000000..a5bb4aa --- /dev/null +++ b/tests/Fixtures/AcceptGrowth/AcceptGrowthBench.php @@ -0,0 +1,64 @@ + [10]; + yield 'twenty' => [20]; + yield 'forty' => [40]; + } + + public static function fromZero(): iterable + { + yield 'zero' => [0]; + yield 'one' => [1]; + yield 'two' => [2]; + } + + #[DataProvider('sizes')] + #[Constant('size')] + public function benchUp(int $size): void + { + } + + #[DataProvider('sizes')] + #[Linear(size: 'size')] + public function benchDown(int $size): void + { + } + + #[DataProvider('sizes')] + #[Linearithmic('size')] + public function benchKept(int $size): void + { + } + + #[DataProvider('sizes')] + #[Complexity(size: 'size', formula: 'n^3')] + public function benchFormula(int $size): void + { + } + + #[DataProvider('fromZero')] + #[Constant('size')] + public function benchUnbounded(int $size): void + { + } + + #[DataProvider('sizes')] + #[Constant('size')] + public function benchNotMeasured(int $size): void + { + } + +} diff --git a/tests/Fixtures/Results/accept-growth/benchDown-forty.json b/tests/Fixtures/Results/accept-growth/benchDown-forty.json new file mode 100644 index 0000000..c71b1d7 --- /dev/null +++ b/tests/Fixtures/Results/accept-growth/benchDown-forty.json @@ -0,0 +1,8 @@ +{ + "id": "Tally\\Tests\\Fixtures\\AcceptGrowth\\AcceptGrowthBench::benchDown@forty", + "metrics": { + "instructions": 1602, + "cycles": 1602, + "memory": 0 + } +} diff --git a/tests/Fixtures/Results/accept-growth/benchDown-ten.json b/tests/Fixtures/Results/accept-growth/benchDown-ten.json new file mode 100644 index 0000000..3800a07 --- /dev/null +++ b/tests/Fixtures/Results/accept-growth/benchDown-ten.json @@ -0,0 +1,8 @@ +{ + "id": "Tally\\Tests\\Fixtures\\AcceptGrowth\\AcceptGrowthBench::benchDown@ten", + "metrics": { + "instructions": 1000, + "cycles": 1000, + "memory": 0 + } +} diff --git a/tests/Fixtures/Results/accept-growth/benchDown-twenty.json b/tests/Fixtures/Results/accept-growth/benchDown-twenty.json new file mode 100644 index 0000000..7b0d6bf --- /dev/null +++ b/tests/Fixtures/Results/accept-growth/benchDown-twenty.json @@ -0,0 +1,8 @@ +{ + "id": "Tally\\Tests\\Fixtures\\AcceptGrowth\\AcceptGrowthBench::benchDown@twenty", + "metrics": { + "instructions": 1301, + "cycles": 1301, + "memory": 0 + } +} diff --git a/tests/Fixtures/Results/accept-growth/benchFormula-forty.json b/tests/Fixtures/Results/accept-growth/benchFormula-forty.json new file mode 100644 index 0000000..81f6448 --- /dev/null +++ b/tests/Fixtures/Results/accept-growth/benchFormula-forty.json @@ -0,0 +1,8 @@ +{ + "id": "Tally\\Tests\\Fixtures\\AcceptGrowth\\AcceptGrowthBench::benchFormula@forty", + "metrics": { + "instructions": 16000, + "cycles": 16000, + "memory": 0 + } +} diff --git a/tests/Fixtures/Results/accept-growth/benchFormula-ten.json b/tests/Fixtures/Results/accept-growth/benchFormula-ten.json new file mode 100644 index 0000000..b25f196 --- /dev/null +++ b/tests/Fixtures/Results/accept-growth/benchFormula-ten.json @@ -0,0 +1,8 @@ +{ + "id": "Tally\\Tests\\Fixtures\\AcceptGrowth\\AcceptGrowthBench::benchFormula@ten", + "metrics": { + "instructions": 1000, + "cycles": 1000, + "memory": 0 + } +} diff --git a/tests/Fixtures/Results/accept-growth/benchFormula-twenty.json b/tests/Fixtures/Results/accept-growth/benchFormula-twenty.json new file mode 100644 index 0000000..2062c81 --- /dev/null +++ b/tests/Fixtures/Results/accept-growth/benchFormula-twenty.json @@ -0,0 +1,8 @@ +{ + "id": "Tally\\Tests\\Fixtures\\AcceptGrowth\\AcceptGrowthBench::benchFormula@twenty", + "metrics": { + "instructions": 4000, + "cycles": 4000, + "memory": 0 + } +} diff --git a/tests/Fixtures/Results/accept-growth/benchKept-forty.json b/tests/Fixtures/Results/accept-growth/benchKept-forty.json new file mode 100644 index 0000000..8557f13 --- /dev/null +++ b/tests/Fixtures/Results/accept-growth/benchKept-forty.json @@ -0,0 +1,8 @@ +{ + "id": "Tally\\Tests\\Fixtures\\AcceptGrowth\\AcceptGrowthBench::benchKept@forty", + "metrics": { + "instructions": 6408, + "cycles": 6408, + "memory": 0 + } +} diff --git a/tests/Fixtures/Results/accept-growth/benchKept-ten.json b/tests/Fixtures/Results/accept-growth/benchKept-ten.json new file mode 100644 index 0000000..c59f614 --- /dev/null +++ b/tests/Fixtures/Results/accept-growth/benchKept-ten.json @@ -0,0 +1,8 @@ +{ + "id": "Tally\\Tests\\Fixtures\\AcceptGrowth\\AcceptGrowthBench::benchKept@ten", + "metrics": { + "instructions": 1000, + "cycles": 1000, + "memory": 0 + } +} diff --git a/tests/Fixtures/Results/accept-growth/benchKept-twenty.json b/tests/Fixtures/Results/accept-growth/benchKept-twenty.json new file mode 100644 index 0000000..095af69 --- /dev/null +++ b/tests/Fixtures/Results/accept-growth/benchKept-twenty.json @@ -0,0 +1,8 @@ +{ + "id": "Tally\\Tests\\Fixtures\\AcceptGrowth\\AcceptGrowthBench::benchKept@twenty", + "metrics": { + "instructions": 2602, + "cycles": 2602, + "memory": 0 + } +} diff --git a/tests/Fixtures/Results/accept-growth/benchNotMeasured-ten.json b/tests/Fixtures/Results/accept-growth/benchNotMeasured-ten.json new file mode 100644 index 0000000..36430c0 --- /dev/null +++ b/tests/Fixtures/Results/accept-growth/benchNotMeasured-ten.json @@ -0,0 +1,8 @@ +{ + "id": "Tally\\Tests\\Fixtures\\AcceptGrowth\\AcceptGrowthBench::benchNotMeasured@ten", + "metrics": { + "instructions": 1000, + "cycles": 1000, + "memory": 0 + } +} diff --git a/tests/Fixtures/Results/accept-growth/benchNotMeasured-twenty.json b/tests/Fixtures/Results/accept-growth/benchNotMeasured-twenty.json new file mode 100644 index 0000000..cacb46f --- /dev/null +++ b/tests/Fixtures/Results/accept-growth/benchNotMeasured-twenty.json @@ -0,0 +1,8 @@ +{ + "id": "Tally\\Tests\\Fixtures\\AcceptGrowth\\AcceptGrowthBench::benchNotMeasured@twenty", + "metrics": { + "instructions": 5000, + "cycles": 5000, + "memory": 0 + } +} diff --git a/tests/Fixtures/Results/accept-growth/benchUnbounded-one.json b/tests/Fixtures/Results/accept-growth/benchUnbounded-one.json new file mode 100644 index 0000000..d3e37f8 --- /dev/null +++ b/tests/Fixtures/Results/accept-growth/benchUnbounded-one.json @@ -0,0 +1,8 @@ +{ + "id": "Tally\\Tests\\Fixtures\\AcceptGrowth\\AcceptGrowthBench::benchUnbounded@one", + "metrics": { + "instructions": 5000, + "cycles": 5000, + "memory": 0 + } +} diff --git a/tests/Fixtures/Results/accept-growth/benchUnbounded-two.json b/tests/Fixtures/Results/accept-growth/benchUnbounded-two.json new file mode 100644 index 0000000..1d9f925 --- /dev/null +++ b/tests/Fixtures/Results/accept-growth/benchUnbounded-two.json @@ -0,0 +1,8 @@ +{ + "id": "Tally\\Tests\\Fixtures\\AcceptGrowth\\AcceptGrowthBench::benchUnbounded@two", + "metrics": { + "instructions": 100000, + "cycles": 100000, + "memory": 0 + } +} diff --git a/tests/Fixtures/Results/accept-growth/benchUnbounded-zero.json b/tests/Fixtures/Results/accept-growth/benchUnbounded-zero.json new file mode 100644 index 0000000..768d48e --- /dev/null +++ b/tests/Fixtures/Results/accept-growth/benchUnbounded-zero.json @@ -0,0 +1,8 @@ +{ + "id": "Tally\\Tests\\Fixtures\\AcceptGrowth\\AcceptGrowthBench::benchUnbounded@zero", + "metrics": { + "instructions": 1000, + "cycles": 1000, + "memory": 0 + } +} diff --git a/tests/Fixtures/Results/accept-growth/benchUp-forty.json b/tests/Fixtures/Results/accept-growth/benchUp-forty.json new file mode 100644 index 0000000..012af42 --- /dev/null +++ b/tests/Fixtures/Results/accept-growth/benchUp-forty.json @@ -0,0 +1,8 @@ +{ + "id": "Tally\\Tests\\Fixtures\\AcceptGrowth\\AcceptGrowthBench::benchUp@forty", + "metrics": { + "instructions": 1602, + "cycles": 1602, + "memory": 0 + } +} diff --git a/tests/Fixtures/Results/accept-growth/benchUp-ten.json b/tests/Fixtures/Results/accept-growth/benchUp-ten.json new file mode 100644 index 0000000..85454c3 --- /dev/null +++ b/tests/Fixtures/Results/accept-growth/benchUp-ten.json @@ -0,0 +1,8 @@ +{ + "id": "Tally\\Tests\\Fixtures\\AcceptGrowth\\AcceptGrowthBench::benchUp@ten", + "metrics": { + "instructions": 1000, + "cycles": 1000, + "memory": 0 + } +} diff --git a/tests/Fixtures/Results/accept-growth/benchUp-twenty.json b/tests/Fixtures/Results/accept-growth/benchUp-twenty.json new file mode 100644 index 0000000..b27a984 --- /dev/null +++ b/tests/Fixtures/Results/accept-growth/benchUp-twenty.json @@ -0,0 +1,8 @@ +{ + "id": "Tally\\Tests\\Fixtures\\AcceptGrowth\\AcceptGrowthBench::benchUp@twenty", + "metrics": { + "instructions": 1301, + "cycles": 1301, + "memory": 0 + } +} diff --git a/tests/Integration/Cli/CompareAndAcceptCommandTest.php b/tests/Integration/Cli/CompareAndAcceptCommandTest.php index b71fcda..ebc95f2 100644 --- a/tests/Integration/Cli/CompareAndAcceptCommandTest.php +++ b/tests/Integration/Cli/CompareAndAcceptCommandTest.php @@ -115,6 +115,58 @@ public function testAcceptWritesTheChangedCasesOnly(): void ); } + public function testAcceptReplacesAPresetComplexityClassWithTheNextOneTheGrowthFits(): void + { + $bench = $this->directory . '/AcceptGrowthBench.php'; + $original = (string) file_get_contents(__DIR__ . '/../../Fixtures/AcceptGrowth/AcceptGrowthBench.php'); + file_put_contents($bench, $original); + $class = 'Tally\\Tests\\Fixtures\\AcceptGrowth\\AcceptGrowthBench'; + + [$exitCode, $output, $errorOutput] = InProcessTally::run(__DIR__, [], ['accept', __DIR__ . '/../../Fixtures/Results/accept-growth', $bench]); + + $this->assertSame(2, $exitCode); + $this->assertSame('Cannot accept ' . $class . "::benchUnbounded: no preset complexity class bounds how its instructions grow, write a #[Complexity] that does.\n", $errorOutput); + $this->assertSame( + 'faster ' . $class . "::benchDown: O(n) to O(log n)\n" + . 'slower ' . $class . "::benchUp: O(1) to O(log n)\n" + . "\nAccepted the complexity classes of 2 benchmarks.\n", + $output, + ); + $this->assertSame( + str_replace( + [ + "use Tally\\Attribute\\Linear;\n", + "use Tally\\Attribute\\Linearithmic;\n", + "#[Constant('size')]\n\tpublic function benchUp(", + "#[Linear(size: 'size')]", + ], + [ + '', + "use Tally\\Attribute\\Linearithmic;\nuse Tally\\Attribute\\Logarithmic;\n", + "#[Logarithmic('size')]\n\tpublic function benchUp(", + "#[Logarithmic(size: 'size')]", + ], + $original, + ), + file_get_contents($bench), + ); + } + + public function testAcceptReportsAComplexityClassThatCannotBeWritten(): void + { + $bench = $this->directory . '/AcceptGrowthBench.php'; + file_put_contents($bench, (string) file_get_contents(__DIR__ . '/../../Fixtures/AcceptGrowth/AcceptGrowthBench.php')); + chmod($bench, 0444); + $class = 'Tally\\Tests\\Fixtures\\AcceptGrowth\\AcceptGrowthBench'; + + [$exitCode, $output, $errorOutput] = InProcessTally::run(__DIR__, [], ['accept', '--filter=/bench(Up|Down)/', __DIR__ . '/../../Fixtures/Results/accept-growth', $bench]); + chmod($bench, 0644); + + $this->assertSame(2, $exitCode); + $this->assertSame(sprintf("Cannot accept %1\$s::benchDown, %1\$s::benchUp: Cannot write %2\$s.\n", $class, $bench), $errorOutput); + $this->assertSame("Accepted the numbers of 0 cases.\n", $output); + } + public function testAcceptReadsTheBenchmarksFromTheConfiguration(): void { [$exitCode, $output, $errorOutput] = InProcessTally::run(__DIR__, [], ['accept', '--config=' . self::COMPARE . '/tally.php', __DIR__ . '/../../Fixtures/Results/ok']); diff --git a/tests/Integration/Cli/RunCommandTest.php b/tests/Integration/Cli/RunCommandTest.php index f327d95..bc99d73 100644 --- a/tests/Integration/Cli/RunCommandTest.php +++ b/tests/Integration/Cli/RunCommandTest.php @@ -94,18 +94,16 @@ public function testComparesTheGrowthWithTheBaseThatFinishesLater(): void . "Benchmarks failed: 1 slower, 1 unchanged\n" . "Allowed difference: instructions grow at most as O(n) from the smallest data set, ±10 % of each metric\n\n" . "Slower than expected\n" - . "These benchmarks do more work than expected. Find out why before merging. If it is the price of a fix, loosen the complexity class.\n\n" + . "These benchmarks do more work than expected. Find out why before merging. If it is the price of a fix, accept the new complexity class.\n\n" . " Benchmark Metric Expected Measured Change Allowed\n" . ' ' . $growth . "@large instructions 2.000K 2.400K +20.0 % ±10 %\n\n" . "Reproduce locally\n" - . " vendor/bin/tally run --filter='" . $growth . "@large'\n", + . " vendor/bin/tally run --filter='" . $growth . "@large'\n\n" + . "Accept the new numbers\n" + . " vendor/bin/tally accept '" . $this->output . "-growth'\n", $output, ); - - [$exitCode, $output, $errorOutput] = InProcessTally::run(__DIR__, $environment, ['accept', $this->output . '-growth', __DIR__ . '/../../Fixtures/Growth']); (new DirectoryRemover())->remove($this->output . '-growth'); - $this->assertSame(0, $exitCode, $errorOutput); - $this->assertSame("Accepted the numbers of 0 cases.\n", $output); } public function testPassesWhenEveryCaseMatches(): void diff --git a/tests/LinearGrowth.php b/tests/LinearGrowth.php new file mode 100644 index 0000000..09b55d5 --- /dev/null +++ b/tests/LinearGrowth.php @@ -0,0 +1,27 @@ +index(''), $base->class, $base->method, new Linear('numbers')), + $base, + $size, + $growth, + ); + } + +} diff --git a/tests/Unit/Comparison/ComparatorTest.php b/tests/Unit/Comparison/ComparatorTest.php index f94f13b..8742e5c 100644 --- a/tests/Unit/Comparison/ComparatorTest.php +++ b/tests/Unit/Comparison/ComparatorTest.php @@ -12,13 +12,13 @@ use Tally\Comparison\Percent; use Tally\Comparison\Status; use Tally\Comparison\TolerancePolicy; -use Tally\Complexity\GrowthPoint; use Tally\Expectation\ExpectationException; use Tally\Expectation\SourceLocation; use Tally\Measurement; use Tally\Metrics; use Tally\Tests\Fixtures\Tolerance\Exact; use Tally\Tests\InMemoryExpectations; +use Tally\Tests\LinearGrowth; use function array_map; use function sprintf; @@ -165,7 +165,7 @@ public function testCompareGrowth(string $id, TolerancePolicy $tolerance, ?Measu $comparison = (new Comparator())->compareGrowth( CaseId::fromString($id), $location, - new GrowthPoint('O(n)', CaseId::fromString('A::bench@small'), 10.0), + LinearGrowth::point(CaseId::fromString('A::bench@small'), 10.0, 10.0), $tolerance, $measurement, $base, @@ -192,7 +192,7 @@ public function testGrowthRoundsTheExpectedInstructions(): void $comparison = (new Comparator())->compareGrowth( CaseId::fromString('A::bench@large'), new SourceLocation('/bench/ABench.php', 12), - new GrowthPoint('O(n)', CaseId::fromString('A::bench@small'), 1.25), + LinearGrowth::point(CaseId::fromString('A::bench@small'), 1.25, 1.25), new Exact(), Measurement::measured(CaseId::fromString('A::bench@large'), Metrics::fromArray(['instructions' => 4])), Measurement::measured(CaseId::fromString('A::bench@small'), Metrics::fromArray(['instructions' => 3])), @@ -208,8 +208,8 @@ public function testGrowthRoundsTheExpectedInstructions(): void public function testCompareCaseComparesByExpectationsOrByGrowth(): void { $expectations = new InMemoryExpectations(Metrics::fromArray(['instructions' => 100]), '/bench/ABench.php'); - $small = new BenchmarkCase(CaseId::fromString('A::bench@small'), '/benchmarks/FooBench.php', [], 0, [], [], $expectations, new RelativeTolerance(0.1), new GrowthPoint('O(n)', CaseId::fromString('A::bench@small'), 1.0)); - $large = $small->withGrowth(new GrowthPoint('O(n)', CaseId::fromString('A::bench@small'), 3.0)); + $small = new BenchmarkCase(CaseId::fromString('A::bench@small'), '/benchmarks/FooBench.php', [], 0, [], [], $expectations, new RelativeTolerance(0.1), LinearGrowth::point(CaseId::fromString('A::bench@small'), 1.0, 1.0)); + $large = $small->withGrowth(LinearGrowth::point(CaseId::fromString('A::bench@small'), 3.0, 3.0)); $large = new BenchmarkCase(CaseId::fromString('A::bench@large'), '/benchmarks/FooBench.php', [], 0, [], [], $expectations, new RelativeTolerance(0.1), $large->growth); $plain = new BenchmarkCase(CaseId::fromString('A::plain'), '/benchmarks/FooBench.php', [], 0, [], [], $expectations, new RelativeTolerance(0.1), null); $measurements = [ diff --git a/tests/Unit/Complexity/PresetFitTest.php b/tests/Unit/Complexity/PresetFitTest.php new file mode 100644 index 0000000..1f2999b --- /dev/null +++ b/tests/Unit/Complexity/PresetFitTest.php @@ -0,0 +1,179 @@ +}> + */ + private static function presets(): array + { + return [ + [new Constant(self::SIZE), [1000, 1000, 1000]], + // log(20) / log(10) is 1.301 + [new Logarithmic(self::SIZE), [1000, 1301, 1602]], + [new Linear(self::SIZE), [1000, 2000, 4000]], + // 20 log(20) / (10 log(10)) is 2.602 + [new Linearithmic(self::SIZE), [1000, 2602, 6408]], + [new Quadratic(self::SIZE), [1000, 4000, 16000]], + [new Polynomial(self::SIZE, 3), [1000, 8000, 64000]], + [new Polynomial(self::SIZE, 4), [1000, 16000, 256000]], + // 2^10 and 2^30 times, which no polynomial below degree 15 allows at size 40 + [new Exponential(self::SIZE), [1000, 1024000, 1073741824000]], + ]; + } + + /** + * @return iterable, ComplexityClass}> + */ + public static function dataNextClass(): iterable + { + $presets = self::presets(); + foreach ($presets as $i => [$lower, $lowerGrowth]) { + $next = $presets[$i + 1] ?? null; + if ($next === null) { + break; + } + + [$higher, $higherGrowth] = $next; + yield $lower->describe() . ' up to ' . $higher->describe() => [$lower, $higherGrowth, $higher]; + yield $higher->describe() . ' down to ' . $lower->describe() => [$higher, $lowerGrowth, $lower]; + } + } + + /** + * @param non-empty-list $instructions at the sizes 10, 20, and 40 + */ + #[DataProvider('dataNextClass')] + public function testNextClass(ComplexityClass $current, array $instructions, ComplexityClass $fitted): void + { + // equal, not only described the same: #[Quadratic] and not #[Polynomial(degree: 2)] + $this->assertEquals($fitted, self::fit($current, self::points([10, 20, 40], $instructions))); + } + + /** + * @return iterable}> + */ + public static function dataKeepsTheClass(): iterable + { + yield 'the class that fits' => [new Linearithmic(self::SIZE), [1000, 2602, 6408]]; + yield 'a polynomial of degree 1 is linear' => [new Polynomial(self::SIZE, 1), [1000, 2000, 4000]]; + yield 'a polynomial of degree 2 is quadratic' => [new Polynomial(self::SIZE, 2), [1000, 4000, 16000]]; + yield 'not a preset' => [new Complexity(self::SIZE, 'n^3'), [1000, 2000, 4000]]; + } + + /** + * @param non-empty-list $instructions at the sizes 10, 20, and 40 + */ + #[DataProvider('dataKeepsTheClass')] + public function testKeepsTheClass(ComplexityClass $current, array $instructions): void + { + $this->assertSame($current, self::fit($current, self::points([10, 20, 40], $instructions))); + } + + public function testTheBaseIsTheSmallestSize(): void + { + $fit = self::fit(new Linear(self::SIZE), self::points([40, 10, 20], [16000, 1000, 4000])); + + $this->assertSame('O(n^2)', $fit?->describe()); + } + + public function testWithinTheTolerance(): void + { + // 9 % more than the formula allows at size 20 + $fit = self::fit(new Linear(self::SIZE), self::points([10, 20, 40], [1000, 2180, 4000])); + + $this->assertSame('O(n)', $fit?->describe()); + $this->assertSame('O(n log n)', self::fit(new Linear(self::SIZE), self::points([10, 20, 40], [1000, 2220, 4000]))?->describe()); + } + + public function testAClassThatIsNotDefinedAtEverySizeIsSkipped(): void + { + // log(1) is 0 + $this->assertEquals(new Linear(self::SIZE), self::fit(new Constant(self::SIZE), self::points([1, 2, 4], [1000, 2000, 4000]))); + } + + /** + * @return iterable + */ + public static function dataRoundsTheExpectedInstructions(): iterable + { + // 3 times 2.5 rounds up to 8, which 8 is within + yield 'up' => [25, new Linear(self::SIZE)]; + // 3 times 2.4 rounds down to 7, which 8 is 14 % more than + yield 'down' => [24, new Linearithmic(self::SIZE)]; + } + + #[DataProvider('dataRoundsTheExpectedInstructions')] + public function testRoundsTheExpectedInstructionsLikeTallyRun(int $size, ComplexityClass $fitted): void + { + $this->assertEquals($fitted, self::fit(new Linear(self::SIZE), self::points([10, $size], [3, 8]))); + } + + public function testExponentialAllowsLessThanAPolynomialAtSmallSizes(): void + { + // n^3 allows 27 times at size 6, 2^n 16 times + $fit = self::fit(new Polynomial(self::SIZE, 3), self::points([2, 4, 6], [1000, 4000, 16000])); + + $this->assertSame('O(2^n)', $fit?->describe()); + } + + public function testAPolynomialThatAllowsLessThanExponential(): void + { + // n^3 allows 27 times at size 6 and 2^n 16 times, which the instructions grew more than + $fit = self::fit(new Exponential(self::SIZE), self::points([2, 4, 6], [1000, 8000, 27000])); + + $this->assertSame('O(n^3)', $fit?->describe()); + } + + public function testNoPresetFits(): void + { + // every formula but 1 and 2^n is 0 or undefined at size 0, and 2^n allows 4 times at size 2 + $this->assertNull(self::fit(new Linear(self::SIZE), self::points([0, 1, 2], [1000, 5000, 100000]))); + } + + /** + * @param non-empty-list $points + */ + private static function fit(ComplexityClass $current, array $points): ?ComplexityClass + { + $expressions = new Expressions(); + + return (new PresetFit($expressions->parser(), $expressions->evaluator(), new Comparator()))->fit($current, $points, new RelativeTolerance(0.1)); + } + + /** + * @param non-empty-list $sizes + * @param non-empty-list $instructions + * @return non-empty-list + */ + private static function points(array $sizes, array $instructions): array + { + return array_map(static fn (int $size, int $count): array => [(float) $size, $count], $sizes, $instructions); + } + +} diff --git a/tests/Unit/Discovery/CaseExpanderTest.php b/tests/Unit/Discovery/CaseExpanderTest.php index 2b2d678..09283a4 100644 --- a/tests/Unit/Discovery/CaseExpanderTest.php +++ b/tests/Unit/Discovery/CaseExpanderTest.php @@ -253,7 +253,7 @@ private static function expand(string $class, string $method, Configuration $con */ private static function rewrite(string $code, BenchmarkCase $case, Metrics $metrics): array { - $plan = (new ExpectationChanges())->plan([[$case, $metrics]], [$case->expectations->file() => $code]); + $plan = (new ExpectationChanges())->plan([[$case, $metrics]], [], [$case->expectations->file() => $code]); self::assertSame([], $plan->failures); return array_map(static fn (FileChange $file): string => $file->after, $plan->files); diff --git a/tests/Unit/Discovery/GrowthPointsTest.php b/tests/Unit/Discovery/GrowthPointsTest.php index 2d599aa..b17a862 100644 --- a/tests/Unit/Discovery/GrowthPointsTest.php +++ b/tests/Unit/Discovery/GrowthPointsTest.php @@ -49,7 +49,7 @@ public function testPoints(string $method, string $bound, string $base, array $g $actual = []; foreach (self::expand($method) as $case) { $this->assertNotNull($case->growth); - $this->assertSame($bound, $case->growth->bound); + $this->assertSame($bound, $case->growth->attribute->complexity->describe()); $this->assertSame(ComplexityBench::class . '::' . $method . '@' . $base, $case->growth->base->toString()); $actual[$case->id->dataSet ?? ''] = $case->growth->growth; } diff --git a/tests/Unit/Expectation/ExpectationChangesTest.php b/tests/Unit/Expectation/ExpectationChangesTest.php index b43fe1e..0e2aea5 100644 --- a/tests/Unit/Expectation/ExpectationChangesTest.php +++ b/tests/Unit/Expectation/ExpectationChangesTest.php @@ -3,11 +3,14 @@ namespace Tally\Tests\Unit\Expectation; use PHPUnit\Framework\TestCase; +use Tally\Attribute\Linear; +use Tally\Attribute\Quadratic; use Tally\Attribute\RelativeTolerance; use Tally\BenchmarkCase; use Tally\CaseId; use Tally\Expectation\AttributeExpectations; use Tally\Expectation\CodeEdit; +use Tally\Expectation\ComplexityAttribute; use Tally\Expectation\ExpectationChanges; use Tally\Expectation\ExpectationSource; use Tally\Expectation\ExpectedAttributeRewriter; @@ -29,6 +32,7 @@ public function testCombinesTheChangesOfAFileInTheOrderOfTheCases(): void [self::case('B::benchB', new InMemoryExpectations(null, '/b.php')), Metrics::fromArray(['instructions' => 2])], [self::case('A::benchC', new InMemoryExpectations(null, '/a.php')), Metrics::fromArray(['instructions' => 3])], ], + [], ['/a.php' => " " ''], ); @@ -54,6 +58,7 @@ public function testACaseThatCannotBeRewrittenDoesNotStopTheOthersInItsFile(): v [self::case('App\SortBench::benchLarge@large', new AttributeExpectations($rewriter, '/SortBench.php', $index, 'App\SortBench', 'benchLarge', 'large', null, false)), Metrics::fromArray(['instructions' => 1, 'cycles' => 1, 'memory' => 0])], [self::case('App\SortBench::benchSort', new AttributeExpectations($rewriter, '/SortBench.php', $index, 'App\SortBench', 'benchSort', null, null, false)), Metrics::fromArray(['instructions' => 2, 'cycles' => 2, 'memory' => 0])], ], + [], ['/SortBench.php' => $code, '/GoneBench.php' => false], ); @@ -88,6 +93,7 @@ public function testCasesThatAddTheSameImportAddItOnceAndKeepTheImportsSorted(): [self::case('App\SortBench::benchB', new AttributeExpectations($rewriter, '/SortBench.php', $index, 'App\SortBench', 'benchB', null, null, false)), $metrics], [self::case('App\SortBench::benchC', new AttributeExpectations($rewriter, '/SortBench.php', $index, 'App\SortBench', 'benchC', null, null, false)), $metrics], ], + [], ['/SortBench.php' => $code], ); @@ -106,6 +112,31 @@ public function testCasesThatAddTheSameImportAddItOnceAndKeepTheImportsSorted(): $this->assertSame([], $plan->failures); } + public function testAComplexityClassAndExpectationsInOneFileAreOneChange(): void + { + $code = "index($code); + + $plan = (new ExpectationChanges())->plan( + [[self::case('App\SortBench::benchNew', new AttributeExpectations($rewriter, '/SortBench.php', $index, 'App\SortBench', 'benchNew', null, null, false)), Metrics::fromArray(['instructions' => 1, 'cycles' => 2, 'memory' => 0])]], + [[CaseId::fromString('App\SortBench::benchGrow'), new ComplexityAttribute($rewriter, '/SortBench.php', $index, 'App\SortBench', 'benchGrow', new Linear('numbers')), new Quadratic('numbers')]], + ['/SortBench.php' => $code], + ); + + $this->assertSame( + [[ + '/SortBench.php', + $code, + "files), + ); + $this->assertSame([], $plan->failures); + } + public function testEditsAtOneOffsetGoInTheNaturalOrderOfTheirTextAndInsertionsBeforeReplacements(): void { $plan = (new ExpectationChanges())->plan( @@ -114,6 +145,7 @@ public function testEditsAtOneOffsetGoInTheNaturalOrderOfTheirTextAndInsertionsB [self::case('A::bench2', new FixedEdits('/a.php', [new CodeEdit(1, 0, '<2>')])), Metrics::none()], [self::case('A::bench1', new FixedEdits('/a.php', [new CodeEdit(1, 0, '<10>'), new CodeEdit(3, 0, '<3>')])), Metrics::none()], ], + [], ['/a.php' => 'abcd'], ); @@ -130,6 +162,7 @@ public function testACaseWhoseEditsOverlapAnotherCaseIsLeftOut(): void [self::case('A::benchC', new FixedEdits('/a.php', [new CodeEdit(1, 2, 'X'), new CodeEdit(3, 1, 'W')])), Metrics::none()], [self::case('A::benchD', new FixedEdits('/a.php', [new CodeEdit(2, 1, 'V')])), Metrics::none()], ], + [], ['/a.php' => 'abcd'], ); @@ -139,7 +172,7 @@ public function testACaseWhoseEditsOverlapAnotherCaseIsLeftOut(): void public function testAFileThatWasNotReadCannotBeRewritten(): void { - $plan = (new ExpectationChanges())->plan([[self::case('A::bench', new InMemoryExpectations(null, '/a.php')), Metrics::none()]], []); + $plan = (new ExpectationChanges())->plan([[self::case('A::bench', new InMemoryExpectations(null, '/a.php')), Metrics::none()]], [], []); $this->assertSame([], $plan->files); $this->assertSame(['A::bench' => 'Cannot read /a.php.'], $plan->failures); @@ -147,7 +180,7 @@ public function testAFileThatWasNotReadCannotBeRewritten(): void public function testNothingToChange(): void { - $plan = (new ExpectationChanges())->plan([], ['/a.php' => '']); + $plan = (new ExpectationChanges())->plan([], [], ['/a.php' => '']); $this->assertSame([], $plan->files); $this->assertSame([], $plan->failures); diff --git a/tests/Unit/Expectation/ExpectedAttributeRewriterTest.php b/tests/Unit/Expectation/ExpectedAttributeRewriterTest.php index ae85210..76c1259 100644 --- a/tests/Unit/Expectation/ExpectedAttributeRewriterTest.php +++ b/tests/Unit/Expectation/ExpectedAttributeRewriterTest.php @@ -5,12 +5,26 @@ use InvalidArgumentException; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; +use Tally\Attribute\Constant; +use Tally\Attribute\Linear; +use Tally\Attribute\Linearithmic; +use Tally\Attribute\Logarithmic; +use Tally\Attribute\Polynomial; +use Tally\Attribute\Quadratic; +use Tally\CaseId; +use Tally\Complexity\ComplexityClass; +use Tally\Expectation\ChangePlan; +use Tally\Expectation\ComplexityAttribute; +use Tally\Expectation\ExpectationChanges; use Tally\Expectation\ExpectationException; use Tally\Expectation\ExpectedAttributeRewriter; +use Tally\Expectation\FileChange; use Tally\Expectation\MethodNotFoundException; use Tally\Metrics; use Tally\Tests\Fixtures\Rewriter\SortBench; +use function array_map; use function file_get_contents; +use function str_contains; use function str_replace; use function substr; @@ -414,6 +428,153 @@ private static function locate(string $code, string $class, string $method, ?str return $rewriter->locate($rewriter->index($code), $class, $method, $dataSet); } + /** + * @return iterable + */ + public static function dataReplaceComplexity(): iterable + { + $head = " [ + new Linear('ignored'), + new Linearithmic('ignored'), + $head . "use Tally\\Attribute\\DataProvider;\nuse Tally\\Attribute\\Linear;\nuse function sort;\nclass FooBench {\n\t#[DataProvider('lists')]\n\t#[Linear('numbers')]\n\tpublic function bench() {}\n}\n", + $head . "use Tally\\Attribute\\DataProvider;\nuse Tally\\Attribute\\Linearithmic;\nuse function sort;\nclass FooBench {\n\t#[DataProvider('lists')]\n\t#[Linearithmic('numbers')]\n\tpublic function bench() {}\n}\n", + ]; + yield 'a named size' => [ + new Linear('ignored'), + new Constant('ignored'), + $head . "use Tally\\Attribute\\Linear;\n\nclass FooBench {\n\t#[Linear(size: 'input.rows')]\n\tpublic function bench() {}\n}\n", + $head . "use Tally\\Attribute\\Constant;\n\nclass FooBench {\n\t#[Constant(size: 'input.rows')]\n\tpublic function bench() {}\n}\n", + ]; + yield 'a polynomial of another degree' => [ + new Polynomial('ignored', 3), + new Polynomial('ignored', 4), + $head . "use Tally\\Attribute\\Polynomial;\nclass FooBench {\n\t#[Polynomial('numbers', degree: 3)]\n\tpublic function bench() {}\n}\n", + $head . "use Tally\\Attribute\\Polynomial;\nclass FooBench {\n\t#[Polynomial('numbers', degree: 4)]\n\tpublic function bench() {}\n}\n", + ]; + yield 'the size after the degree' => [ + new Polynomial('ignored', 3), + new Quadratic('ignored'), + $head . "use Tally\\Attribute\\Polynomial;\nuse Tally\\Attribute\\Quadratic;\nclass FooBench {\n\t#[Polynomial(degree: 3,size: self::SIZES[0])]\n\tpublic function bench() {}\n\t#[Quadratic('numbers')]\n\tpublic function benchOther() {}\n}\n", + $head . "use Tally\\Attribute\\Quadratic;\nclass FooBench {\n\t#[Quadratic(size: self::SIZES[0])]\n\tpublic function bench() {}\n\t#[Quadratic('numbers')]\n\tpublic function benchOther() {}\n}\n", + ]; + yield 'only the attribute of the method, the class used elsewhere stays imported' => [ + new Linear('ignored'), + new Polynomial('ignored', 3), + $head . "use Tally\\Attribute\\Linear;\nclass FooBench {\n\t#[Linear('numbers')]\n\tpublic function benchOther() {}\n\t#[Linear('numbers'), Warmup(1)]\n\tpublic function bench() {}\n}\n", + $head . "use Tally\\Attribute\\Linear;\nuse Tally\\Attribute\\Polynomial;\nclass FooBench {\n\t#[Linear('numbers')]\n\tpublic function benchOther() {}\n\t#[Polynomial('numbers', degree: 3), Warmup(1)]\n\tpublic function bench() {}\n}\n", + ]; + yield 'after an earlier import, a constant, and a comma in brackets' => [ + new Linear('ignored'), + new Constant('ignored'), + $head . "use App\\Sizes;\nuse Tally\\Attribute\\Linear; // O(n)\nclass FooBench {\n\t#[Linear(SIZE . ['rows', 'columns'][0])]\n\tpublic function bench() {}\n}\n", + $head . "use App\\Sizes;\nuse Tally\\Attribute\\Constant; // O(n)\nclass FooBench {\n\t#[Constant(SIZE . ['rows', 'columns'][0])]\n\tpublic function bench() {}\n}\n", + ]; + yield 'the new import goes before a later one' => [ + new Linear('ignored'), + new Constant('ignored'), + $head . "use Tally\\Attribute\\DataProvider;\nuse Tally\\Attribute\\Linear;\nclass FooBench {\n\t#[Linear('numbers')]\n\tpublic function bench() {}\n}\n", + $head . "use Tally\\Attribute\\Constant;\nuse Tally\\Attribute\\DataProvider;\nclass FooBench {\n\t#[Constant('numbers')]\n\tpublic function bench() {}\n}\n", + ]; + yield 'the new import takes the place of the replaced one before a later one' => [ + new Linear('ignored'), + new Constant('ignored'), + $head . "use Tally\\Attribute\\Linear; // O(n)\nuse Tally\\Attribute\\Polynomial;\nclass FooBench {\n\t#[Linear('numbers')]\n\tpublic function bench() {}\n}\n", + $head . "use Tally\\Attribute\\Constant; // O(n)\nuse Tally\\Attribute\\Polynomial;\nclass FooBench {\n\t#[Constant('numbers')]\n\tpublic function bench() {}\n}\n", + ]; + yield 'the new import goes after a later one' => [ + new Linear('ignored'), + new Logarithmic('ignored'), + $head . "use Tally\\Attribute\\Linear;\nuse Tally\\Attribute\\Linearithmic;\nclass FooBench {\n\t#[Linear('numbers')]\n\tpublic function bench() {}\n}\n", + $head . "use Tally\\Attribute\\Linearithmic;\nuse Tally\\Attribute\\Logarithmic;\nclass FooBench {\n\t#[Logarithmic('numbers')]\n\tpublic function bench() {}\n}\n", + ]; + yield 'an import without a line break after it' => [ + new Linear('ignored'), + new Quadratic('ignored'), + $head . "use Tally\\Attribute\\Linear; use Tally\\Attribute\\Polynomial;\nclass FooBench {\n\t#[Linear('numbers')]\n\tpublic function bench() {}\n}\n", + $head . " use Tally\\Attribute\\Polynomial;\nuse Tally\\Attribute\\Quadratic;\nclass FooBench {\n\t#[Quadratic('numbers')]\n\tpublic function bench() {}\n}\n", + ]; + yield 'a trailing comma' => [ + new Linear('ignored'), + new Constant('ignored'), + $head . "use Tally\\Attribute\\Linear;\nclass FooBench {\n\t#[Linear('numbers'),]\n\tpublic function bench() {}\n}\n", + $head . "use Tally\\Attribute\\Constant;\nclass FooBench {\n\t#[Constant('numbers'),]\n\tpublic function bench() {}\n}\n", + ]; + yield 'an import among others in one statement stays' => [ + new Linear('ignored'), + new Constant('ignored'), + $head . "use Tally\\Attribute\\{DataProvider, Linear};\nclass FooBench {\n\t#[Linear('numbers')]\n\tpublic function bench() {}\n}\n", + $head . "use Tally\\Attribute\\Constant;\nuse Tally\\Attribute\\{DataProvider, Linear};\nclass FooBench {\n\t#[Constant('numbers')]\n\tpublic function bench() {}\n}\n", + ]; + yield 'an alias' => [ + new Linear('ignored'), + new Logarithmic('ignored'), + $head . "use Tally\\Attribute as T;\nuse Tally\\Attribute\\Linear as OfN;\nclass FooBench {\n\t#[OfN('numbers')]\n\tpublic function bench() {}\n}\n", + $head . "use Tally\\Attribute as T;\nuse Tally\\Attribute\\Logarithmic;\nclass FooBench {\n\t#[Logarithmic('numbers')]\n\tpublic function bench() {}\n}\n", + ]; + yield 'a qualified name keeps the import it is qualified by' => [ + new Linear('ignored'), + new Logarithmic('ignored'), + $head . "use Tally\\Attribute;\nclass FooBench {\n\t#[Attribute\\Linear('numbers')]\n\tpublic function bench() {}\n}\n", + $head . "use Tally\\Attribute;\nuse Tally\\Attribute\\Logarithmic;\nclass FooBench {\n\t#[Logarithmic('numbers')]\n\tpublic function bench() {}\n}\n", + ]; + yield 'a fully qualified name keeps the import of the class' => [ + new Linear('ignored'), + new Constant('ignored'), + $head . "use Tally\\Attribute\\Linear;\nclass FooBench {\n\t#[\\Tally\\Attribute\\Linear('numbers')]\n\tpublic function bench() {}\n}\n", + $head . "use Tally\\Attribute\\Constant;\nuse Tally\\Attribute\\Linear;\nclass FooBench {\n\t#[Constant('numbers')]\n\tpublic function bench() {}\n}\n", + ]; + yield 'in the global namespace' => [ + new Linear('ignored'), + new Quadratic('ignored'), + "assertSame([], $plan->failures); + $this->assertSame([$expected], array_map(static fn (FileChange $file): string => $file->after, $plan->files)); + } + + /** + * @return iterable + */ + public static function dataReplaceComplexityFails(): iterable + { + $code = " [$code, 'Method App\\FooBench::bench() has no #[Linear] attribute.']; + yield 'in code that changed after it was loaded' => [$code . "\n", '/FooBench.php changed after Tally loaded it, run the command again.']; + } + + #[DataProvider('dataReplaceComplexityFails')] + public function testReplaceComplexityFails(string $code, string $failure): void + { + $plan = self::replaceComplexity("assertSame([], $plan->files); + $this->assertSame(['App\\FooBench::bench' => $failure], $plan->failures); + } + + /** + * Replaces the complexity class of FooBench::bench() as tally accept does. + * + * @param string $loaded the code when the benchmarks were loaded + * @param string $code the code when it is rewritten + */ + private static function replaceComplexity(string $loaded, string $code, ComplexityClass $current, ComplexityClass $replacement): ChangePlan + { + $class = str_contains($code, 'namespace App;') ? 'App\\FooBench' : 'FooBench'; + $rewriter = new ExpectedAttributeRewriter(); + $attribute = new ComplexityAttribute($rewriter, '/FooBench.php', $rewriter->index($loaded), $class, 'bench', $current); + + return (new ExpectationChanges())->plan([], [[new CaseId($class, 'bench', null), $attribute, $replacement]], ['/FooBench.php' => $code]); + } + private static function read(string $file): string { $code = file_get_contents($file); diff --git a/tests/Unit/Report/Comparisons.php b/tests/Unit/Report/Comparisons.php index 70e68a2..3f2dcbc 100644 --- a/tests/Unit/Report/Comparisons.php +++ b/tests/Unit/Report/Comparisons.php @@ -7,11 +7,11 @@ use Tally\Comparison\CaseComparison; use Tally\Comparison\Comparator; use Tally\Comparison\TolerancePolicy; -use Tally\Complexity\GrowthPoint; use Tally\Expectation\SourceLocation; use Tally\Measurement; use Tally\Metrics; use Tally\Report\Report; +use Tally\Tests\LinearGrowth; /** Comparisons of every status, for the report tests */ final class Comparisons @@ -52,7 +52,7 @@ public static function grown(string $id, int $instructions): CaseComparison return (new Comparator())->compareGrowth( $caseId, new SourceLocation(self::FILE, 1), - new GrowthPoint('O(n)', $base, 10.0), + LinearGrowth::point($base, 10.0, 10.0), new RelativeTolerance(0.1), Measurement::measured($caseId, Metrics::fromArray(['instructions' => $instructions])), Measurement::measured($base, Metrics::fromArray(['instructions' => 1000])), diff --git a/tests/Unit/Report/ReportTest.php b/tests/Unit/Report/ReportTest.php index 336fec0..d941346 100644 --- a/tests/Unit/Report/ReportTest.php +++ b/tests/Unit/Report/ReportTest.php @@ -118,8 +118,8 @@ public static function dataSlowerText(): iterable $growing = Comparisons::grown('A::a@large', 20_000); $prefix = 'These benchmarks do more work than expected. Find out why before merging. If it is the price of a fix, '; yield 'expected numbers' => [[$numbers, $numbers], $prefix . 'accept the new numbers.']; - yield 'growth' => [[$growing, $growing], $prefix . 'loosen the complexity class.']; - yield 'both' => [[$numbers, $growing], $prefix . 'accept the new numbers or loosen the complexity class.']; + yield 'growth' => [[$growing, $growing], $prefix . 'accept the new complexity class.']; + yield 'both' => [[$numbers, $growing], $prefix . 'accept the new numbers and complexity classes.']; } /** @@ -134,7 +134,7 @@ public function testSlowerText(array $cases, string $text): void /** * @return iterable, bool}> */ - public static function dataHasNumbersToAccept(): iterable + public static function dataHasChangesToAccept(): iterable { yield 'nothing' => [[], false]; yield 'unchanged' => [[Comparisons::compare('A::b', 1, ['instructions' => 1], ['instructions' => 1], null)], false]; @@ -142,17 +142,17 @@ public static function dataHasNumbersToAccept(): iterable yield 'slower' => [[Comparisons::compare('A::b', 1, ['instructions' => 1], ['instructions' => 2], null)], true]; yield 'faster' => [[Comparisons::compare('A::b', 1, ['instructions' => 10], ['instructions' => 1], null)], true]; yield 'new' => [[Comparisons::compare('A::b', 1, null, ['instructions' => 1], null)], true]; - yield 'a growth that is slower' => [[Comparisons::grown('A::a@large', 20_000)], false]; - yield 'a growth that is slower and a number that is new' => [[Comparisons::grown('A::a@large', 20_000), Comparisons::compare('A::b', 1, null, ['instructions' => 1], null)], true]; + yield 'a growth that is slower' => [[Comparisons::grown('A::a@large', 20_000)], true]; + yield 'a growth within its class' => [[Comparisons::grown('A::a@large', 5_000)], false]; } /** * @param list $cases */ - #[DataProvider('dataHasNumbersToAccept')] - public function testHasNumbersToAccept(array $cases, bool $has): void + #[DataProvider('dataHasChangesToAccept')] + public function testHasChangesToAccept(array $cases, bool $has): void { - $this->assertSame($has, (new Report($cases))->hasNumbersToAccept()); + $this->assertSame($has, (new Report($cases))->hasChangesToAccept()); } } diff --git a/tests/Unit/Selection/Cases.php b/tests/Unit/Selection/Cases.php index 95e4083..88d7f96 100644 --- a/tests/Unit/Selection/Cases.php +++ b/tests/Unit/Selection/Cases.php @@ -5,9 +5,9 @@ use Tally\Attribute\RelativeTolerance; use Tally\BenchmarkCase; use Tally\CaseId; -use Tally\Complexity\GrowthPoint; use Tally\Metrics; use Tally\Tests\InMemoryExpectations; +use Tally\Tests\LinearGrowth; use function array_map; final class Cases @@ -41,7 +41,7 @@ public static function withBases(array $costs, array $bases): array [], new InMemoryExpectations($instructions === null ? null : Metrics::fromArray(['instructions' => $instructions]), '/benchmarks/InMemoryBench.php'), new RelativeTolerance(0.1), - $base === null ? null : new GrowthPoint('O(n)', CaseId::fromString($base), 1.0), + $base === null ? null : LinearGrowth::point(CaseId::fromString($base), 1.0, 1.0), ); } From 67c2d80b3fa8c1c7c82b09a89aa4d1c828e0d1a7 Mon Sep 17 00:00:00 2001 From: "ondrej@mirtes.cz" Date: Sat, 14 Nov 2026 22:01:00 +0000 Subject: [PATCH 2/2] Keep indexing benchmark files as fast as before the complexity class replacement The Benchmarks job measured index() 44 % slower and planning 2.5 % slower with 68 % more memory: indexing counted every name of the file and read the size argument of every attribute, and plan() built a closure per case. Now the index only records where a method's other attributes are, accept tokenizes the one complexity attribute it replaces and looks for other uses of its import then, and complexity changes get a loop of their own. Locally under CodSpeed the benchmarks are within 0.6 % of instructions and 1.1 % of memory of 1.x. Amp-Thread-ID: https://ampcode.com/threads/T-01a1d9f0-e845-746f-86dc-7977af8ab66d --- src/Expectation/AttributeIndex.php | 15 +- src/Expectation/ExpectationChanges.php | 30 ++- src/Expectation/ExpectedAttributeRewriter.php | 220 +++++++++--------- .../Expectation/ExpectationChangesTest.php | 30 +++ 4 files changed, 170 insertions(+), 125 deletions(-) diff --git a/src/Expectation/AttributeIndex.php b/src/Expectation/AttributeIndex.php index 7d97f56..f641fa3 100644 --- a/src/Expectation/AttributeIndex.php +++ b/src/Expectation/AttributeIndex.php @@ -3,12 +3,14 @@ namespace Tally\Expectation; /** - * Where the attributes of every method of a file are, such as #[Expected] and #[Linear], and where + * Where the #[Expected] and #[ExpectedFor] attributes of every method of a file are, and where * missing ones and their imports go, found by one pass of ExpectedAttributeRewriter over the - * code. Offsets are in bytes of the code. + * code, with the method's other attributes, such as its complexity class. Offsets are in bytes of + * the code. * - * @phpstan-type Attribute array{start: int, end: int, name: string, class: string, dataSet: string|null, size: string|null, line: int, literal: bool} - * @phpstan-type Member array{attributes: list, insertAt: int, line: int, indent: string} + * @phpstan-type Attribute array{start: int, end: int, name: string, dataSet: string|null, line: int, literal: bool} + * @phpstan-type Other array{start: int, end: int, name: string, class: string} + * @phpstan-type Member array{attributes: list, others: list, insertAt: int, line: int, indent: string} * @phpstan-type Import array{name: string, alias: string, start: int, end: int} */ final class AttributeIndex @@ -21,8 +23,6 @@ final class AttributeIndex * @param int|null $otherUse the first import of a function or a constant * @param array $members the methods by lowercase Class::method, the attributes * before each, and where an attribute goes after them, with its line and indentation - * @param array $names how often each lowercase name is used outside the imports, - * so that an import nothing uses any more can go */ public function __construct( public readonly string $code, @@ -30,8 +30,7 @@ public function __construct( public readonly ?int $namespaceEnd, public readonly array $imports, public readonly ?int $otherUse, - public readonly array $members, - public readonly array $names + public readonly array $members ) { } diff --git a/src/Expectation/ExpectationChanges.php b/src/Expectation/ExpectationChanges.php index c9c657d..afc3701 100644 --- a/src/Expectation/ExpectationChanges.php +++ b/src/Expectation/ExpectationChanges.php @@ -30,17 +30,29 @@ public function plan(array $accepted, array $complexities, array $code): ChangeP { /** @var array>}> $editsByFile the code and the edits by case */ $editsByFile = []; - $changes = []; + $failures = []; foreach ($accepted as [$case, $measured]) { - $changes[] = [$case->id, $case->expectations->file(), static fn (string $code): array => $case->expectations->edits($code, $measured)]; - } - foreach ($complexities as [$id, $attribute, $replacement]) { - $changes[] = [$id, $attribute->file(), static fn (string $code): array => $attribute->edits($code, $replacement)]; - } + $id = $case->id->toString(); + $file = $case->expectations->file(); + $before = $code[$file] ?? false; + if ($before === false) { + $failures[$id] = sprintf('Cannot read %s.', $file); + continue; + } - $failures = []; - foreach ($changes as [$caseId, $file, $edits]) { + try { + $edits = $case->expectations->edits($before, $measured); + } catch (ExpectationException $e) { + $failures[$id] = $e->getMessage(); + continue; + } + $editsByFile[$file] ??= [$before, []]; + $editsByFile[$file][1][$id] = $edits; + } + // a loop of its own, not closures shared with the cases, which would cost each case + foreach ($complexities as [$caseId, $attribute, $replacement]) { $id = $caseId->toString(); + $file = $attribute->file(); $before = $code[$file] ?? false; if ($before === false) { $failures[$id] = sprintf('Cannot read %s.', $file); @@ -48,7 +60,7 @@ public function plan(array $accepted, array $complexities, array $code): ChangeP } try { - $edits = $edits($before); + $edits = $attribute->edits($before, $replacement); } catch (ExpectationException $e) { $failures[$id] = $e->getMessage(); continue; diff --git a/src/Expectation/ExpectedAttributeRewriter.php b/src/Expectation/ExpectedAttributeRewriter.php index 0b2386f..990ff0b 100644 --- a/src/Expectation/ExpectedAttributeRewriter.php +++ b/src/Expectation/ExpectedAttributeRewriter.php @@ -15,6 +15,7 @@ use function implode; use function in_array; use function ltrim; +use function preg_match_all; use function sprintf; use function str_starts_with; use function strcasecmp; @@ -47,6 +48,7 @@ * is added after the method's other attributes, and imported when the file does not import it yet. * * @phpstan-import-type Attribute from AttributeIndex + * @phpstan-import-type Other from AttributeIndex * @phpstan-import-type Member from AttributeIndex * @phpstan-import-type Import from AttributeIndex */ @@ -64,7 +66,7 @@ final class ExpectedAttributeRewriter public function locate(AttributeIndex $index, string $class, string $method, ?string $dataSet): int { $member = $this->member($index, $class, $method); - $attribute = $this->findAttribute($member, $dataSet === null ? Expected::class : ExpectedFor::class, $dataSet); + $attribute = $this->findAttribute($member, $dataSet); return $attribute !== null ? $attribute['line'] : $member['line']; } @@ -84,12 +86,12 @@ public function edits(AttributeIndex $index, string $class, string $method, ?str { $arguments = $this->arguments($dataSet, $metrics, $namedArguments); $member = $this->member($index, $class, $method); - $attributeClass = $dataSet === null ? Expected::class : ExpectedFor::class; - $attribute = $this->findAttribute($member, $attributeClass, $dataSet); + $attribute = $this->findAttribute($member, $dataSet); if ($attribute !== null) { return [new CodeEdit($attribute['start'], $attribute['end'] - $attribute['start'], $attribute['name'] . '(' . $arguments . ')')]; } + $attributeClass = $dataSet === null ? Expected::class : ExpectedFor::class; $name = $this->localName($index, $attributeClass); $imports = []; if ($name === null) { @@ -112,9 +114,11 @@ public function edits(AttributeIndex $index, string $class, string $method, ?str */ public function complexityEdits(AttributeIndex $index, string $class, string $method, string $current, ComplexityClass $replacement): array { - $attribute = $this->findAttribute($this->member($index, $class, $method), $current, null) + $attribute = $this->findOther($this->member($index, $class, $method), $current) ?? throw new ExpectationException(sprintf('Method %s::%s() has no #[%s] attribute.', $class, $method, $this->shortName($current))); - $arguments = $attribute['size'] ?? ''; + // only the attribute, so that accept tokenizes its arguments and index() does not + $tokens = new Tokens('code, $attribute['start'], $attribute['end'] - $attribute['start'])); + $arguments = $this->sizeArgument($tokens, $tokens->next(1), $tokens->count() - 1); if ($replacement instanceof Polynomial) { $arguments .= ', degree: ' . $replacement->degree(); } @@ -144,11 +148,69 @@ public function complexityEdits(AttributeIndex $index, string $class, string $me return [...$edits, new CodeEdit($attribute['start'], $attribute['end'] - $attribute['start'], $name . '(' . $arguments . ')')]; } + /** + * @param Member $member + * @param class-string $class + * @return Other|null + */ + private function findOther(array $member, string $class): ?array + { + foreach ($member['others'] as $attribute) { + if (strcasecmp($attribute['class'], $class) === 0) { + return $attribute; + } + } + + return null; + } + + /** + * The size argument of a complexity class as written: the first argument, unless it is named, + * else the one named size. Discovery checks that the attribute has one. + * + * @param int $open the opening parenthesis + * @param int $close the closing parenthesis + */ + private function sizeArgument(Tokens $tokens, int $open, int $close): string + { + $nesting = 0; + $argumentStart = $open + 1; + for ($i = $argumentStart;; $i++) { + $text = $tokens->at($i)->text; + if (in_array($text, ['(', '['], true)) { + $nesting++; + continue; + } + if ($nesting > 0) { + if (in_array($text, [')', ']'], true)) { + $nesting--; + } + continue; + } + if ($text !== ',' && $i !== $close) { + continue; + } + + $first = $tokens->next($argumentStart - 1); + $named = $tokens->at($first)->is(T_STRING) && $tokens->at($tokens->next($first))->text === ':'; + if (!$named || $tokens->at($first)->text === 'size') { + $argument = ''; + for ($j = $first; $j <= $tokens->previous($i); $j++) { + $argument .= $tokens->at($j)->text; + } + + return $argument; + } + $argumentStart = $i + 1; + } + } + /** * The import of the attribute's class, when the attribute uses its alias and nothing else in - * the code does. An import among others in one use statement stays. + * the code does. An import among others in one use statement stays, and so does one whose name + * appears elsewhere, even in a comment or a string. * - * @param Attribute $attribute + * @param Other $attribute * @return array{int, Import}|null its key in the imports of the index, and the import */ private function unusedImport(AttributeIndex $index, array $attribute): ?array @@ -162,15 +224,15 @@ private function unusedImport(AttributeIndex $index, array $attribute): ?array return null; } } - if (strcasecmp($attribute['name'], $import['alias']) !== 0) { return null; } - // the attribute's own name is one of the uses - $alias = strtolower($import['alias']); + // uses come after the imports, the attribute is one of them; an alias needs no quoting, and a + // name after a backslash is part of another name + $uses = preg_match_all('~(?code, $import['end'])); - return isset($index->names[$alias]) && $index->names[$alias] > 1 ? null : [$key, $import]; + return $uses > 1 ? null : [$key, $import]; } return null; @@ -232,13 +294,12 @@ private function arguments(?string $dataSet, Metrics $metrics, bool $namedArgume /** * @param Member $member - * @param class-string $class * @return Attribute|null */ - private function findAttribute(array $member, string $class, ?string $dataSet): ?array + private function findAttribute(array $member, ?string $dataSet): ?array { foreach ($member['attributes'] as $attribute) { - if (strcasecmp($attribute['class'], $class) === 0 && $attribute['dataSet'] === $dataSet) { + if ($attribute['dataSet'] === $dataSet) { return $attribute; } } @@ -319,9 +380,9 @@ private function scan(Tokens $tokens, string $code): AttributeIndex // the depth of the body of $currentClass $classDepth = null; $attributes = []; + $others = []; $insertAt = null; $members = []; - $names = []; $count = $tokens->count(); for ($i = 0; $i < $count; $i++) { @@ -329,9 +390,6 @@ private function scan(Tokens $tokens, string $code): AttributeIndex if ($token->isIgnorable()) { continue; } - if ($token->is(T_STRING)) { - $names[strtolower($token->text)] = ($names[strtolower($token->text)] ?? 0) + 1; - } if ($token->text === '{' || $token->is(T_DOLLAR_OPEN_CURLY_BRACES)) { $depth++; @@ -340,6 +398,7 @@ private function scan(Tokens $tokens, string $code): AttributeIndex $classDepth = $depth; $pendingClass = null; $attributes = []; + $others = []; $insertAt = null; } continue; @@ -352,6 +411,7 @@ private function scan(Tokens $tokens, string $code): AttributeIndex } $depth--; $attributes = []; + $others = []; $insertAt = null; continue; } @@ -394,20 +454,15 @@ private function scan(Tokens $tokens, string $code): AttributeIndex if ($token->text === ';') { $attributes = []; + $others = []; $insertAt = null; continue; } if ($token->is(T_ATTRIBUTE)) { - [$groupAttributes, $groupEnd] = $this->parseAttributeGroup($tokens, $i, $namespace, $imports); + [$groupAttributes, $groupOthers, $groupEnd] = $this->parseAttributeGroup($tokens, $i, $namespace, $imports); $attributes = [...$attributes, ...$groupAttributes]; - for ($j = $i; $j < $groupEnd; $j++) { - if (!$tokens->at($j)->is(T_STRING)) { - continue; - } - - $names[strtolower($tokens->at($j)->text)] = ($names[strtolower($tokens->at($j)->text)] ?? 0) + 1; - } + $others = [...$others, ...$groupOthers]; $insertAt = null; $i = $groupEnd; continue; @@ -428,20 +483,22 @@ private function scan(Tokens $tokens, string $code): AttributeIndex // the first declaration of the method counts, like PHP's $members[strtolower($currentClass . '::' . $tokens->at($tokens->next($i))->text)] ??= [ 'attributes' => $attributes, + 'others' => $others, 'insertAt' => $tokens->at($insertAt)->pos, 'line' => $tokens->at($insertAt)->line, 'indent' => $indent, ]; $attributes = []; + $others = []; $insertAt = null; } - return new AttributeIndex($code, $namespace, $namespaceEnd, $imports, $otherUse, $members, $names); + return new AttributeIndex($code, $namespace, $namespaceEnd, $imports, $otherUse, $members); } /** * @param list $imports - * @return array{list, int} the attributes of the group, and the index of its closing bracket + * @return array{list, list, int} the #[Expected] and #[ExpectedFor] attributes of the group, its other attributes, and the index of its closing bracket */ private function parseAttributeGroup(Tokens $tokens, int $start, ?string $namespace, array $imports): array { @@ -451,6 +508,7 @@ private function parseAttributeGroup(Tokens $tokens, int $start, ?string $namesp } $attributes = []; + $others = []; $nesting = 0; $segmentStart = $start + 1; // $start is #[ @@ -470,91 +528,37 @@ private function parseAttributeGroup(Tokens $tokens, int $start, ?string $namesp continue; } - // nothing after a trailing comma - if ($tokens->next($segmentStart - 1) !== $i) { - $attributes[] = $this->parseAttribute($tokens, $segmentStart, $i, $namespace, $aliases); - } + $name = $tokens->next($segmentStart - 1); $segmentStart = $i + 1; - if ($text === ']') { - return [$attributes, $i]; - } - } - } - - /** - * @param array $aliases - * @return Attribute - */ - private function parseAttribute(Tokens $tokens, int $start, int $end, ?string $namespace, array $aliases): array - { - $name = $tokens->next($start - 1); - $resolved = $this->resolve($tokens->at($name)->text, $namespace, $aliases); - $isExpectedFor = strcasecmp($resolved, ExpectedFor::class) === 0; - - $open = $tokens->next($name); - $hasArguments = $tokens->at($open)->text === '('; - $attributeEnd = $hasArguments ? $tokens->previous($end) : $name; - $dataSet = null; - if ($isExpectedFor) { - $value = $hasArguments ? $this->dataSetArgument($tokens, $open, $attributeEnd) : null; - $dataSet = $value === null ? null : $this->stringValue($tokens->at($value)->text); - } - - return [ - 'start' => $tokens->at($name)->pos, - 'end' => $tokens->at($attributeEnd)->pos + strlen($tokens->at($attributeEnd)->text), - 'name' => $tokens->at($name)->text, - 'class' => $resolved, - 'dataSet' => $dataSet, - 'size' => $hasArguments ? $this->sizeArgument($tokens, $open, $attributeEnd) : null, - 'line' => $tokens->at($name)->line, - // member() rejects it only on the method it looks for - 'literal' => !$isExpectedFor || $dataSet !== null, - ]; - } - - /** - * The size argument of a complexity class as written: the first argument, unless it is named, - * else the one named size. - * - * @param int $open the opening parenthesis - * @param int $close the closing parenthesis - * @return string|null null when the attribute has no such argument - */ - private function sizeArgument(Tokens $tokens, int $open, int $close): ?string - { - $nesting = 0; - $argumentStart = $open + 1; - for ($i = $argumentStart; $i <= $close; $i++) { - $text = $tokens->at($i)->text; - if (in_array($text, ['(', '['], true)) { - $nesting++; - continue; - } - if ($nesting > 0) { - if (in_array($text, [')', ']'], true)) { - $nesting--; + // nothing after a trailing comma + if ($name !== $i) { + $resolved = $this->resolve($tokens->at($name)->text, $namespace, $aliases); + $open = $tokens->next($name); + $hasArguments = $tokens->at($open)->text === '('; + $attributeEnd = $hasArguments ? $tokens->previous($i) : $name; + $location = [ + 'start' => $tokens->at($name)->pos, + 'end' => $tokens->at($attributeEnd)->pos + strlen($tokens->at($attributeEnd)->text), + 'name' => $tokens->at($name)->text, + ]; + $isExpected = strcasecmp($resolved, Expected::class) === 0; + if ($isExpected || strcasecmp($resolved, ExpectedFor::class) === 0) { + $value = !$isExpected && $hasArguments ? $this->dataSetArgument($tokens, $open, $attributeEnd) : null; + $dataSet = $value === null ? null : $this->stringValue($tokens->at($value)->text); + $attributes[] = $location + [ + 'dataSet' => $dataSet, + 'line' => $tokens->at($name)->line, + // member() rejects it only on the method it looks for + 'literal' => $isExpected || $dataSet !== null, + ]; + } else { + $others[] = $location + ['class' => $resolved]; } - continue; - } - if ($text !== ',' && $i !== $close) { - continue; } - - $first = $tokens->next($argumentStart - 1); - $named = $tokens->at($first)->is(T_STRING) && $tokens->at($tokens->next($first))->text === ':'; - if ($first !== $i && (!$named || $tokens->at($first)->text === 'size')) { - $argument = ''; - for ($j = $first; $j <= $tokens->previous($i); $j++) { - $argument .= $tokens->at($j)->text; - } - - return $argument; + if ($text === ']') { + return [$attributes, $others, $i]; } - $argumentStart = $i + 1; } - - return null; } /** diff --git a/tests/Unit/Expectation/ExpectationChangesTest.php b/tests/Unit/Expectation/ExpectationChangesTest.php index 0e2aea5..100ba70 100644 --- a/tests/Unit/Expectation/ExpectationChangesTest.php +++ b/tests/Unit/Expectation/ExpectationChangesTest.php @@ -137,6 +137,36 @@ public function testAComplexityClassAndExpectationsInOneFileAreOneChange(): void $this->assertSame([], $plan->failures); } + public function testAComplexityClassThatCannotBeReplacedDoesNotStopTheOthers(): void + { + $code = "index($code); + $attribute = static fn (string $file, string $method): ComplexityAttribute => new ComplexityAttribute($rewriter, $file, $index, 'App\SortBench', $method, new Linear('numbers')); + + $plan = (new ExpectationChanges())->plan( + [], + [ + [CaseId::fromString('App\GoneBench::benchGone'), $attribute('/GoneBench.php', 'benchA'), new Quadratic('numbers')], + [CaseId::fromString('App\SortBench::benchC'), $attribute('/SortBench.php', 'benchC'), new Quadratic('numbers')], + [CaseId::fromString('App\SortBench::benchB'), $attribute('/SortBench.php', 'benchB'), new Quadratic('numbers')], + ], + ['/SortBench.php' => $code, '/GoneBench.php' => false], + ); + + $this->assertSame( + [str_replace("#[Linear('numbers')]\n\tpublic function benchB", "#[Quadratic('numbers')]\n\tpublic function benchB", str_replace('use Tally\\Attribute\\Linear;', "use Tally\\Attribute\\Linear;\nuse Tally\\Attribute\\Quadratic;", $code))], + array_map(static fn (FileChange $file): string => $file->after, $plan->files), + ); + $this->assertSame( + [ + 'App\GoneBench::benchGone' => 'Cannot read /GoneBench.php.', + 'App\SortBench::benchC' => 'Method App\SortBench::benchC() has no #[Linear] attribute.', + ], + $plan->failures, + ); + } + public function testEditsAtOneOffsetGoInTheNaturalOrderOfTheirTextAndInsertionsBeforeReplacements(): void { $plan = (new ExpectationChanges())->plan(