WordPress.org

Make WordPress Core

Opened 3 weeks ago

Last modified 3 weeks ago

#50679 new enhancement

Optimization for wp_get_attachment_metadata()

Reported by: Tkama Owned by:
Milestone: 5.6 Priority: normal
Severity: normal Version: 5.4.2
Component: Media Keywords: has-patch has-unit-tests
Focuses: performance Cc:

Description

Link: https://developer.wordpress.org/reference/functions/wp_get_attachment_metadata/#source

function wp_get_attachment_metadata( $attachment_id = 0, $unfiltered = false ) {
        $attachment_id = (int) $attachment_id;
        $post          = get_post( $attachment_id );
        if ( ! $post ) {
                return false;
        }

        $data = get_post_meta( $post->ID, '_wp_attachment_metadata', true );

        if ( $unfiltered ) {
                return $data;
        }

        /**
         * Filters the attachment meta data.
         *
         * @since 2.1.0
         *
         * @param array|bool $data          Array of meta data for the given attachment, or false
         *                                  if the object does not exist.
         * @param int        $attachment_id Attachment post ID.
         */
        return apply_filters( 'wp_get_attachment_metadata', $data, $post->ID );
}

What this code for?

        $attachment_id = (int) $attachment_id;
        $post          = get_post( $attachment_id );
        if ( ! $post ) {
                return false;
        }

Why we can't just pass $attachment_id to first parameter of get_post_meta() and check result?

This no-needed code makes the function a little slower without any advantages.

Change History (5)

#1 @SergeyBiryukov
3 weeks ago

  • Component changed from General to Media

#2 @SergeyBiryukov
3 weeks ago

  • Focuses performance added

Hi there, thanks for the ticket!

This acts as a quick check to verify that the post actually exists before proceeding further to get_metadata(), update_meta_cache(), etc.

Some performance benchmarks for various cases (existing attachment ID, non-existing ID, invalid ID) would be helpful here to show if removing that code makes any difference.

#3 @Tkama
3 weeks ago

<?php
// attachment invalid ID
$attachment_id = 'aa1342'; 
$post = get_post( $attachment_id = (int) $attachment_id );                // 0.0000001 sec.
$data = get_post_meta( $attachment_id, '_wp_attachment_metadata', true ); // 0.0000001 sec.

for( $i=1; $i<50000; $i++ ){
    $post = get_post( $attachment_id = (int) $attachment_id );                // 0.010389 sec.          
    $data = get_post_meta( $attachment_id, '_wp_attachment_metadata', true ); // 0.005079 sec.
}

// this attachment exists
$attachment_id = 13423; 
clean_post_cache( $attachment_id );
$post = get_post( $attachment_id = (int) $attachment_id );                // 0.000927 sec.
$data = get_post_meta( $attachment_id, '_wp_attachment_metadata', true ); // 0.001256 sec.

for( $i=1; $i<50000; $i++ ){
    $post = get_post( $attachment_id = (int) $attachment_id );                // 0.126539 sec.
    $data = get_post_meta( $attachment_id, '_wp_attachment_metadata', true ); // 0.210088 sec.
}


// attachment NOT exists
$attachment_id = 13423333; 
clean_post_cache( $attachment_id );
$post = get_post( $attachment_id = (int) $attachment_id );                // 0.000516 sec.
$data = get_post_meta( $attachment_id, '_wp_attachment_metadata', true ); // 0.000432 sec. 

for( $i=1; $i<50000; $i++ ){
    $post = get_post( $attachment_id = (int) $attachment_id );                // 10.004103 sec.
    $data = get_post_meta( $attachment_id, '_wp_attachment_metadata', true ); // 0.199436 sec.
}

NOTE about code: each line of code with benchmark was run separately.

My conclusion: we no need any additional checks to run get_post_meta() — code above it only slow down the code. We even can't pass WP_Post object into the wp_get_attachment_metadata() to increase the function speed. For example, I need to call wp_get_attachment_metadata() more then 1200 times per site page generation and I have no choice except use get_post_meta( $post->ID, '_wp_attachment_metadata', true ) directly.

P.S. I was shocked when seen get_post() for not existence attachment - 10.004103 sec. fro 50k iterations.

#4 @SergeyBiryukov
3 weeks ago

  • Milestone changed from Awaiting Review to 5.6

Thanks for the benchmarks, this looks great!

This ticket was mentioned in PR #417 on WordPress/wordpress-develop by donmhico.


3 weeks ago

  • Keywords has-patch has-unit-tests added

Optimize wp_get_attachment_metadata () by removing the get_post() logic and pass $attachment_id directly to get_post_meta(). See benchmarks posted here - https://core.trac.wordpress.org/ticket/50679#comment:3

Props to Tkama for creating the ticket and providing the benchmarks. Great work!

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

Note: See TracTickets for help on using tickets.