Repository navigation
Update Tally and accept its benchmark numbers - #7068
Merged
Merged
Conversation
Tally from release/1.x counts with its own build of CodSpeed's Valgrind and the tally_hooks extension instead of the codspeed CLI and instrument-hooks over FFI. Instructions stay the same, but the cycles of the smallest benchmarks shift by a few compulsory cache misses: Callgrind flushes the simulated cache when each span starts, and the code the hooks run before a span now shares fewer cache lines with the benchmark. LoaderFactoryBench reads its config with fewer instructions and less memory. The memory of the two optional-keys blowup cases had grown already on 2.3.x before the switch. Amp-Thread-ID: https://ampcode.com/threads/T-01a1d73a-bab0-7223-9245-50c2c00f22c7
The cycles of the smallest benchmarks no longer depend on the cache lines the code just before them touches, and the default tolerance is now ±2 % instructions, ±5 % memory, and ±5 % cycles but at least 2,000 cycles. Amp-Thread-ID: https://ampcode.com/threads/T-01a1d73a-bab0-7223-9245-50c2c00f22c7
The benchmarks name it dataSet:, and tally accept wrote it as the first argument in the cases it rewrote. Amp-Thread-ID: https://ampcode.com/threads/T-01a1d73a-bab0-7223-9245-50c2c00f22c7
The cycles of most benchmarks dropped now that the warm-up calls fill the simulated caches. The memory of bug-4300.php and bug-7214.php moved by about 64 KiB with the new Tally at the same code. bug-15061.php and bug-4308.php stay as they were: they are slower on 2.3.x with the old Tally too, since commits after 20c433f. Amp-Thread-ID: https://ampcode.com/threads/T-01a1d73a-bab0-7223-9245-50c2c00f22c7
ondrejmirtes
changed the title
The after methods and the end of the process, such as freeing the container, no longer run under Callgrind's cache simulation. The counts stay the same. Amp-Thread-ID: https://ampcode.com/threads/T-01a1d73a-bab0-7223-9245-50c2c00f22c7
Its instructions and cycles depend on the memory layout of the process: at the same code and Tally, a few more kilobytes of environment move them by up to 2 % and 10 %, likely through whether serialize() can grow its string in place. The update of Tally moved the layout. Amp-Thread-ID: https://ampcode.com/threads/T-01a1d73a-bab0-7223-9245-50c2c00f22c7
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Since #7067, Tally counts with its own build of CodSpeed's Valgrind and its
tally_hooksextension, not with the codspeed CLI and instrument-hooks over FFI. The first commit accepted the numbers of that switch. After that, phpstan/tally#4 changed two things:#[Warmup]calls, and only switches collection on and off for each span. Instruction counts are unchanged; the cycles of most benchmarks dropped.AnalysisBenchkeeps its ownDurationTolerance.This PR:
phpstan/tallytodev-release/1.xatc14039c,withNamedArguments(true)intally.php, sotally acceptwrites#[ExpectedFor(dataSet: '…')]like the existing benchmarks, and restores thedataSet:names the first accept dropped,bug-4300.phpandbug-7214.phpat the same PHPStan code. These memory peaks move in 64 KiB steps when anything earlier in the process changes;bug-4308.phpmoved the same way between older and newer2.3.xcommits.Benchmarks will still fail on two cases, on purpose:
bug-15061.php(+23 % instructions) andbug-4308.php(+70 % memory). They also fail on2.3.xat 7ef85f5 with the old Tally, so they come from PHPStan commits after 20c433f, not from Tally, and this PR doesn't accept them. Against the artifacts of the last run,tally comparereports exactly those two and 451 unchanged.