php-tuf / composer-integration

PHP-TUF Composer Plugin.
7 stars 5 forks source link

To support asynchronous HTTP requests, use Guzzle instead of Composer's HttpDownloader #117

Open phenaproxima opened 3 months ago

phenaproxima commented 3 months ago

I've been trying to make PHP-TUF more asynchronous for a while now, because as it stands, this plugin slows Composer's performance way down, since every delegated target search involves many synchronous (that is, blocking) HTTP requests.

After mulling it over for a while and trying some experiments, I realized that the path of least resistance is for us to use Guzzle's HTTP client to download TUF metadata, rather than Composer's HTTP downloader. Composer's downloader is capable of operating asynchronously, but it's very opaque and uses a much less "useful" implementation of promises than PHP-TUF does. We cannot, for example, wait for a specific promise if we use Composer's downloader. Getting it work properly would have been very tricky, and the code would have been brittle and prone to weird bugs. It would also have been much more difficult to understand, and significantly harder to unit test.

It's a lot simpler for us to simply use Guzzle's download engine. It already uses the same promises API as PHP-TUF, and it fully supports asynchronous requests. This doesn't change anything user-facing at all, and it's really not a problem if the plugin is using a different underlying HTTP system than Composer does (although, frankly, they're both ultimately based on cURL, so we're really just talking about two different ways of using cURL, one of which is a very idiosyncratic and internal part of Composer, and the other a long-lived, battle-tested API with nearly 700 million downloads). From PHP-TUF's perspective, it's all the same -- the details are hidden behind the abstraction of LoaderInterface (which, indeed, was the point of introducing it).

The addition of Guzzle introduces only ONE new dependency, which is guzzlehttp/guzzle itself. All of its dependencies are either already indirectly brought in by PHP-TUF, or were already direct dependencies of the plugin. So this doesn't introduce any real risk there.

Having said that, I don't know if this actually, realistically, makes things any faster. That's up to manual testing. But it certainly makes maintenance easier and lets us take a step back from Composer's internals.

tedbow commented 3 months ago

Having said that, I don't know if this actually, realistically, makes things any faster. That's up to manual testing. But it certainly makes maintenance easier and lets us take a step back from Composer's internals.

Is it possible it actually makes things slower? Looking at main https://github.com/php-tuf/drupal-project/actions/runs/9967556087/job/27541425642 For php 8.3 this took 3m 50s and it installed more modules. While the action for the MR linked in the description https://github.com/php-tuf/drupal-project/actions/runs/9908898414/job/27375931439 actually took 4m 13s and it only installed 3 modules. So it is likely to run even slower for more modules.

If I am not linking to the correct actions can you link to correct ones?

phenaproxima commented 3 months ago

I think it is entirely possible this could be making it run slower. I don't see how, but that just means I need to dig more deeply here.