Config: only determine screen width if/when needed (performance improvement) #3831
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Config: only determine screen width if/when needed
I'm not sure about *nix, but on Windows, the call to
shell_exec('stty ...')
is slow.As things were, the call to
shell_exec('stty ...')
would be:reportWidth
is being set inrestoreDefaults()
.CodeSniffer.conf
file contained areport_width => 'auto'
entry.<config name="report-width" value="auto"/>
entry.--report-width=auto
would be passed from the command-line.This is inefficient for the following reasons:
shell_exec('stty ...')
does not change between calls (well, providing the user doesn't resize their terminal in the microseconds between calls)'auto'
translates to, until thereportWidth
will be used.Taking the above into account, making the same call up to four times is not desirable.
This commit moves the translation from
'auto'
to an actual terminal width from the__set()
method to the__get()
method and overwrites thereportWidth
value from'auto'
with the actual terminal width value, if available, and with the default value if the terminal width could not be determined.This means that subsequent calls to
__get()
for thereportWidth
will automatically use the previously determined value instead of trying to determine value again.This removes the inefficiency and should make PHPCS runs a little bit faster (at the very least on Windows).
The only time multiple calls to
shell_exec('stty...')
could now need to be made, would be if thereportWidth
would be changed (back to'auto'
) between the first retrieval and a subsequent retrieval of thereportWidth
value. As things are, this will never happen in a normal PHPCS run, though could possibly happen in a test situation.AbstractMethodUnitTest: take advantage of the change in reportWidth handling
For the tests using the
AbstractMethodUnitTest
class, thereportWidth
and most other config settings are irrelevant.This commit changes some of the set up/tear down for the test to make the use of the
Config
class more efficient.This should make the tests using the
AbstractMethodUnitTest
class as their base significantly faster (at the very least on Windows).While not benchmarked properly, I have done some comparisons with the test runs on my local machine on Windows.
phpunit --filter Core
(= the tests which use this base class + a few extra tests):Before: 2 minutes.
After: 8 seconds.
phpunit
without a--filter
:Before: 7 minutes.
After: 5 minutes.
Before: 2.5 minutes.
After: 1 second.
Suggested changelog entry
Related issues/external references
Follow up on #3761 for which tests were added in #3820.
Types of changes