Skip to content

Add note about possibly needed memory_limit increase#36

Closed
thinkspill wants to merge 1 commit into
phpstan:masterfrom
thinkspill:patch-1
Closed

Add note about possibly needed memory_limit increase#36
thinkspill wants to merge 1 commit into
phpstan:masterfrom
thinkspill:patch-1

Conversation

@thinkspill

Copy link
Copy Markdown

No description provided.

@ondrejmirtes

Copy link
Copy Markdown
Member

I was rather thinking about detecting this state and showing message about memory limit in the next run. It's more intuitive than putting it in the README. What do you think?

@thinkspill

Copy link
Copy Markdown
Author

Agreed, that would be ideal.

Mentioning it in the README may be a quick fix until that happens.

@ondrejmirtes

Copy link
Copy Markdown
Member

Solved it in c4a5846 and a few further commits.

@thinkspill

Copy link
Copy Markdown
Author

I tried this change today, and I'm not sure it solves the issue.

When I first ran analyse and got the unexplained crash, my first instinct wasn't to run it again (which, now, would cause the OOM message to appear). Instead I Googled it and checked the Issues here, where I then learned of the OOM issue.

The issue is that there is still no up-front notice or warning about the very likely OOM crash. Only if you run it twice in an OOM situation will you then be notified. At the very least, I would mention this possibility in the docs.

As another possible solution, you could potentially warn up-front on first-run by guesstimating memory usage needs based on how many files are to be included in the scan, though this may require adding a "pre-scan scan". PHPStan is fast enough compared to phan that this wouldn't bother me much.

@ondrejmirtes

Copy link
Copy Markdown
Member

Yep, my solution assumes the user will re-run the application after it crashes for the first time. I think it's a quite natural thing to do. If something crashes unexpectedly, my reaction is "whoa, what happened!? Let me make sure I haven't overlooked anything when the application was executed by rerunning it one more time".

I don't think there is any deterministic way to guesstimate how much memory will be consumed by pre-scanning the codebase. I don't think it's worth it to slow down PHPStan for everyone just to inform people with low memory limit about a possible crash. But I can add a note about this to the README. Thanks for the suggestions!

@igads igads mentioned this pull request Aug 27, 2021
@evandrojr evandrojr mentioned this pull request Jul 10, 2023
@johntheteam johntheteam mentioned this pull request Aug 17, 2024
@klewi1983 klewi1983 mentioned this pull request May 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants