Temporarily increase D8 memory limit to reflect current requirements

So I think there are two things we should do:

• Temporarily increase the memory limit to something honest, both for DRUPAL_MINIMUM_PHP_MEMORY_LIMIT and SIMPLETEST_MINIMUM_PHP_MEMORY_LIMIT.
• Have a postponed critical to reduce the memory footprint (and the constants) to what we consider acceptable for release.

I temporarily changed the doc https://www.drupal.org/requirements/php
It should be updated again once a number is decided.

Tagging novice - patch itself should be straightforward.

Also let's update the issue summary to use the standard template and list the tasks. :)

Would something like this be enough? Or is there a need to have SIMPLETEST_MINIMUM_PHP_MEMORY_LIMIT set to an even higher value for now?

FYI, in my testing, loading the admin/config/development/testing page has proven to be painful to render with 128M (got WSODs). If did require 2 or 3 page loads to finally load it. Also, would EnvironmentTest.php have to be updated as well? I'm thinking about:

public function providerTestCheckMemoryLimit() {
  $memory_limit = ini_get('memory_limit');
  $twice_avail_memory = ($memory_limit * 2) . 'MB';

  return array(
    // Minimal amount of memory should be available.
    array('30MB', NULL, TRUE),
    // Exceed a custom (unlimited) memory limit.
    array($twice_avail_memory, -1, TRUE),
    // Exceed a custom memory limit.
    array('30MB', '16MB', FALSE),
    // Available = required.
    array('30MB', '30MB', TRUE),
  );
}

• For me admin/config/development/testing often whitescreens on a cold cache with 128 MB if I've installed Standard, but works fine on the second page load. Some of the longer-running tests have also timed out for me in the past on 128 MB. So I could go either way on that point... let's get more feedback:

  • Is there any way to deterministically determine what the maximum memory we actually need for the current test suite is?
  • What are the implications of setting it too high, other than not being able to run the test suite on a crappy host which I don't think we care about during development anyway?
  • Should we change the simpletest error in simpletest_requirements() from a requirements error to a requirements warning?

   

• I think we should also add an @todo to both docblocks explaining that these are temporary minima and referencing https://www.drupal.org/node/2289201.

   

• I considered whether we should also add a text message in the installer indicating that this is a temporary development requirement and people don't freak thinking we're going to ship requiring 128 MB. I'm on the fence about that since you can still install Drupal; you're just warned that you might need more.

   

Is there any way to deterministically determine what the maximum memory we actually need for the current test suite is?

I tried to find the issue where this was discussed for the installer (i.e. to handle different memory requirements of different distributions), but couldn't locate it. The short answer is 'no'.

What are the implications of setting it too high, other than not being able to run the test suite on a crappy host which I don't think we care about during development anyway?
Should we change the simpletest error in simpletest_requirements() from a requirements error to a requirements warning?

Yes to both.

I'm OK with the embarrassment of an embarrassingly high memory requirement on the installer for now, let's not add any promises that it's going to be fixed.

Here's a new patch with the recommendations of #8 and #9.

Oops. This should be better.

The completely revised + final incarnation of #697760: Replace getInfo() in tests with native phpDoc + annotations (following PHPUnit) (not posted yet) reduces the total memory consumption for discovering tests to ~11MB (with a peak of ~32MB).

In other words, the current memory consumption for the (simple)test (UI) should be ignored for this issue.

--

-const DRUPAL_MINIMUM_PHP_MEMORY_LIMIT = '32M';
+const DRUPAL_MINIMUM_PHP_MEMORY_LIMIT = '128M';

Before bumping the regular memory limit from 32MB to 128MB, it would be sensible to first perform an analysis to determine what exactly causes the heavy memory consumption.

In other words, the current memory consumption for the (simple)test (UI) should be ignored for this issue.

Excellent news. Should we wait until it has landed to make any adjustment though?

Before bumping the regular memory limit from 32MB to 128MB, it would be sensible to first perform an analysis to determine what exactly causes the heavy memory consumption

I'm very much aligned with this but we should then clarify

I'm OK with the embarrassment of an embarrassingly high memory requirement on the installer for now, let's not add any promises that it's going to be fixed.

in #9. I'm not sure what could not be fixed with profiling and performance work and assume it's more a matter of releasing D8 sooner than later, even if that means we should have much higher requirements than we would normally find acceptable.

I think we can independently move forward here; simply remove the changes to simpletest.

I'm surprised by @catch's OK in #9. Though I could get behind it, if we already know the cause and have a plan to fix it. (which I'm not aware of)

