Add support for cache results #1011
Conversation
merge master
| @@ -396,7 +397,7 @@ protected function addViolation( | |||
| 'args' => $args, | |||
| ); | |||
|
|
|||
| $ruleViolation = new RuleViolation($this, $node, $message, $metric); | |||
| $ruleViolation = new RuleViolation($this, NodeInfoFactory::fromNode($node), $message, $metric); | |||
There was a problem hiding this comment.
To be able to create a RuleViolation without Node, changed signature to NodeInfo object that's serializable.
| * @param \PHPMD\RuleSetFactory $ruleSetFactory | ||
| * @param \PHPMD\Report $report | ||
| * @param \PHPMD\RuleSet[] $ruleSetList | ||
| * @param \PHPMD\Report $report | ||
| * @return void | ||
| */ | ||
| public function processFiles( |
There was a problem hiding this comment.
Changed signature to allow $ruleSetFactory->createRuleSet be called before processFiles is called.
| @@ -88,6 +89,7 @@ private function init(Engine $pdepend, PHPMD $phpmd) | |||
| $this->initInput($pdepend, $phpmd); | |||
| $this->initIgnores($pdepend, $phpmd); | |||
| $this->initExtensions($pdepend, $phpmd); | |||
| $this->initResultCache($pdepend, $phpmd); | |||
There was a problem hiding this comment.
Hook into pdepend filter system to filter files from pdepend inspection if cache is fresh.
|
No idea why |
|
No worry for the sarif test it will be handled in #1007 |
Codecov ReportPatch coverage:
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more. Additional details and impacted files@@ Coverage Diff @@
## master #1011 +/- ##
============================================
+ Coverage 88.65% 89.10% +0.44%
- Complexity 1076 1160 +84
============================================
Files 96 107 +11
Lines 2698 2956 +258
============================================
+ Hits 2392 2634 +242
- Misses 306 322 +16
☔ View full report in Codecov by Sentry. |
|
Hi @frankdekker, thanks a lot for implementing the phpmd result cache feature! Good job. Happily set the new options in our (huge) project ... but without any speed improvement. It might be a bit specific for our project but others might have the same problems? a) We run b) We do not use a baseline file. Tracked the problem down to Any chances to make both composer files and the baseline file optional? (Commented the Best, mvoelker |
Type: feature
Issue: Resolves #534
Breaking change: no.
Feature:
Added the ability to enable (default disabled, enable via
--cache) result caching. This will write on the first run a cache file (.phpmd.result-cache.php, configurable via--cache-file) and will be used in the next run. Any files that have not been changed since then based on either the sha1 of content or file modified timestamp (can be set via--cache-strategy). Any changes to the configuration will invalidate the entire cache. Cache key is based on:strict mode,baseline file,composer.json,composer.lockmodified, any modification to the rules and changes to PHP version.Command line options:
--cache: if set will enable reading and writing the result cache.--cache-file <file>: will change the result cache file location. Default./.phpmd.result-cache.php--cache-strategy [content|timestamp]: changes the strategy if a file is modified is determined. DefaultcontentPerformance:
Some basic performance tests on a large codebase result in:
Technical
As the structure of PHPMD didnt quite allow a clean integration for result caching, I looked into PDepend how to hook into the file inspection foreach. I found
PDepend\Input\Filter, which I added as last filter in PHPMD. When PDepend now iterates over each file, theResultCacheFileFilterwill check if the file is modified or not. If the file is modified, the file will not be filtered out and be inspected by PDepend. If however a file is not modified, we keep track of this file to later add the possible stored violations to the final report.The
ResultCacheEngineis the main control of the cache, and consists of 3 stages:Technical challenges
As described above, I needed to add
RuleViolationclass to the report based on data from the cache. I changed the RuleViolation constructor signature to avoid including theAbstractNodetype and replaced it withNodeInfoclass. This class contains the same information that was used from the AbstractNode, and is far easier to construct.Second problem was that
RuleViolationalso requires aRuleclass. As PHPMD::processFiles method was passed a RuleSetFactory, which I also needed beforePHPMD::processFileswas invoked, I moved the methodcalls to
RuleSetFactoryoutside thePHPMD::processFilesmethod.Changes:
/Cache/namespace with several classes to read, write, and calculate the result cache./Cache/Modelfor some data models.