Make WordPress Core

Opened 5 years ago

Last modified 10 days ago

#51817 new enhancement

WP-Admin - Optimize main wpquery with 'fields' => 'ids' causes notice

Reported by: eumene's profile eumene Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 5.5.3
Component: Administration Keywords: has-patch reporter-feedback
Focuses: administration, performance Cc:

Description

Hi all

I have a post_type (Woocommerce product) with a lot of data in the post contents. So, in wp-admin, WP requires a lot of memory to list them (sometimes I got an OUT-OF-MEMORY)

So, I tried to make a lighter query adding via hooks the query params
'fields' => 'ids'

It works! The query is more fast and WP does not require so many memory
but I got some annoying notices

In method _display_rows in WP_Posts_List_Table you have

foreach ( $posts as $a_post ) {
 $post_ids[] = $a_post->ID;
}

The method supposes that $posts will be an array of WP_Post object .
In my case it is an array of integer

Can you handle this scenario?

Change History (3)

This ticket was mentioned in PR #1182 on WordPress/wordpress-develop by eumene.


4 years ago
#1

  • Keywords has-patch added

Data optimization in wp-admin / edit page / WP_Posts_List_Table data

https://core.trac.wordpress.org/ticket/51817

In wp-admin/edit.php the main wp_query retrives all data and it requires a lot of memory

Then, WP_Posts_List_Table calls single_row method for each post in wp_query result set
In this method, the table calls
$post = get_post( $post );
to get al post data, already returned by the wp_query

So, I suggest to limit wp_query to post ID and parent ID for hierarchical post_type.
This solution:

  • optimize memory usage
  • speed up query time

This change adds one issue in WP_Posts_List_Table::_display_rows.
I added also a fix here to keep backward compatibility

Trac ticket: https://core.trac.wordpress.org/ticket/51817

#2 follow-up: @flixos90
10 days ago

  • Keywords reporter-feedback added

@eumene Thank you for the report, and apologies that nobody looked at it before.

At first glance, your proposal seems like a reasonable workaround, allowing for $a_post to alternatively be a number. But then how does it work in the foreach loop after where it renders the rows? For that it would need the full object anyways to render the post information, right?

#3 in reply to: ↑ 2 @eumene
10 days ago

Well...
This is the current _display_rows methdo

<?php
private function _display_rows( $posts, $level = 0 ) {
                $post_type = $this->screen->post_type;

                // Create array of post IDs.
                $post_ids = array();

                foreach ( $posts as $a_post ) {
                        $post_ids[] = $a_post->ID;
                }

                if ( post_type_supports( $post_type, 'comments' ) ) {
                        $this->comment_pending_count = get_pending_comments_num( $post_ids );
                }
                update_post_author_caches( $posts );

                foreach ( $posts as $post ) {
                        $this->single_row( $post, $level );
                }
        }

foreach post retrived it will call single_row method, which is

<?php

        /**
         * @global WP_Post $post Global post object.
         *
         * @param int|WP_Post $post
         * @param int         $level
         */
        public function single_row( $post, $level = 0 ) {
                $global_post = get_post();

                $post                = get_post( $post );
                $this->current_level = $level;

                $GLOBALS['post'] = $post;
                setup_postdata( $post );

                $classes = 'iedit author-' . ( get_current_user_id() === (int) $post->post_author ? 'self' : 'other' );

                $lock_holder = wp_check_post_lock( $post->ID );

                if ( $lock_holder ) {
                        $classes .= ' wp-locked';
                }

                if ( $post->post_parent ) {
                        $count    = count( get_post_ancestors( $post->ID ) );
                        $classes .= ' level-' . $count;
                } else {
                        $classes .= ' level-0';
                }
                ?>
                <tr id="post-<?php echo $post->ID; ?>" class="<?php echo implode( ' ', get_post_class( $classes, $post->ID ) ); ?>">
                        <?php $this->single_row_columns( $post ); ?>
                </tr>
                <?php
                $GLOBALS['post'] = $global_post;
        }

it contains the line

$post = get_post( $post );

it means that for each post (it could be just the ID) the full object will be retrived to fill the table.

The point is that I do not want load all post data twice:

  • first time in the WP_Posts_List_Table main query (if I have huge post_content, this query will be able to create a memory issue)
  • second time for each single_row call

I hope it could be more clear

Diego


Note: See TracTickets for help on using tickets.