pretty-package-versions icon indicating copy to clipboard operation
pretty-package-versions copied to clipboard

High memory usage

Open Jean85 opened this issue 1 year ago • 4 comments

During https://github.com/getsentry/sentry-php/issues/1606, the user discovered a high memory usage by Jean85\PrettyVersions::getVersion, more specifically in the Jean85\PrettyVersions::checkProvidedPackage private method:

memory_usage

Jean85 avatar Oct 26 '23 08:10 Jean85

Hey there! Thanks for the fast reply. I’ll try and get more details for you.

Maybe you have a very wide dependency graph?

This is totally possible. I have not dug that far in our codebase yet.

I did notice a possible route that would cause more memory than you expect.

Composer’s InstalledVersions can possibly require files: https://github.com/composer/composer/blob/7a09e05560652f24a2d14e5966fa2f118f499e3e/src/Composer/InstalledVersions.php#L332C12-L332C12

if any of those files have side effects that could explain the unexpected increase

jarstelfox avatar Oct 26 '23 08:10 jarstelfox

Okay, I ran this down a bit.

We are running a decent-sized app, but the composer.json is reasonable. 80 total packages (17 are require-dev).

I am not noticing anything super deep when running composer show --tree. The max depth I at any given point is 5, but we do have a decent number of 4-5 depth dependencies.

As I was digging, I ran across https://github.com/composer/composer/issues/11018

Due to this, I decided to change the /vendors/composer/installed.php's version array to 'versions' => array() and the memory usage was fine.

It's likely you use this information so this is a non-starter. However, it does indicate it may be related.


Update: I found vendor/composer/installed.php is nearly the same amount of bytes used. Thus, it's likely that the file is simply quiet large.

I see the usefulness of using this composer class / file, but it may be possible to bypass requiring and instead parsing the file?

jarstelfox avatar Oct 26 '23 16:10 jarstelfox

Yes you found the correct bottleneck: I use the Composer API to reach into installed.php and obtain the needed info. I've tried reproducing the Sentry use case in a naked PHP file and profile it, and tried a couple of ways to cut out the memory usage, but the size of that array is still unavoidable, the profile had the same size nearly down to the bit: that array contains info for all dependencies, direct and indirect, so you don't have any way around it.

I see the usefulness of using this composer class / file, but it may be possible to bypass requiring and instead parsing the file?

What do you mean with "parsing the file"?

Jean85 avatar Oct 27 '23 08:10 Jean85

What do you mean with "parsing the file"?

Trading memory for CPU effectively: Instead of loading the entire file into memory (via composer), you could write a custom implementation that regexes the file for relevant info.

I do not believe it is a good idea, but I thought I'd suggest it. It's a poor option for maintainability.

The main reason I am suggesting it is that you should be able to only get the exact information from the file on a string-by-string basis keeping memory overhead low.

You do not need to this approach.

jarstelfox avatar Oct 27 '23 17:10 jarstelfox