Make WordPress Core

Opened 7 years ago

Closed 6 years ago

Last modified 5 years ago

#41641 closed enhancement (fixed)

Unit testing reaching the memory limit

Reported by: hristo-sg's profile Hristo Sg Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: 5.1 Priority: normal
Severity: normal Version:
Component: Build/Test Tools Keywords: commit
Focuses: Cc:

Description

While setting the unit testing on a standard GoGeek account, we hit the memory limit of 768M. It's quite a big number so I believe there should be something done to lower the memory consumption of the test or even implementing batch processing of the tests.

............................................................. 6466 / 8021 ( 80%)
...........................................................
Fatal error: Out of memory (allocated 770179072) (tried to allocate 64 bytes) in /home/*/public_html/wp-test-runner/src/wp-includes/rest-api/endpoints/class-wp-rest-controller.php on line 519
Error: Failed to perform operation.

Attachments (1)

41641.1.diff (402.3 KB) - added by danielbachhuber 6 years ago.
Replace use of $this->server with rest_get_server() for better memory recycling

Download all attachments as: .zip

Change History (17)

#1 @danielbachhuber
7 years ago

@jorbin @johnbillion Before I dive into this, are you aware of any recent efforts to profile the PHPUnit test suite's memory consumption?

This issue is in relation to this project: https://github.com/wordpress/phpunit-test-runner The end goal is to have the test suite run on a variety of shared hosting plans. We'd ideally like to be able to do so without any environment modifications.

#2 @netweb
7 years ago

There's some stats in #33806 from ~2 years ago with 3 of 4 reports using ~230mb

#3 @johnbillion
7 years ago

  • Type changed from defect (bug) to enhancement
  • Version trunk deleted

The highest memory usage I can see on Travis is the full Multisite suite on PHP 5.3 which peaks at 918.25MB.

More than happy to help with memory usage reductions, but I've not done any profiling myself recently.

This ticket was mentioned in Slack in #hosting-community by pandjarov. View the logs.


7 years ago

This ticket was mentioned in Slack in #core by coderkevin. View the logs.


7 years ago

#6 @coderkevin
7 years ago

I observed some massive memory usage when running PHPUnit tests for the REST API. I dug into this about halfway during the WCUS contributor day, but ran out of time. Here's what I observed so far as I can recall:

  • A lot of memory churn occurs from bringing up basically all of WordPress with each test and shutting it down again.
  • The tests I observed didn't seem to get cleared from memory, the memory consumption increased linearly with each test run.

At the time, I ran this on vvv using the wordpress-develop server. I also ran out of memory when I tried to run all of the REST API tests.

I will take a look at profiling this again and see what I can come up with and report back here. If anyone else has done some profiling or wants to, I'm happy to collaborate.

#7 @danielbachhuber
6 years ago

  • Milestone changed from Awaiting Review to Future Release

I spent some time profiling and researching this morning.

Running the test suite locally, here's observed memory usage:

75% test suite completion == 2 GB resident memory

https://user-images.githubusercontent.com/36432/36432869-1336403e-1610-11e8-844d-1bcc66ab9db9.png

99% test suite completion == 3.1 GB resident memory

https://user-images.githubusercontent.com/36432/36432929-37c4cb5a-1610-11e8-8c4b-c7141f2b5cf6.png

Generally, memory usage scaled in a relatively linear way throughout the test suite. However, the Blackfire report for the same test run only reports 505 MB memory usage.

From StackOverflow, I learned:

Regarding memory_get_usage(), this reports the heap memory actually allocated by systems using PHP's internal memory manager. This means, libraries which directly use other memory managers of the system (mmap(2) or malloc(3)) do not expose their memory usage here. [This is for example why mysqlnd does show much memory usage while libmysqlclient does not - the latter uses malloc() internally.]

My guess at this point is that some PHP extension (ImageMagick?) is holding on to memory as the test suite progresses.

The REST API tests are definitely a source of slowness. In particular, WP_Test_REST_Attachments_Controller calls copy() twice before each test, and there's a lot of image resizing going on.

I also rediscovered how slow remove_added_uploads() is when you already have files in your uploads directory.

Not quite sure of next steps yet. Open to suggestions.

#8 @danielbachhuber
6 years ago

99% test suite completion == 3.1 GB resident memory

Interestingly, I can't reproduce this now in calling phpunit directly. Maybe Blackfire injects some observing code that increases memory usage (I can't imagine it's optimized for long-running processes).

I also tried adding Imagick::setResourceLimit( Imagick::RESOURCETYPE_MEMORY, 100 * 1024 ); to WP_UnitTestCase::setUp(), but that didn't seem to have an impact.

TIL the difference between stack and heap memory, and how PHP doesn't have great tooling for measuring the latter.

@danielbachhuber
6 years ago

Replace use of $this->server with rest_get_server() for better memory recycling

#9 @danielbachhuber
6 years ago

  • Keywords commit added
  • Milestone changed from Future Release to 5.0

An anonymous tipster that doesn't want to take credit reported:

The excessive memory consumption looks like it's mostly down to dangling references to Spy_REST_Server due to using $this->server, preventing garbage collection. Removing this var and replacing with a direct call to rest_get_server() gets running phpunit --group restapi locally down from 466.00MB to 102.00MB, and running the whole single site test suite from 502.20MB to 134.20MB

41641.1.diff implements such. I've verified the results locally.

This ticket was mentioned in Slack in #core by danielbachhuber. View the logs.


6 years ago

This ticket was mentioned in Slack in #hosting-community by danielbachhuber. View the logs.


6 years ago

#12 @SergeyBiryukov
6 years ago

  • Owner set to SergeyBiryukov
  • Resolution set to fixed
  • Status changed from new to closed

In 42724:

Tests: Replace use of $this->server with rest_get_server() for better memory recycling.

Props danielbachhuber.
Fixes #41641.

#13 @jorbin
5 years ago

  • Milestone changed from 5.0 to 5.1

#14 @jeremyfelt
5 years ago

In 44234:

Tests: Replace uses of $this->server with rest_get_server().

In [42724], $this->server was replaced with rest_get_server() for better memory recycling.

[43862], from the 5.0 branch, was merged into trunk in [44225] and used the now unavailable $this->server.

This updates the new tests from the 5.0 branch to use the expected rest_get_server().

See #45269, #41641.

#15 @SergeyBiryukov
5 years ago

In 44255:

Tests: Replace use of $this->server with rest_get_server() in test_registered_query_params().

In [42724], $this->server was replaced with rest_get_server() for better memory recycling.

[43897], from the 5.0 branch, was merged into trunk in [44250] and used the now unavailable $this->server.

This updates the new test from the 5.0 branch to use the expected rest_get_server().

See #43316, #41641.

#16 @SergeyBiryukov
5 years ago

In 44256:

Tests: Replace use of $this->server with rest_get_server() in test_get_additional_field_registration_null_schema().

In [42724], $this->server was replaced with rest_get_server() for better memory recycling.

[43908], from the 5.0 branch, was merged into trunk in [44254] and used the now unavailable $this->server.

This updates the new test from the 5.0 branch to use the expected rest_get_server().

See #45220, #41641.

Note: See TracTickets for help on using tickets.