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

Remove the memory limit hard coded #842

Closed
auroraeosrose opened this issue Jun 27, 2018 · 7 comments
Closed

Remove the memory limit hard coded #842

auroraeosrose opened this issue Jun 27, 2018 · 7 comments

Comments

@auroraeosrose
Copy link

auroraeosrose commented Jun 27, 2018

The excuse to just use the thread option is useless on windows since it doesn't support the threading with the pcntl forking model

And on windows - the 32 bit version of PHP is still in use in many places (sigh)

With the memory limit commented out, everything works fine.

Checks took 1.29 seconds and used 57.952MB of memory
Psalm was able to infer types for 86.760% of analyzed code (37 files)

If you're absolutely sure you need to police people - I'd recommend making the memory limit a command line option so people can override it without hacking code.

@auroraeosrose auroraeosrose changed the title Remove the memory limit option Remove the memory limit hard coded Jun 27, 2018
@muglug muglug closed this as completed in 4a97588 Jun 27, 2018
@muglug
Copy link
Collaborator

muglug commented Jun 27, 2018

I don't want to disable the memory limit in a minor version, but I've added --use-ini-defaults to prevent Psalm from overriding any PHP defaults.

@auroraeosrose
Copy link
Author

thank you :)

@muglug
Copy link
Collaborator

muglug commented Jun 30, 2018

Keeping this around for next major version

@SignpostMarv
Copy link
Contributor

without:

Fatal error: Allowed memory size of 2097152 bytes exhausted (tried to allocate 114688 bytes) in D:_git\daft-framework\daft-markup\vendor\composer\autoload_static.php on line 254
PHP Fatal error: Allowed memory size of 2097152 bytes exhausted (tried to allocate 114688 bytes) in D:_git\daft-framework\daft-markup\vendor\composer\autoload_static.php on line 254

with:

Checks took 7.25 seconds and used 56.890MB of memory
Psalm was able to infer types for 100.000% of analyzed code (18 files)

something funky is going on here if 56mb > 4gb :P

@SignpostMarv
Copy link
Contributor

@muglug @auroraeosrose have just ran some experiments by manually editing ./vendor/vimeo/psalm/src/psalm.php:

  • ini_set('memory_limit', '4096M'); : fails
  • ini_set('memory_limit', '4096MB'); : fails
  • ini_set('memory_limit', '4G'); : fails
  • ini_set('memory_limit', '4GB'); : fails
  • ini_set('memory_limit', 4294967296); : passes
  • ini_set('memory_limit', '4294967296'); : passes
  • ini_set('memory_limit', '4294967296b'); : passes
  • ini_set('memory_limit', 4 * 1024 * 1024 * 1024); : passes

$ php -v
PHP 7.0.20 (cli) (built: Jun 6 2017 14:32:47) ( ZTS )
Copyright (c) 1997-2017 The PHP Group
Zend Engine v3.0.0, Copyright (c) 1998-2017 Zend Technologies

looks like --use-ini-defaults is unnecessary if ini_set() is called with an int for memory_limit ?

SignpostMarv added a commit to SignpostMarv/psalm that referenced this issue Jul 3, 2018
 without requiring --use-ini-defaults argument to be passed
SignpostMarv added a commit to SignpostMarv/psalm that referenced this issue Jul 3, 2018
 without requiring --use-ini-defaults argument to be passed
muglug pushed a commit that referenced this issue Jul 3, 2018
…ithout requiring --use-ini-defaults argument to be passed (#860)
@fkupper
Copy link

fkupper commented Jan 22, 2020

Is this ever gonna change?

@weirdan
Copy link
Collaborator

weirdan commented Aug 17, 2020

There's a flag (--memory-limit) introduced in #3947

@weirdan weirdan closed this as completed Aug 17, 2020
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

5 participants