Improving the perfomance of a plugin action

Question

I want to override a 3rd party plugin action with a new action, but in comments to my another question @pieter-goosen suggested that this plugin action is very very expensive, in terms of using server resources. So, my question is how to improve this plugin action to make it less expensive?

The only thing that I think will improve this plugin action is to add a new query argument - 'fields' => 'ids', that will improve its performance. Are here other things to do for this?

add_action( 'adverts_event_expire_ads', 'adverts_event_expire_ads' );

/**
 * Expires ads
 * 
 * Function finds Adverts that already expired (value in _expiration_date
 * meta field is lower then current timestamp) and changes their status to 'expired'.
 * 
 * @since 0.1
 * @return void
 */
function adverts_event_expire_ads() {

    // find adverts with status 'publish' which exceeded expiration date
    // (_expiration_date is a timestamp)
    $posts = new WP_Query( array(
        "fields" => "ids", //added for performance
        "post_type" => "advert",
        "post_status" => "publish",
        "meta_query" => array(
            array(
                "key" => "_expiration_date",
                "value" => current_time( 'timestamp' ),
                "compare" => "<="
            )
        )
    ) );


    if( $posts->post_count ) {
        foreach($posts->posts as $post) {
            // change post status to expired.
            $update = wp_update_post( array( 
                "ID" => $post->ID,
                "post_status" => "expired"
            ) );
        } // endforeach
    } // endif

}

UPDATE

I updated for performance (I hope) and overrode the code, so it now not just 'expire' posts, but also inform post authors about this, and it works (tested). But the question is the same: can it be improved/optimised for performance?

remove_action( 'adverts_event_expire_ads', 'adverts_event_expire_ads' );
add_action( 'adverts_event_expire_ads', 'my_adverts_event_expire_ads' );

/**
 * Expires ads
 * 
 * Function finds Adverts that already expired (value in _expiration_date
 * meta field is lower then current timestamp) and changes their status to 'expired'.
 * 
 * @since 0.1
 * @return void
 */

function my_adverts_event_expire_ads() {
    // Set our query arguments
    $args = [
        'fields'      => 'ids',
        'post_type'   => 'advert',
        'post_status' => 'publish',
        'meta_query'  => [
            [
                 'key'     => '_expiration_date',
                 'value'   => current_time( 'timestamp' ),
                 'compare' => '<='
             ]
         ]
    ];

    // get adverts with status 'publish' which exceeded expiration date
    // (_expiration_date is a timestamp)
    $posts = get_posts( $args );

    // Check if we have adverts with exceeded expiration date, if not, return false
    if ( !$posts )
        return false;

    // If we have posts with exceeded expiration time, lets change their status to expired
    foreach($posts as $id) {
        // change post status to expired.
        $update = wp_update_post( array( 
            "ID" => $id,
            "post_status" => "expired"
        ) );

        // Send an email about expiration to the post author
        $post = get_post($id);
        $author = get_userdata($post->post_author);
        $author_name = get_post_meta( $id, 'adverts_person', true );
        $message = 
            "Hi " . $author_name . ", " . 
            "\n\nYour advert, \"" . $post->post_title . "\", expired and will be deleted in 7 days." . 
            "\n\nBest regards, \n\"" . get_bloginfo() . "\" team";
        wp_mail($author->user_email, "Your article expired!", $message);

    } // endforeach
}

Answer 1

Your original piece of code (the code from the previous question) did need some performance boost in the context it was given. As for the updated code, you will need to remove the fields parameter as you need to get the full post object and not just the ID.

To optimize code in queries, you will need to know before hand what you will need from the post. I will use your two examples above to explain.

Before I do, quickly do the following: (This will really help in understanding what I'm trying to say)

• Create a page template, and use the template to create a new page and add the following lines in the template

  timer_start();
  
  // We will add our queries here
  
  echo get_num_queries(); . ' queries in ' . timer_stop(1, 5); . ' seconds.';
  

• Load the page and note the amount of queries. You will need to subtract this number from all your queries that you will run between the timer

• Once you have a benchmark, add the following two queries separately in place of // We will add our queries here and note the difference in time taken to run the queries. (This is a great way to test queries and how effecient they are)

  • 1.)

    $q = get_posts( 'post_type=advert&posts_per_page=-1' );
    var_dump( $q );
    

  • 2.)

    $q = get_posts( 'post_type=advert&posts_per_page=-1&fields=ids' );
    var_dump( $q );
    

Just a another note, do not use global variables ($posts and $post) as local variables, this breaks the global variables and give unexpected output. Such cases are really hard to debug. Use local variables that are unique and outside of the global variable scope. The only global that you will have to use as a local variable is $post when you set up postdata as setup_postdata( $post ) expects the $post global. Just remeber to reset it when you are done with wp_reset_postdata().

CODE BLOCK 1

In the context of the first block of code, all you need from the query are post ID's. The simple reason is that wp_update_post() only requires the post ID. If you look at the source code, wp_update_post() makes its own call to get the full post object to merge with the array passed to it.

