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

exceeded memory limit with HHVM and PHP 4.8.24 #7035

Closed
photodude opened this issue May 4, 2016 · 32 comments
Closed

exceeded memory limit with HHVM and PHP 4.8.24 #7035

photodude opened this issue May 4, 2016 · 32 comments
Assignees
Labels

Comments

@photodude
Copy link
Contributor

photodude commented May 4, 2016

HHVM Version

3.15.1
3.15.0
3.14.1
3.13.1
3.12.8
3.9.9
3.6.6
3.5.0

Standalone code, or other way to reproduce the problem

you can see the results on travis ci testing against the staging branch of Joomla (3.5.2/3.6.0)
https://travis-ci.org/photodude/joomla-cms/jobs/127696426

Expected result

No error running out of memory
even repeated with memory_limit = -1 (reported limit 9223372036854775807) no change

Actual result

Fatal error: request has exceeded memory limit in /vendor/phpunit/php-timer/src/Timer.php on line 97

Related Issue #5520 (showing an older segfault and this same out of memory issue)

@photodude
Copy link
Contributor Author

photodude commented May 20, 2016

Memory is always reporting as memory_get_peak_usage() ... int(2097152) which is a value below the actual memory limit
even when the memory limit is set to -1 we get memory_get_peak_usage() ... int(2097152) and still get the Fatal error: request has exceeded memory limit

@Orvid
Copy link
Contributor

Orvid commented May 20, 2016

In that job you are calling memory_get_peak_usagein a different process, which is not going to give you the actual memory usage. What does it give you if you have it output some info while running the tests?

@photodude
Copy link
Contributor Author

photodude commented May 20, 2016

@Orvid I guess I'm not sure how to use memory_get_peak_usage() from the command line to get the peak memory useage, (my option since this is testing on Travis CI)

I tried doing var_dump(memory_get_peak_usage()); in the actual unit test that is running when the memory failure occurs, but I got request has exceeded memory limit in joomla-cms/tests/unit/suites/finderIndexer/stemmer/FinderIndexerStemmerPorter_EnTest.php on line 122 which is the line where I added the memory_get_peak_usage() Each function I move the memory_get_peak_usage() to results in the same issue.

@photodude
Copy link
Contributor Author

@Orvid

In my continued attempts to test this issue, I'm finding a consistent pattern. Using memory_get_peak_usage() or memory_get_usage() functions will almost always result in Fatal error: request has exceeded memory limit I'm hunting for what the first test is that this occurs on.

Taking a look at the timer function in phpunit that fails at the end of my tests, it is basically a call to memory_get_peak_usage()

@photodude
Copy link
Contributor Author

