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

Add note about possibly needed memory_limit increase #36

Closed
wants to merge 1 commit into from

Conversation

thinkspill
Copy link

No description provided.

@ondrejmirtes
Copy link
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
Author

Agreed, that would be ideal.

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

@ondrejmirtes
Copy link
Member

Solved it in c4a5846 and a few further commits.

@thinkspill
Copy link
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
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
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.

None yet

2 participants