What your code does is, it retrieves the full post object, you get the ID from that, pass that to wp_update_post() and then wp_update_post() again makes a call to get the full post object from the ID. This is expensive and unnecessary, it is like eating the same piece of meat twice.

You have to remember, even though posts are cached into a cache, it still takes time to get the relevant data, and the more data to get, the more time it takes. This is where 'fields'=>'ids' comes in handy as it will retrieve an array of post ID's, and not an array of post objects (Check the test data I have asked you to test). Everything still works as normal, only the loop will be slightly different because your posts are returned as ID's and not objects

So code block one can look something like this

add_action( 'adverts_event_expire_ads', 'adverts_event_expire_ads' );

/**
 * Expires ads
 * 
 * Function finds Adverts that already expired (value in _expiration_date
 * meta field is lower then current timestamp) and changes their status to 'expired'.
 * 
 * @since 0.1
 * @return void
 */
function adverts_event_expire_ads() {

    // find adverts with status 'publish' which exceeded expiration date
    // (_expiration_date is a timestamp)
    $q = new WP_Query( array(
        "fields" => "ids", //added for performance
        "post_type" => "advert",
        "post_status" => "publish",
        "meta_query" => array(
            array(
                "key" => "_expiration_date",
                "value" => current_time( 'timestamp' ),
                "compare" => "<="
            )
        )
    ) );


    if( $q->post_count ) {
        foreach($q->posts as $post_id) {
            // NOTE: $q->posts are an array of post ID's, so $post_id is a post ID
            // change post status to expired.
            $update = wp_update_post( array( 
                "ID" => $post_id,
                "post_status" => "expired"
            ) );
        } // endforeach
    } // endif

}

CODE BLOCK 2

In context, you can remove 'fields'=>'ids' here. Two reasons

• You need info like post author and post title from the post, not just the post ID. In a case where you need post info other that the ID, you have no choice but to get the full post object. You can maybe try the posts_fields filter to only return certain fields, but on testing, this really does not make any noticable difference. Also note, this filter does not wotk on get_posts() by default

• You are using get_post() to get the post data from the post ID. This does not create extra db queries if a post is in the post cache, but it does take extra time to get the post object. This is all unnecessary. It is really better to get the full post object from the start

So you can rewrite your function in code block 2 as follow:

remove_action( 'adverts_event_expire_ads', 'adverts_event_expire_ads' );
add_action( 'adverts_event_expire_ads', 'my_adverts_event_expire_ads' );

/**
 * Expires ads
 * 
 * Function finds Adverts that already expired (value in _expiration_date
 * meta field is lower then current timestamp) and changes their status to 'expired'.
 * 
 * @since 0.1
 * @return void
 */

function my_adverts_event_expire_ads() {
    // Set our query arguments
    $args = [
        'post_type'   => 'advert',
        'post_status' => 'publish',
        'meta_query'  => [
            [
                 'key'     => '_expiration_date',
                 'value'   => current_time( 'timestamp' ),
                 'compare' => '<='
             ]
         ]
    ];

    // get adverts with status 'publish' which exceeded expiration date
    // (_expiration_date is a timestamp)
    $q = get_posts( $args );

    // Check if we have adverts with exceeded expiration date, if not, return false
    if ( !$q )
        return;

    // If we have posts with exceeded expiration time, lets change their status to expired
    foreach($q as $post_object) {
        // $post_object is now an object of post data
        // change post status to expired.
        $update = wp_update_post( array( 
            "ID" => $post_object->ID,
            "post_status" => "expired"
        ) );

        // Send an email about expiration to the post author
        $author = get_userdata($post_object->post_author);
        $author_name = get_post_meta( $post_object->ID, 'adverts_person', true );
        $message = 
            "Hi " . $author_name . ", " . 
            "\n\nYour advert, \"" . $post_object->post_title . "\", expired and will be deleted in 7 days." . 
            "\n\nBest regards, \n\"" . get_bloginfo() . "\" team";
        wp_mail($author->user_email, "Your article expired!", $message);

    } // endforeach
}

EXTRA NOTES:

• As I have stated, before you can optimize a query, you need to know what do you need from the query.

• Before you use a function, make sure you know how it works internally. As example, get_category_link() accepts the the category ID or category object as parameter. Using the wrong one of the two have huge impact on performace, specially if you have a lot of categories. When you pass the ID, the function makes a db call to get the category object to build the link. If you pass the object, the object is used as is to build the links, so no db call is done.

  developer.wordpress.org is a great way to look up functions and its source code, so bookmark this address and abuse it

• Make use of the cache parameters in WP_Query as well to improve performance to turn of the caching of term and custom field data if you do not need to cache these.

• Make use of the little testing script I have given you to test queries and custom code. This way you will know how your code will perform. Also, download and install a plugins like Query Monitor and Debug Objects on your local test install. This will break queries down so you can inspect each query and check how suffecient it really is. It also helps with debugging. I personally think everyone should have these two pluins installed on their test installation