Prevent unnecessary IO in ComposerJsonAndInstalledJsonSourceLocatorMaker

[staabm]

similar to #2594

[staabm]

we don't need this check, because of the try/catch below

[staabm]

in non-composer projects we will already early exit in line 46

[staabm]

we don't need this call (and also not the one below for is_dir), because of

$classMapFiles = array_filter($classMapPaths, 'is_file');
$classMapDirectories = array_filter($classMapPaths, 'is_dir');

above

[phpstan-bot]

This pull request has been marked as ready for review.

[ondrejmirtes]

Hi, I'm pretty skeptical about this. It's not worth it much to optimize things that happen literally a couple of times in a typical project. Additionally, you save is_file call, but because of that, additional call to stream_resolve_include_path happens instead.

[staabm]

Additionally, you save is_file call, but because of that, additional call to stream_resolve_include_path happens instead.

I don't think so. we will very likely hit the file_get_contents which will return successfully.

and if I read it correctly, we are triggering this path in every phpstan worker.

[staabm]

I did some measures running PHPStan on phpstan-src while adding some debugging logs:

diff --git a/src/File/FileReader.php b/src/File/FileReader.php
index 46f893391..12b4e3008 100644
--- a/src/File/FileReader.php
+++ b/src/File/FileReader.php
@@ -15,14 +15,17 @@ class FileReader
 	{
 		$path = $fileName;
 
+		echo "file_get_contents\n";
 		$contents = @file_get_contents($path);
 		if ($contents === false) {
+			echo "stream_resolve_include_path\n";
 			$path = stream_resolve_include_path($fileName);
 
 			if ($path === false) {
 				throw new CouldNotReadFileException($fileName);
 			}
 
+			echo "file_get_contents\n";
 			$contents = @file_get_contents($path);
 		}
 
diff --git a/src/Reflection/BetterReflection/SourceLocator/ComposerJsonAndInstalledJsonSourceLocatorMaker.php b/src/Reflection/BetterReflection/SourceLocator/ComposerJsonAndInstalledJsonSourceLocatorMaker.php
index 34c6105a5..b1e45fe0c 100644
--- a/src/Reflection/BetterReflection/SourceLocator/ComposerJsonAndInstalledJsonSourceLocatorMaker.php
+++ b/src/Reflection/BetterReflection/SourceLocator/ComposerJsonAndInstalledJsonSourceLocatorMaker.php
@@ -49,6 +49,7 @@ class ComposerJsonAndInstalledJsonSourceLocatorMaker
 		$vendorDirectory = ComposerHelper::getVendorDirFromComposerConfig($projectInstallationPath, $composer);
 
 		$installedJsonPath = $vendorDirectory . '/composer/installed.json';
+		echo "is_file\n";
 		if (!is_file($installedJsonPath)) {
 			return null;
 		}
@@ -112,6 +113,7 @@ class ComposerJsonAndInstalledJsonSourceLocatorMaker
 		);
 
 		foreach ($classMapDirectories as $classMapDirectory) {
+			echo "is_dir\n";
 			if (!is_dir($classMapDirectory)) {
 				continue;
 			}
@@ -121,6 +123,7 @@ class ComposerJsonAndInstalledJsonSourceLocatorMaker
 		$files = [];
 
 		foreach (array_merge($classMapFiles, $filePaths) as $file) {
+			echo "is_file\n";
 			if (!is_file($file)) {
 				continue;
 			}

➜  phpstan-src git:(1.10.x) ✗ php bin/phpstan clear-result-cache -q && php bin/phpstan analyze -q > 1.10.x_9f93828ba.log
...

➜  phpstan-src git:(1.10.x) ✗ sort patched.log | uniq -c # with the PR applied
8525 file_get_contents
➜  phpstan-src git:(1.10.x) ✗ sort 1.10.x_9f93828ba.log | uniq -c # before the PR
8525 file_get_contents
  80 is_dir
  50 is_file

-> we save roughly 130 IO operations which is ~1,5% of operations (across all processes involved)

-> in a project with a composer.json in the project root it seems stream_resolve_include_path is not involved a single time