Optimization for wp_get_attachment_metadata()

[Tkama]

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.

[SergeyBiryukov]

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.

[Tkama]

<?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.

[SergeyBiryukov]

Thanks for the benchmarks, this looks great!

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

[helen]

In 49084:

Media: Don't unnecessarily check for a valid attachment before getting meta.

This makes wp_get_attachment_metadata() run significantly faster. See ticket for benchmarking.

Props Tkama, donmhico.
Fixes #50679.

Committed in ​https://github.com/WordPress/wordpress-develop/commit/150d9bd17e99857cf24fa6f0139b2e76ba6761ad. Thanks for the patch!

[helen]

In 49085:

Tests: Fix a linting error in a test.

See #50679.

[SergeyBiryukov]

Follow-up: #52196, #52389.