OK, back to the regular SIMPLETEST_MINIMUM_PHP_MEMORY_LIMIT setting (which ATM is 64M, not 32M).

Re #12:

We should not wait to find out why the memory usage is high.
We know it is. People installing Drupal 8 for the first time(s) think they just need what our current requirements are (they need more), and they are misled by the errors they get on a site with too little memory.

#2289201: [Meta] Make drupal install and run within reasonable php memory limits so we can reset the memory requirements to lower levels Should have a child issue that is: perform an analysis to determine what exactly causes the heavy memory consumption (or something like that.)

We can do that in that issue.

People installing Drupal 8 for the first time(s) think they just need what our current requirements are (they need more), and they are misled by the errors they get on a site with too little memory.

I just tried installing HEAD with 32M and it worked fine. Do we have any precise example of what is failing for people so that I can reproduce?

$ php -c /etc/php5/apache2/php.ini -i | grep memory_limit
memory_limit => 32M => 32M

@anavarre in general: #2021029: During Install, Inform User that they have Insufficient Memory Rather than Server Error: AJAX HTTP Result Code: 500 [edit: we could ask those people for more information. what kind of info do we want to ask them for? we should also tell them how they can look up the info we ask for.]

opened #2294569: Determine cause of high memory consumption

Aha, I think you've just brought up the perfect issue to move forward. Thanks!

In #2021029: During Install, Inform User that they have Insufficient Memory Rather than Server Error: AJAX HTTP Result Code: 500 comment #26 says that with APC on the installer works fine with 32M but as soon as it's turned off it fails. Not only it makes a great deal of sense but I was also able to reproduce this. Looks like it failed around the time it was installing the image module (step 27/38)

An AJAX HTTP error occurred.
HTTP Result Code: 500
Debugging information follows.
Path: http://temp.local/core/install.php?langcode=en&profile=standard&id=1&op=do_nojs&op=do
StatusText: Internal Server Error
ResponseText: 

I'm running PHP 5.5 which means I also benefit from the built-in Zend opcode cache. I had to disable it and then it failed.

So, what I think we should focus on is people using 5.4 or 5.5 with APC or Zend opcode cache turned off. I'll ask just that in #2021029: During Install, Inform User that they have Insufficient Memory Rather than Server Error: AJAX HTTP Result Code: 500.

FWIW the installer works fine with 64M and Zend Opcache turned off. If we were to indeed temporary up the requirements, 64M would seem like a good compromise until #2294569: Determine cause of high memory consumption is fixed. I can't test with 5.4 but if anybody could confirm it behaves the same, that would be really helpful.

@anavarre, can you please provide interdiffs when you upload new patches?

+++ b/core/includes/bootstrap.inc
@@ -24,8 +24,10 @@
+ * @todo This is a temporary minima pending https://www.drupal.org/node/2289201

Minima is plural. Mimumum is the singular. :)

+++ b/core/modules/simpletest/simpletest.install
@@ -57,7 +57,7 @@ function simpletest_requirements($phase) {
-    $requirements['php_memory_limit']['severity'] = REQUIREMENT_ERROR;
+    $requirements['php_memory_limit']['severity'] = REQUIREMENT_WARNING;

This change was intended to be combined with increasing the simpletest memory limit to something big; it's irrelevant without the change to the simpletest memory.

I agree with @YesCT; it doesn't make sense to postpone increasing the limit on other issues. We have a critical followup to get it back down.

Even with my local memory limit at 128 MB, it's now taking 3 page loads to load the complete test list page after installing simpletest. So +1 for #697760: Replace getInfo() in tests with native phpDoc + annotations (following PHPUnit). Note that the issue is beta deadline so would need to be done soon...

So, after experimenting with my dev environment a bit, I'd still recommend increasing the simpletest memory limit to 256 and referencing both the critical followup issue and perhaps sun's issue as well in @todo in the docblock.

@xjm: thanks for your guidance. Let me please know if there's still something I need to learn how to do better?

• Set minimum requirement to 64M for the installer per #20 until someone confirms/denies it's fine for now.
• Up simpletest's minimum requirement to 256M per #22 and pending @sun's #697760: Replace getInfo() in tests with native phpDoc + annotations (following PHPUnit)
• Leave simpletest's REQUIREMENT_WARNING as is since we're aiming at 256M for now

If the requeriments change when and OPCACHE is enabled Why do not use a conditional?

if ((extension_loaded('apc') && ini_get('apc.enabled')) || ini_get(opcache.enable)) {
  const DRUPAL_MINIMUM_PHP_MEMORY_LIMIT = '32M';
}
else {
  const DRUPAL_MINIMUM_PHP_MEMORY_LIMIT = '64M';
}

@sun by #9 I mean:

1. People cannot install Drupal for development now, so we should just raise the memory limit to reflect reality. No point pretending it's better than it is.

2. Similarly, let's not sugar coat how bad the memory usage is in the requirements description, if it really is 128M on some systems that is bad whether we're 'in development' or not.

3. #1744302: [meta] Resolve known performance regressions in Drupal 8 is already open for performance issues - however I do think this deserves its own dedicated critical issue. Drupal 7 memory requirements didn't really get analyzed until about a month before release. To meaningfully look at memory usage, we should be checking both the installer, and a standard (or bigger than standard) install with completely empty caches. I'd probably look at YAML discovery/parsing first.

@oriol_e9g: to my knowledge you can't do this: http://www.php.net/manual/en/language.constants.syntax.php

constants defined using the const keyword must be declared at the top-level scope because they are defined at compile-time. This means that they cannot be declared inside functions, loops, if statements or try/ catch blocks.

Also, for PHP 5.5, the extension check would have to be extension_loaded('Zend Opcache').

Thanks @anavarre. I think that's what we want. Sorry for all the criss-crossed recommendations. :) I just tweaked the formatting of the @todo a bit based on catch's comment and wrapped at 80 chars based on the documentation standards.

I also tested installing standard with 64 MB on MAMP with both 5.4.26 and 5.5.10, no opcode cache, and was able to get through the installer. (This wasn't true earlier in the cycle, so yay.) So until we get someone who can confirm they can't install with 64 MB on their environment, I think that makes sense as the limit we recommend.

Agreed with #25.

It's possible to define constants conditionally with define() instead of const. That might be worth looking into; however, it gets us into a broader discussion around whether or not the Drupal installer should respond to specific environment configurations (like the availability of APC, see #2248767: Use fast, local cache back-end (APCu, if available) for low-write caches (bootstrap, discovery, and config) for example). So maybe we can look into that in a followup issue.

TIL we don't do anything with REQUIREMENT_WARNING during the install phase for modules. (See in drupal_check_error()). IMO that's a bug and I might file a separate patch; a warning should at least display a dsm() IMO if you implement hook_requirements() that way.

Here's screenshots of too-low memory limits for both installing core and the status report page after installing simpletest.

Filed #2295051: Display requirements warnings on module installation.

I closed #2062057: Update PHP requirements page for Drupal 8: execution time as a duplicate. While that issue was older, this one has more activity and a patch.

I updated the title to reflect the other issue's claim that max_execution_time also needs to be raised.

let's reuse that other issue or make a new one for the execution time.

I did not test manually, but that was done by others here. great.

I read all the lines in the patch.
It stays in scope, is well documented with related issues, and the standards stuff is good too.

... updating the issue summary and removing Needs issue summary update tag.

not sure about change record... let's see.

+++ b/core/modules/simpletest/simpletest.install
@@ -10,8 +10,12 @@
 /**
  * Minimum value of PHP memory_limit for SimpleTest.
+ *
+ * @todo Reduce the memory required to use SimpleTest on some environments in
+ *   https://www.drupal.org/node/2289201 and https://www.drupal.org/node/697760
+ *   and then then decrease this limit.
  */
-const SIMPLETEST_MINIMUM_PHP_MEMORY_LIMIT = '64M';
+const SIMPLETEST_MINIMUM_PHP_MEMORY_LIMIT = '256M';

Unfortunately this is not currently enough to render the Simpletest UI when the cache needs rebuilding. See #2307163: BrokenSetUpTest failing randomly with filling up memory. Not entirely sure what to do here.

This doesn't need a CR, I don't think. We've already updated https://www.drupal.org/requirements.

Context for #33: The current increase in the need for memory for the test runner UI was probably introduced with #1825952: Turn on twig autoescape by default, which is why 256 was enough before but isn't now.

@alexpott, I think we should just be honest about the current memory requirements and then reduce the number when we have addressed the issues with overall memory usage and with the SimpleTestUI.

Patch attached that changes the simpletest limit to 320 MB (which @alexpott confirmed is sufficient) and updates the @todo to remove the reference to a fixed issue and adds one to the new critical.

Leaving at RTBC; please bump back if you think the 320 number should have more discussion. :)

Also noticed a typo. :)

These are fine, we can review upwards and downwards as needed, thanks for the critical follow-up to sort out the actual issues.

Committed/pushed to 8.x, thanks!