php-tuf / composer-integration

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

Implement static caching to improve performance #89

Closed phenaproxima closed 12 months ago

phenaproxima commented 12 months ago

This plugin's performance sucks.

87 improved things a tad by making us transfer less network traffic if we don't have to, but we are still spending an awful lot of time talking to the server, asking for unchanged files...and those HTTP requests are blocking. In Composer repositories with a lot of files (i.e., all of them), this degrades performance to unacceptable levels.

To fix this and make performance a lot better, I propose we add static caching to Loader. Once we have downloaded TUF metadata from the server (or read unmodified data from the filesystem), we can statically cache those streams so we don't even have to talk to the server; or, for that matter, the filesystem.

tedbow commented 12 months ago

Would this change the functionality that would could test in \Tuf\ComposerIntegration\Tests\ComposerCommandsTest::testRequireAndRemove I am wondering if we required the package again at the end of the test the debug information would be the same, correct? Meaning each composer command invocation will still load the files from the server. I think this is still what we want.

We simply added the require at the end of the test could make sure we never try to put in more aggressive caching that would cause it be cached between commands.

Also in that test we have $debug = $this->composer('require', $package, '--with-all-dependencies', '-vvv') ->getErrorOutput(); and we do asserts like $this->assertStringContainsString('[TUF] Root metadata for http://localhost:8080/targets loaded from ', $debug);

Since we don't assert how many times something like this appears in the debug output I am not clear if this new static caching would make it clear in the new debug output that we only downloaded that one time.

If that is something we could test and could test locally that test would fail without these changes, that would be good.

If the static caching does not result in changes to the debug output of downloading files could we make a test scenario that would have differences with and without this changes? Like maybe requiring 2 packages in 1 command , where 1 package was dependent on the other?