@Orvid The in the sequence of our unit tests I found where it doesn't fail with 'Fatal error: request has exceeded memory limitI get a reported int(77594624) formemory_get_peak_usage()`

with ini_get("memory_limit") reporting string(11) "17179869184" for the available limit.

Here is a link to the travis build https://travis-ci.org/photodude/joomla-cms/jobs/133418228

@Orvid
Copy link
Contributor

Orvid commented May 27, 2016

I'm really not sure what the issue here is, if it's always claiming the request has exceeded the memory limit, it should be able to run in the first place...

@photodude
Copy link
Contributor Author

@Orvid As far as I can tell it seems we get the Fatal error: request has exceeded memory limit when a certain level of memory is used and memory_get_peak_usage() is called.

So in the initial report we were getting the error when PHP Unit was finalizing the tests and makes a call to phpunit's php-timer Timer::resourceUsage() which is basically just a call to the function memory_get_peak_usage().

Digging deeper I was seeing every attempt to check our various unit test with memory_get_peak_usage() also failing by Fatal error: request has exceeded memory limit. Eventually, I had gone through enough of the test list to find a test that didn't fail when I called memory_get_peak_usage() in the unit test. That test had a memory_get_peak_usage() = int(77594624) well below the reported system memory limit of string(11) "17179869184" I agree with reported memory numbers like those there should be no issue with running things

@Orvid
Copy link
Contributor

Orvid commented May 28, 2016

Well, we know that there was a refcounting issue in interp mode that @mxw caused when he converted a bunch of things to TypedValue, but that's not part of the 3.13.1 release, and has already been fixed in trunk, so it shouldn't be that.

Are you sure the request really needs 77mb of memory to run? As you mentioned above, the base memory point is 2mb, so 75mb sounds a bit excessive for a single test.

@photodude
Copy link
Contributor Author

photodude commented May 28, 2016

@Orvid as far as I can tell it seems when running under hhvm the memory use just grows with each unit test. The ~74mb is just the value in one test where I could get a report from memory_get_peak_usage() without the whole test sequence crashing with Fatal error: request has exceeded memory limit at the point I tried to get the memory useage.

Here is a comparison of the total memory usage from our unit tests all of which are well below the system memory limit for our HHVM tests of ~16384mb

PHP version Total Memory use by PHP Unit tests
PHP 5.3 Memory: 352.50Mb
PHP 5.4 Memory: 313.75Mb
PHP 5.5 Memory: 314.50Mb
PHP 5.6 Memory: 314.50Mb
PHP 7.0 Memory: 176.00Mb
HHVM Fatal error: request has exceeded memory limit

I also tried testing with the 3.12 and the nightly build about a week ago, but those releases did not fix this issue

@photodude
Copy link
Contributor Author

photodude commented May 30, 2016

I just did a massive review of our unit tests, I was able to shave off a sizable amount of memory use. but it didn't fix this issue with HHVM

After recent Patch

PHP version Total Memory use by PHP Unit tests
PHP 5.3 Memory: 271.50Mb
PHP 5.4 Memory: 240.00Mb
PHP 5.5 Memory: 241.25Mb
PHP 5.6 Memory: 240.25Mb
PHP 7.0 Memory: 138.00Mb
HHVM Fatal error: request has exceeded memory limit

@photodude
Copy link
Contributor Author

Possibly related to #4723

@photodude
Copy link
Contributor Author

photodude commented Sep 6, 2016

@Orvid I was able to pin down that there is an issue in memory_get_peak_usage() causing the error, but I haven't been able to determine how much memory is being used at the point of failure.

All of our unit tests are running with less than 300MB now and the system is reporting plenty of memory to support that. My best guess is memory_get_peak_usage() is dying when memory for a process exceeds some value below 300MB.

Not sure if there is an easy way to write a script to take up memory in increasing amounts and report the value until it either fails or reaches the system limit. with a little script like that we could run memory_get_peak_usage() through the paces to see if/when it fails. maybe something like this

@photodude
Copy link
Contributor Author

photodude commented Sep 12, 2016

Based on some information in my recent tests I believe just before this failure 712 mb of memory are being used; well below the 7gb assigned to HHVM

@photodude
Copy link
Contributor Author

photodude commented Sep 13, 2016

@Orvid are there any ini settings that could be set at runtime which might shed more light on this failure?

@photodude
Copy link
Contributor Author

I've done a little testing with running phpunit with -d memory_limit=##M I found at or under 19M a few tests completed and I get the following error
Fatal error: request has exceeded memory limit in /vendor/sebastian/global-state/src/Snapshot.php on line 313 (php 5.6 fails the test suite at anything under 230M)

but at >20M all the tests complete but I get the original error
Fatal error: request has exceeded memory limit in /vendor/phpunit/php-timer/src/Timer.php on line 97

pushing the other way, to 1G or even -1 (unlimited) there was no change. the tests complete and I still get the error.

@aorenste aorenste self-assigned this Sep 14, 2016
@aorenste
Copy link
Contributor

I was able to reproduce this locally. The test suite also revealed a few (fairly new) unrelated compiler bugs.
:)

@photodude
Copy link
Contributor Author

@aorenste glad you were able to reproduce the issues. I look forward to these being fixed.

@aorenste
Copy link
Contributor

The issue is due to this implementation of ini_restore:

static void HHVM_FUNCTION(ini_restore, const String& varname) {
}

I'll work on fixing it tomorrow.

@photodude
Copy link
Contributor Author

photodude commented Sep 16, 2016

@aorenste On a semi unrelated note: I wonder if the "unrelated compiler bugs" could be related to the failure of phpunit's self test suite on HHVM? see build https://travis-ci.org/photodude/phpunit/jobs/160342275
I assume that most of the failures of phpunit's self test suite on HHVM with php 7 mode are related to #7198 see build https://travis-ci.org/photodude/phpunit/jobs/160342276

@photodude
Copy link
Contributor Author

@aorenste I have updated the issue with all of the HHVM versions where I have seen memory_get_peak_usage() cause the Fatal error: request has exceeded memory limit related to the implementation of ini_restore

@photodude
Copy link
Contributor Author

@aorenste will this fix be included in 3.15.1 and older affected releases?

@aorenste
Copy link
Contributor

It should be in 3.15.1 but not older releases.

@photodude
Copy link
Contributor Author

@aorenste I look forward to 3.15.1 so I can resume testing.
I wish I could get the nightly builds running on Travis CI right now, it's unfortunate that apt-get continues to install 3.15.0 when nightly is specified.

Have the other unrelated compiler bugs with the test suit you mentioned been addressed?

@aorenste
Copy link
Contributor

The 3.15.1 branch has been cut but we're going to cherry-pick the required patches so they should be in there.

@photodude
Copy link
Contributor Author

I look forward to testing the fixes in 3.15.1 (since I still can't get nightly to install on travis ci)
just a note: I added a logger on memory resources to tell me the current state after each unit test. the last report said 255.97MB so it seems memory_get_peak_usage() was failing at about 256MB

@photodude
Copy link
Contributor Author

photodude commented Oct 2, 2016

@aorenste I saw HHVM 3.15.1 has been released. did the patch for this make it into that release?

I'm still seeing the issue with memory_get_peak_usage() failing at just under 256MB on 3.15.1
https://travis-ci.org/joomla/joomla-cms/jobs/164357654

@photodude
Copy link
Contributor Author

@aorenste I can confirm the fix in hhvm-nightly, but not with hhvm 3.15.1. Will the patch make it into 3.15.2?

paulbiss pushed a commit that referenced this issue Oct 3, 2016
Summary:
Save ini values so they can be restored with ini_restore()

Closes #7035

Reviewed By: markw65

Differential Revision: D3878335

fbshipit-source-id: 8f052aa63264fa1d70ca095bcd650b3663872cc5
@paulbiss
Copy link
Contributor

paulbiss commented Oct 3, 2016

@photodude: apologies, it seems to have been overlooked 🙁

I pushed the commit to the 3.15 branch, so 3.15.2 will absolutely have this fix.

@photodude
Copy link
Contributor Author

Thanks @paulbiss

@photodude
Copy link
Contributor Author

@paulbiss would it be possible for the patch to be applied to the 3.12 and 3.9 branches?
These branches are also affected and were also recently updated.

@paulbiss
Copy link
Contributor

paulbiss commented Oct 5, 2016

@photodude 3.9 is EOL so it's unlikely to receive any further updates. As for 3.12 it's old enough now that I'm really only comfortable with security patches, is there a reason it's important to backport this to 3.12?

@photodude
Copy link
Contributor Author

photodude commented Oct 5, 2016

No, important reason. It's likely that we won't support anything older that has this issue since we will never be able to test against it. I only asked because those versions were affected and I had seen new releases. I was actually surprised to see a 3.9 release I figured the docs must be out of dates since I was under the impression that 3.9 was EOL on 15 July 2016.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants