Skip to content
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

High memory usage #47

Open
Jean85 opened this issue Oct 26, 2023 · 4 comments
Open

High memory usage #47

Jean85 opened this issue Oct 26, 2023 · 4 comments

Comments

@Jean85
Copy link
Owner

Jean85 commented Oct 26, 2023

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

memory_usage

@jarstelfox
Copy link

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
Copy link

jarstelfox commented Oct 26, 2023

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 composer/composer#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?

@Jean85
Copy link
Owner Author

Jean85 commented Oct 27, 2023

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"?

@jarstelfox
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants