Eliminate all raise_memory_limit() calls

[François Marier]

There are very few places in Mahara were we raise PHP's memory limit, but it is bad practice since that memory may not be reclaimed by Apache until that child process is terminated.

Unless we are confident that a particular operation is impossible to do without raising the memory limit, we should rewrite code which fails to work within 32 MB of memory.

[Hugh Davenport (hugh-davenport)]

affects the following:

./htdocs/export/html/lib.php: raise_memory_limit('128M');
./htdocs/lib/csvfile.php:raise_memory_limit("512M");
./htdocs/lib/cron.php:raise_memory_limit('128M');
./htdocs/lib/db/upgrade.php: raise_memory_limit('256M');
./htdocs/lib/mahara.php: raise_memory_limit('128M');
./htdocs/lib/mahara.php:function raise_memory_limit ($newlimit) {
./htdocs/admin/users/uploadcsv.php:raise_memory_limit("512M");
./htdocs/admin/users/bulkdownload.php:raise_memory_limit("1024M");
./htdocs/admin/users/bulkimport.php:raise_memory_limit('1024M');

[Robert Lyon (robertl-9)]

Instead of eliminating them we now have more :(

./htdocs/lib/cron.php:raise_memory_limit('128M');
./htdocs/lib/csvfile.php:raise_memory_limit("512M");
./htdocs/lib/mahara.php: raise_memory_limit('128M');
./htdocs/lib/mahara.php:function raise_memory_limit ($newlimit) {
./htdocs/lib/imageresizer.php: raise_memory_limit($newlimitMB . 'M');
./htdocs/lib/db/upgrade.php: raise_memory_limit('256M');
./htdocs/admin/users/bulkimport.php:raise_memory_limit('1024M');
./htdocs/admin/users/uploadcsv.php:raise_memory_limit("512M");
./htdocs/admin/users/bulkdownload.php:raise_memory_limit("1024M");
./htdocs/admin/groups/uploadcsv.php:raise_memory_limit("512M");
./htdocs/admin/groups/uploadmemberscsv.php:raise_memory_limit("512M");
./htdocs/export/html/lib.php: raise_memory_limit('128M');
./htdocs/import/index.php:raise_memory_limit("512M");
./htdocs/search/elasticsearch/cron.php:raise_memory_limit('256M');

[Robert Lyon (robertl-9)]

Would flushing the output buffer for each iteration of the heavy load loops help? Or is more a server machine problem?

[Son Nguyen (ngson2000)]

I would use KCachegrind for analysing the memory usage of this scripts then optimize them.

[Son Nguyen (ngson2000)]

I would classify this bug as a new feature.

[Nigel Cunningham (nigelc-g)]

I'm going to give this a go...

[Mahara Bot (dev-mahara)]

Patch for "master" branch: https://reviews.mahara.org/3526

[Mahara Bot (dev-mahara)]

Patch for "master" branch: https://reviews.mahara.org/3527

[Mahara Bot (dev-mahara)]

Patch for "master" branch: https://reviews.mahara.org/3528

[Mahara Bot (dev-mahara)]

Patch for "master" branch: https://reviews.mahara.org/3529

[Mahara Bot (dev-mahara)]

Patch for "master" branch: https://reviews.mahara.org/3530

[Nigel Cunningham (nigelc-g)]

After having done some profiling and preparation of patches over the last few days, I think 32M is reasonable for normal operation, but we will definitely still need to raise the memory limit for some operations.

The main one I've focused on so far is adding users from a CSV file. Profiling got me focusing on execution time as well as memory use, and I have a few patches that together do better than halving the execution time (one in particular - to handle_event - will probably have far wider implications and accounts for most of the savings) while only raising memory use by a small amount. 64MB is more than enough even for an import of 5000 users.

[Mahara Bot (dev-mahara)]

Patch for "master" branch: https://reviews.mahara.org/3608

[Mahara Bot (dev-mahara)]

Reviewed: https://reviews.mahara.org/3608
Committed: http://gitorious.org/mahara/mahara/commit/039564a50799e31e2048c2aed2da888cfd5dd628
Submitter: Robert Lyon (<email address hidden>)
Branch: master

commit 039564a50799e31e2048c2aed2da888cfd5dd628
Author: Nigel Cunningham <email address hidden>
Date: Wed Jul 30 14:22:15 2014 +1000

(Bug785472) Remove unneeded raise_memory_limit calls

On the basis of my profiling and testing over the last week,
all of the scripts affected by this patch don't need their
raise_memory_limit calls. Ensure_sanity currently sets the
memory limit to 128MB, which has been seen to be more than
sufficient for each of these use cases.

Tests have involved at least 1000 records being imported in
each case, and sometimes as many as 20,000. I would have
liked to have tested with some really full profiles (eg
lots of pictures and content), but am satisfied that there's
enough margin to cover those cases. In any case, such files
are often handled using external apps and therefore won't
be counted toward PHP's memory use anyway. Finally, the
limit can easily be increased by the user if necessary for
a particular case.

Change-Id: Ifecc83fd47da51268bae6cbd6960735eb91f9403
Signed-off-by: Nigel Cunningham <email address hidden>

[Mahara Bot (dev-mahara)]

Patch for "15.04_STABLE" branch: https://reviews.mahara.org/5122

[Mahara Bot (dev-mahara)]

Reviewed: https://reviews.mahara.org/5122
Committed: https://git.nzoss.org.nz/mahara/mahara/commit/19b12b53cdcdb2bdb7024fd0a816416fba0e096f
Submitter: Robert Lyon (<email address hidden>)
Branch: 15.04_STABLE

commit 19b12b53cdcdb2bdb7024fd0a816416fba0e096f
Author: Nigel Cunningham <email address hidden>
Date: Wed Jul 30 14:22:15 2014 +1000

(Bug785472) Remove unneeded raise_memory_limit calls

On the basis of my profiling and testing over the last week,
all of the scripts affected by this patch don't need their
raise_memory_limit calls. Ensure_sanity currently sets the
memory limit to 128MB, which has been seen to be more than
sufficient for each of these use cases.

Tests have involved at least 1000 records being imported in
each case, and sometimes as many as 20,000. I would have
liked to have tested with some really full profiles (eg
lots of pictures and content), but am satisfied that there's
enough margin to cover those cases. In any case, such files
are often handled using external apps and therefore won't
be counted toward PHP's memory use anyway. Finally, the
limit can easily be increased by the user if necessary for
a particular case.

behatnotneeded

Change-Id: Ifecc83fd47da51268bae6cbd6960735eb91f9403
Signed-off-by: Nigel Cunningham <email address hidden>
(cherry picked from commit 039564a50799e31e2048c2aed2da888cfd5dd628)