Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NameSpaceHelper: introduce namespace cache to speedup namespace lookups #939

Merged
merged 1 commit into from
Mar 25, 2020

Conversation

nightlinus
Copy link
Contributor

This pr introduce cache for namespace lookups.

This speed up for 8kloc test-case

  • ReferenceThrowableOnlySniff from 3.8s to 0.4s
  • OptimizedFunctionsWithoutUnpackingSniff from 3.5s to 0.4s
  • ParameterTypeHintSniff from 2.8s to 0.4s
  • ReturnTypeHintSniff from 1.7s to 0.4s

Can this cache cause any unwanted side effects?

return TokenHelper::findNextAll($phpcsFile, T_NAMESPACE, 0);
static $cache = [];

$cacheKey = $phpcsFile->getFilename() . $phpcsFile->getFixedCount();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I moved this logic to SniffLocalCache helper, what do you think about it?

SlevomatCodingStandard/Helpers/NamespaceHelper.php Outdated Show resolved Hide resolved
@kukulich kukulich added this to the 6.2.0 milestone Mar 25, 2020
Copy link
Contributor

@kukulich kukulich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Just some small changes please :)

SlevomatCodingStandard/Helpers/SniffLocalCache.php Outdated Show resolved Hide resolved
SlevomatCodingStandard/Helpers/SniffLocalCache.php Outdated Show resolved Hide resolved
@nightlinus nightlinus force-pushed the master branch 2 times, most recently from 2bc79a5 to 3d0441a Compare March 25, 2020 11:14
@kukulich
Copy link
Contributor

@nightlinus One last change please. Squash it to one commit or move this change 3d0441a#diff-054ee9cafd168683808f20c4b5d722f1R84 to the right commit.

@nightlinus
Copy link
Contributor Author

rebased and squashed

@kukulich kukulich merged commit 01ec1cd into slevomat:master Mar 25, 2020
@kukulich
Copy link
Contributor

Thanks 👍

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

Successfully merging this pull request may close these issues.

None yet

2 participants