-
Notifications
You must be signed in to change notification settings - Fork 28
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
Issue 55 curl multirunner burn cpu #56
base: 2.x
Are you sure you want to change the base?
Conversation
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.
thank you for looking into this. some comments and questions:
$info = curl_multi_info_read($this->multiHandle); | ||
if (false !== $info) { | ||
if ($info !== false) { |
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.
please don't revert yoda style comparisons.
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.
Why?
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.
because it has nothing to do with the change at hand.
i would have expected the code style checker to complain, but seems we don't have a rule for this activated.
} | ||
|
||
do { | ||
$status = curl_multi_exec($this->multiHandle, $active); |
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.
is this now again busy waiting? should we repeat the curl_multi_select in each loop?
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.
I'm not pretty sure. Most examples I've found are like this https://bugs.php.net/bug.php?id=61141#1348321490
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.
its again the same as guzzle does, which is most likely what the symfony http client is using, so i'd hope its the right thing.
Hi i'm running into this, any merge planned ? Any help needed ?
|
from elastic/elasticsearch-php#990: a solution proposed at https://bugs.php.net/bug.php?id=61141#1348321490 is
(which we should format according to our code style, but functionality seems good to me. 100 micro seconds is 1 thousandst of a second, so should be an acceptable delay while lowering the cpu) |
No description provided.