Skip to content

Conversation

staabm
Copy link
Contributor

@staabm staabm commented Sep 17, 2024

@ondrejmirtes
Copy link
Member

I'd rather not call realpath, too much logic, can fail. You can normalize the paths with DIRECTORY_SEPARATOR.

@ondrejmirtes ondrejmirtes merged commit 84a7397 into phpstan:1.12.x Sep 18, 2024
476 of 499 checks passed
@ondrejmirtes
Copy link
Member

Thank you. This should lead to performance improvements as well.

@staabm
Copy link
Contributor Author

staabm commented Sep 18, 2024

I measured the before/after stats of this patch.

I can see a ~5-7% faster end-to-end analysis time after the patch with a primed cache.
with a empty cache I can see a 3-4% improvement.

@staabm
Copy link
Contributor Author

staabm commented Sep 18, 2024

@ondrejmirtes looking at the result cache, I can see \PHPStan\Analyser\Error beeing serialized with a few duplicated properties:

    \PHPStan\Analyser\Error::__set_state(array(
       'message' => 'Call to method isValidPost() of deprecated class CsrfToken:
use DI injected CsrfTokenAbstract instead',
       'file' => 'C:\\dvl\\Workspace\\kunzmann\\app\\portal\\controllers\\ServiceController.php',
       'line' => 145,
       'canBeIgnored' => true,
       'filePath' => 'C:\\dvl\\Workspace\\kunzmann\\app\\portal\\controllers\\ServiceController.php',
       'traitFilePath' => NULL,
       'tip' => NULL,
       'nodeLine' => 145,
       'nodeType' => 'PhpParser\\Node\\Expr\\StaticCall',
       'identifier' => 'staticMethod.deprecatedClass',
       'metadata' => 
      array (
      ),

would you accept a patch, which e.g. would prevent serializing 'file' and 'filePath in case they contain 1:1 the identical string?
(to reduce the amount of data in the result cache file)

@staabm
Copy link
Contributor Author

staabm commented Sep 18, 2024

Found out that in our projects one big "offender" of result-cache bloat was rectory/type-perfect which gets fixed with rectorphp/type-perfect#44

@ondrejmirtes
Copy link
Member

ondrejmirtes commented Sep 18, 2024

would you accept a patch

No, not worth it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants