[wp-trac] [WordPress Trac] #34560: High memory usage ( and possible server error) editing post with many/large revisions

[WordPress Trac]

#34560: High memory usage ( and possible server error) editing post with many/large
revisions
-------------------------------------------------+-------------------------
 Reporter:  pdfernhout                           |       Owner:
     Type:  defect (bug)                         |  adamsilverstein
 Priority:  normal                               |      Status:  assigned
Component:  Revisions                            |   Milestone:  Future
 Severity:  major                                |  Release
 Keywords:  needs-unit-tests has-patch dev-      |     Version:  3.6
  feedback needs-testing                         |  Resolution:
                                                 |     Focuses:
                                                 |  administration
-------------------------------------------------+-------------------------

Comment (by pdfernhout):

 Replying to [comment:27 adamsilverstein]:
 > in [attachment:34560.5.diff] :
 > * cleanup patch
 >
 > memory use now down to ~6.3MB (!!!), the revisions are no longer causing
 high memory consumption on the post edit page.

 @adamsilverstein Thanks for the great patch, and @miqrogroove thanks for
 helping with it too.

 I am testing it now with the production server with the issue. Previously,
 the editor failed to open a ~437K page due to a server error from
 exceeding 256MB. Now the editor page opens, and memory use according to
 WP-Memory-Usage is 27.96MB of 256MB. So, huge improvement! As a baseline,
 23.68MB of 256MB is used in the admin plugin panel on that server, and
 26.55MB of 256MB is used by the editor when creating a new page. So, only
 about 1.21MB is being used by the editor above the new page baseline for
 the really big page. Wow!!!

 In all other ways I spot-tested after applying the patch, WordPress seems
 to be running OK. I'll probably git stash that patch soon though, just in
 case. I still have not tried saving a post on that server, and I have not
 tried anything related to autosaves. Those would be better tested on a
 test setup in case data gets munged somehow (which I think highly unlikely
 given the code changes).

 So, it seems that WordPress was indeed loading all the revisions of a post
 *three* times when editing it with the edit_form_advanced: once to count
 the revisions, once to list the timestamps and other metadata of the
 revisions, and once to check for the latest autosaved revision (if any).

 As far as the issue I originally reported, this patch seems to totally fix
 it (subject to further testing). Thanks.

 It would not surprise me if there were other problematical uses of
 "wp_get_post_revisions", but
 [https://github.com/WordPress/WordPress/search?q=wp_get_post_revisions
 code review for that] could perhaps become a new issue.

 One remaining concern is the one in comment 17 referencing issue #25233
 (the second concern under "Possible bottlenecks") about WordPress needing
 to load all revisions to upgrade them. Does that concern apply here? I do
 not see such code in the
 [https://github.com/WordPress/WordPress/blob/4.3-branch/wp-
 includes/revision.php 4.3 version of that function]. There is a separate
 "_wp_upgrade_revisions_of_post" function in that file. It seems to
 [https://github.com/WordPress/WordPress/search?q=_wp_upgrade_revisions_of_post
 be called] separately when the edit process is started via edit_post (code
 below). Based on inspecting the code for WordPress 4.3 on GitHub, I do not
 think that concern does not apply here far as the patch itself goes -- but
 someone else should review that just in case to ensure there is no risk of
 data corruption.

 See this code in [https://github.com/WordPress/WordPress/blob/4.3-branch
 /wp-admin/includes/post.php includes/post.php] to understand what the
 concern may be:

 {{{
 function edit_post( $post_data = null ) {
         ...
         if ( post_type_supports( $ptype->name, 'revisions' ) ) {
                 $revisions = wp_get_post_revisions( $post_ID, array(
 'order' => 'ASC', 'posts_per_page' => 1 ) );
                 $revision = current( $revisions );
                 // Check if the revisions have been upgraded
                 if ( $revisions && _wp_get_post_revision_version(
 $revision ) < 1 )
                         _wp_upgrade_revisions_of_post( $post,
 wp_get_post_revisions( $post_ID ) );
         }
 }}}

 That fragment of code seems to call "wp_get_post_revisions" twice, btw.
 People seem to like calling that function a lot. LOL. :-)

 Still, what are the differences between the edit_post function in post.php
 and the code in edit_form_advanced.php? Perhaps there is some other bug
 (or maybe two bugs, one on memory in edit_post which calls
 wp_get_post_revisions when it may not need to, and one
 on_wp_upgrade_revisions_of_post not being called in edit-form-advanced.php
 ) which could become new issues issue related to differences between those
 two functions regarding upgrading revisions?

--
Ticket URL: <https://core.trac.wordpress.org/ticket/34560#comment:30>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform