Remove blocking I/O from ReactPHP implementation #8550
Conversation
LibUv is the most optimised loop implementation for ReactPHP. This alters the setup to allow us to properly compare things. Signed-off-by: Luís Cobucci <lcobucci@gmail.com>
Signed-off-by: Luís Cobucci <lcobucci@gmail.com>
This just makes development more efficient as we avoid downloading composer and all the dependencies when we only want to propagate updates to the PHP files. Signed-off-by: Luís Cobucci <lcobucci@gmail.com>
This makes us rely on the automatic execution of the Loop and removes usage of deprecated classes. Signed-off-by: Luís Cobucci <lcobucci@gmail.com>
|
@WyriHaximus please feel free to correct things even further 👍 |
Although this isn't a huge deal, it speeds up development as we don't need to wait docker to trigger SIGKILL. Signed-off-by: Luís Cobucci <lcobucci@gmail.com>
Just as AMPHP, ReactPHP requires asynchronous implementations for it to work as expected. When using PDO we will block the process when establishing the DB connection and sending queries. This replaces the implementation with a fully async MySQL client, also cleaning removing unnecessary extensions from the image. Signed-off-by: Luís Cobucci <lcobucci@gmail.com>
lcobucci
force-pushed
the
remove-blocking-io-from-reactphp
branch
from
2a52437 to
4f128f8
Compare
|
Please update to PHP 8.3 |
Sure, will do around next week |
Signed-off-by: Luís Cobucci <lcobucci@gmail.com>
|
@lcobucci |
|
@joanhey Just checked those logs but don't see any obvious failures there. I will have a look at the code and docker file locally and see if I can spot what's wrong there. |
|
I can check when started to fail. |
|
We update Mysql from 5.7 to 8.0 and now to 9.0, |
|
If it fail for the Mysql, it's OK. |
|
Like I said, problems always exist, but the more important think is that the language or framework, show you where is the problem. But now fail silently. PD: sorry, but it's a good recommendation |
|
Here started to fail:
I'll investigate what changed in the bench, but please also search what changed in ReactPHP !! |
|
Changes in the toolset : Changes in the benchmark code for ReactPHP: PD: if you check the commits, you will see than I try my best, but it is not enough sometimes |
|
The last run that work ReactPHP: The next run that failed: PD: you can check using the IDs for test it locally |
|
A good benchmark it isn't only for performance checks. Check: #9408 |
Yes, which is why I just filed: #9527 |
Yeah checked those, had to put in a crude version of #9527 to find the issue.
It's a lot, and we're thankful for that.
Nothing actually, it was the MySQL upgrades to a version with the auth method ReactPHP currently supports enabled. (We need to add support for the never ones 😅 .) |
Because the connectivity test is done against the |
|
You can test it locally And if you want to test it directly calling the urls: |
|
After add #9527
|
|
Plaintext and json work without problems. |
Lets start there, will file a PR for that 👍 |
FWIW: #8715, #9009, and #9183 all got merged with failing tests.
Yup just filed #9534 for that, will rebase the others once that is passing and merged |
|
I created #9529 yesterday, now it's duplicated. About the Databases update, normally fail, because always exist frameworks that fail. Even frameworks than don't use the database. PD: the frameworks failing in the last run |
Missed that as it wasn't mentioned in the PR title, sorry.
As a maintainer I get it, especially something this big I get it. And I get that #9528 will never be merged, that thing was a discussion starter. However, #9527 is perfectly fine to merge as it doesn't impact the benchmark negatively and will actually positively aid in finding issues with it.
Ow sweet thanks, it's kinda good to see we're not the only ones 😅 . |
|
#9257 only show the error, but don't fix the problem. The tests will fail, and no framework is merged if don't pass the tests in the PR. |
|
I don't mentioned in the title, because the important thing is the update to PHP8.4. |
Fair, I was just expecting it in it's own dedicated PR. But that's up to you as maintainer how you prefer to handle that :) |
The existing ReactPHP implementation doesn't leverage the async capabilities, creating wrong results when benchmarking things.
This optimises the ReactPHP app, allowing things to be a bit more comparable.