search

squizlabs / PHP_CodeSniffer

PHP_CodeSniffer tokenizes PHP files and detects violations of a defined set of coding standards.
BSD 3-Clause "New" or "Revised" License
10.66k stars 1.48k forks source link

New `Performance` report to measure sniff run time performance #3810

Closed jrfnl closed 9 months ago

jrfnl commented 1 year ago

This PR depends on PR #3804 to allow for automated testing of the new report. Either PR #3804 should be merged first and this PR rebased to remove that commit after the merge or the EndToEnd test included in this PR should be removed from the last commit and (after this PR has been merged), the test can be added to PR #3804.

☝🏻 The above is the only reason to open this PR in draft. The actual change in the PR is ready for review.

Util\Timing: split printRunTime() method

... into logical parts to allow the Performance report access to the total run time information (at the point of report creation) as well.

File: add getListenerTimes() method

... to allow for access to the recorded listener times from within a report class.

Config: add trackTime toggle for whether or not to track listener times

As recording the time taken by each sniff has a performance impact in and of itself on a CS run, only record the time taken by each sniff when needed. [*]

Originally, this was already done conditionally based on the PHP_CODESNIFFER_VERBOSITY > 2 condition. However, adding the Performance report would add a second criteria.

This commit adds a new (internal) Config setting trackTime, which will be set to true when PHP_CODESNIFFER_VERBOSITY > 2.

This commit paves the way for adding the second criteria in the next commit.

[*] I've done some unscientific benchmarks for this by running PHPCS multiple times, with and without tracking the listener times, over a 300+ file codebase. Without tracking listener times, the run time was always around 39 seconds with 56Mb memory use. With tracking listener times, the run time was always around 54 seconds with 64Mb memory use.

This, to me, shows a significant enough difference and sufficient reason to put this toggle in place to only track time when needed.

New Performance report to measure sniff run time performance

The report will print the sniff name, cumulative listener run time and % of the total sniff run time for each sniff triggered during a run.

The report is ordered by cumulative listener run time in descending order.

Additionally, it will highlight sniffs which have a cumulative listener run time more than twice the average run time per sniff in orange and sniffs with a cumulative listener run time of more than three times the average run time per sniff in red.

At the bottom of the report it will also compare the total sniff relative run time with the total run time.

Fixes #3784

Includes end-to-end test (depends on PR #3804). Includes mention of the report in the CLI help text.

The report output is as shown in the screenshots in https://github.com/squizlabs/PHP_CodeSniffer/issues/3784#issuecomment-1520203761

jrfnl commented 1 year ago

Rebased to get past merge conflicts.

jrfnl commented 1 year ago

As per my comment in PR #3804, I've removed the End2End test for the report from this PR to unblock it and am marking the PR as ready for review

jrfnl commented 9 months ago

Closing as replaced by https://github.com/PHPCSStandards/PHP_CodeSniffer/pull/60