Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[Process] Adds usleep(1000) to ease CPU usage.#28940
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
miniyarov commentedOct 21, 2018
| Q | A |
|---|---|
| Branch? | master |
| Bug fix? | yes |
| New feature? | no |
| BC breaks? | no |
| Deprecations? | no |
| Tests pass? | yes |
| Fixed tickets | |
| License | MIT |
| Doc PR | symfony/symfony-docs |
Uh oh!
There was an error while loading.Please reload this page.
miniyarov left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Break if process is not running in waitUntil
miniyarov commentedOct 22, 2018
@nicolas-grekas added a break if process is not running |
| $this->fallbackStatus['exitcode'] = (int)$data; | ||
| } | ||
| } | ||
| if ($this->isRunning()) { |
nicolas-grekasOct 22, 2018 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
what about writting in this way:
if ($wait && !$this->isRunning()) { return false;}and make the function return a boolean (type-hint+return true at the end)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Hmm this might change function's specification but doable
nicolas-grekasOct 22, 2018 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
The method is new so we can still do it. It'd make sense to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Ok updated to return a boolean.
nicolas-grekas left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
(with one last comment, thanks!)
| * | ||
| * @param callable $callback | ||
| * | ||
| * @return bool |
nicolas-grekasOct 22, 2018 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
should be removed (same for $callback btw) as we remove docblocks that don't add anything on top of the PHP signature.
miniyarov commentedOct 22, 2018
@nicolas-grekas Thanks to you too! |
nicolas-grekas commentedOct 23, 2018
Thank you@miniyarov. |
This PR was squashed before being merged into the 4.2-dev branch (closes#28940).Discussion----------[Process] Adds usleep(1000) to ease CPU usage.| Q | A| ------------- | ---| Branch? | master| Bug fix? | yes| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets || License | MIT| Doc PR | symfony/symfony-docsCommits-------9d1416a [Process] Adds usleep(1000) to ease CPU usage.
stof commentedOct 23, 2018
Could we have tests for this boolean return value ? |
nicolas-grekas commentedOct 23, 2018
The while condition was the opposite of the previous while.This was obviously leading to the death of this loop. Loop whichwas useful according to some people (seesymfony#28940), so I just movedthe code to the right position.