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

Reorder checks in Filter::isExcluded to touch the FS only when needed #928

Merged
merged 1 commit into from
Aug 30, 2022
Merged

Reorder checks in Filter::isExcluded to touch the FS only when needed #928

merged 1 commit into from
Aug 30, 2022

Conversation

mvorisek
Copy link
Contributor

@mvorisek mvorisek commented Aug 29, 2022

This change reduces is_file calls in short coverage requests significantly, as a lot of is_file calls of (normally excluded/not-included) vendor files will not be needed.

Also reduces Filter::$isFileCache cache size.

@mvorisek mvorisek changed the base branch from main to 9.2 August 29, 2022 16:25
@codecov
Copy link

codecov bot commented Aug 29, 2022

Codecov Report

Merging #928 (43dcd10) into 9.2 (2593003) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##                9.2     #928      +/-   ##
============================================
- Coverage     83.66%   83.65%   -0.01%     
  Complexity     1161     1161              
============================================
  Files            59       59              
  Lines          3415     3413       -2     
============================================
- Hits           2857     2855       -2     
  Misses          558      558              
Impacted Files Coverage Δ
src/Filter.php 97.56% <100.00%> (-0.12%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@mvorisek mvorisek changed the title Check isFile in Filter::isExcluded if really included Reorder checks in Filter::isExcluded to touch the FS only when needed Aug 29, 2022
@sebastianbergmann
Copy link
Owner

Your pull request contains changes (!$filename -> $filename === false, for instance) that are not related to the topic of your pull request.

@mvorisek
Copy link
Contributor Author

Your pull request contains changes (!$filename -> $filename === false, for instance) that are not related to the topic of your pull request.

Yes, this PR also improves the checking of realpath failure. It that change welcomed?

@sebastianbergmann
Copy link
Owner

Yes, this PR also improves the checking of realpath failure. It that change welcomed?

Likely, but please separate the two changes.

@mvorisek mvorisek marked this pull request as draft August 30, 2022 07:51
@mvorisek mvorisek marked this pull request as ready for review August 30, 2022 07:52
@mvorisek
Copy link
Contributor Author

PR updated, now it contains only the reordered checks.

@sebastianbergmann sebastianbergmann merged commit aa3cd05 into sebastianbergmann:9.2 Aug 30, 2022
@mvorisek mvorisek deleted the faster_filter_isexcluded_check branch August 30, 2022 09:27
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