Clarify documentation of --diff and --diff-methods

[bdsl]

No description provided.

[bdsl]

I'm going to try doing this for Psalm's own CircleCI config.

[bdsl]

It would be good to add something about why these are optional and not always on. I haven't been using them up to now, but I don't actually know in what use case it's better to have them switched off.

[weirdan]

@muglug said he intends to enable those flags by default:
https://twitter.com/psalmphp/status/1220126747175325696

[bdsl]

Yes, but I don't know if there'll still be a reason for some users to turn them off.

[muglug]

We had them on at Vimeo, and ran into essentially impossible-to-reproduce bugs in CI when running with --find-unused-code. But our CI is different now and it should be much easier to reproduce issues, so I'll soon be turning it back on and monitoring for a while.

[bdsl]

Running Psalm on my local on it's own codebase I don't actually see much difference in speed between with and without --diff. Time seems to vary between about 70 and 30 seconds irrespective. But then I'm also getting 15 errors, while the self analysis has passed in CircleCI.

[weirdan]

Running Psalm on my local on it's own codebase I don't actually see much difference in speed between with and without --diff. Time seems to vary between about 70 and 30 seconds irrespective. But then I'm also getting 15 errors, while the self analysis has passed in CircleCI.

Doing the same here I get:

$ ./psalm --clear-cache
Cache directory deleted

$ ./psalm --threads=8 --show-info=false
Scanning files...
Analyzing files...

░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░  60 / 762 (7%)
░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░ 120 / 762 (15%)
░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░ 180 / 762 (23%)
░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░ 240 / 762 (31%)
░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░ 300 / 762 (39%)
░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░ 360 / 762 (47%)
░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░ 420 / 762 (55%)
░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░ 480 / 762 (62%)
░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░ 540 / 762 (70%)
░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░ 600 / 762 (78%)
░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░ 660 / 762 (86%)
░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░ 720 / 762 (94%)
░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░

------------------------------
No errors found!
------------------------------
15 other issues found.
------------------------------

Checks took 23.81 seconds and used 515.054MB of memory
Psalm was able to infer types for 99.8332% of the codebase

$ ./psalm --threads=8 --show-info=false --diff --diff-methods
Scanning files...
Analyzing files...



------------------------------
No errors found!
------------------------------
15 other issues found.
------------------------------

Checks took 2.62 seconds and used 495.469MB of memory
No files analyzed

[bdsl]

@weirdan ok so a composer update fixed my failures locally, and using both --diff and --diff methods I can get my time down to about 10 seconds, vs 35 with neither.

[bdsl]

Master is currently failing, so I've rebased this PR onto 947765c - the last passing commit from master.

[bdsl]

Caching in CircleCI only seems to save about two seconds, so maybe isn't worth it: compare https://app.circleci.com/jobs/github/bdsl/psalm/886 in which the cache cannot be loaded because it hasn't been saved yet, and https://app.circleci.com/jobs/github/bdsl/psalm/889 which loaded a cache. The Psalm run was 13.96s in the former and 11.91s in the latter. Maybe would need a bigger codebase to show the benefit.

Feel free to take just the first commit from this PR.

[weirdan]

Caching in CircleCI only seems to save about two seconds, so maybe isn't worth it

Looking at the code I suspect caching just doesn't really work for Psalm CI environment. E.g. ClassLikeStorageCacheProvider includes $this->timestamps into the cache validity token:

psalm/src/Psalm/Internal/Provider/ClassLikeStorageCacheProvider.php

Lines 124 to 127 in 0ffb833

	private function getCacheHash($file_path, $file_contents)
	{
	return sha1(($file_path ? $file_contents : '') . $this->modified_timestamps);
	}

And $this->timestamps includes modification times of some Psalm's files:

psalm/src/Psalm/Internal/Provider/ClassLikeStorageCacheProvider.php

Lines 45 to 52 in 0ffb833

	$storage_dir = dirname(__DIR__, 2) . DIRECTORY_SEPARATOR . 'Storage' . DIRECTORY_SEPARATOR;
	$dependent_files = [
	$storage_dir . 'FileStorage.php',
	$storage_dir . 'FunctionLikeStorage.php',
	$storage_dir . 'ClassLikeStorage.php',
	$storage_dir . 'MethodStorage.php',
	];

as well as Psalm's version (with the commit hash):

psalm/src/Psalm/Internal/Provider/ClassLikeStorageCacheProvider.php

Line 66 in 0ffb833

$this->modified_timestamps .= $this->config->hash;

For Psalm's self-check it's version as well as file mtimes will almost always differ from the previous run, because we're checking another commit.

[muglug]

Thanks!