Support » Plugin: Phoenix Media Rename » r2468595 introduced an extra MySQL query on every request

  • Resolved Rinat Khaziev

    (@rinatkhaziev)


    Hi,

    Thanks for the plugin, works great!

    I just noticed that this commit introduces a new table and the migration logic that meant to create the table is called from the class constructor, effectively resulting in an extra SQL query, which may have a negative impact on performance.

    The quickest/cheapest fix is probably to add an option (pmr_is_table_created) and check for that value before trying to execute a potential write query.

    Cheers

    https://plugins.trac.wordpress.org/changeset/2468595

Viewing 5 replies - 1 through 5 (of 5 total)
  • Plugin Author crossi72

    (@crossi72)

    Hi @rinatkhaziev,
    thank you for your suggestion, but the SQL statement used to create the table contains a ‘if not exists’ condition, so the cost of this query is no different from that of the query needed to check an option.

    The table creation query is called in the class to better support multisite installations, but if you have suggestion (or updates on multisite behavior, giving the fact that I don’t use this kind of installations) I will be glad to use your help.

    Thank you for your analysis
    C.

    Thread Starter Rinat Khaziev

    (@rinatkhaziev)

    Thank you for the swift reply!

    but the SQL statement used to create the table contains a ‘if not exists’ condition, so the cost of this query is no different from that of the query needed to check an option.

    Not exactly, options are cached in object cache, CREATE TABLE IF NOT EXISTS would always result in a query. What that means in practice: say I have 1000 requests, the current approach would result in 1000 queries, get_option would result in just a few (if the object cache is enabled – and it likely is)

    One might think it’s not a big deal to run an extra query, but it compounds and there’s literally no good reason to do so on each frontend request – PMR functionality is all Admin-based. Imagine you have 20 plugins active and each of them would try to do the same thing.

    The table creation query is called in the class to better support multisite installations, but if you have suggestion (or updates on multisite behavior, giving the fact that I don’t use this kind of installations) I will be glad to use your help.

    Sure, what I would do (and done so in similar cases) is put that logic in an activation callback to iterate over subsites and create the table, and add admin_init hook that checks the option and if it’s not present run the create_db_table:

    // Cover the new installs
    register_activation_hook( __FILE__, function() {
    // is_multisite() check is important here because get_sites() is not available on single site installs.
    if ( is_multisite() ) {
    foreach ( get_sites() as $subsite ) {
    switch_to_blog( $subsite->blog_id );
    $pmr->create_db_table();
    restore_current_blog();
    update_option( ‘pmr_table_installed’, true );
    }
    } else {
    $pmr->create_db_table();
    update_option( ‘pmr_table_installed’, true );
    }
    } );

    // Cover plugin upgrades
    add_action( ‘admin_init’, function() {
    if ( get_option( ‘pmr_table_installed’ ) ) {
    return;
    }

    $pmr->create_db_table();
    update_option( ‘pmr_table_installed’, true );
    } );`

    Some further reading:
    https://developer.wordpress.org/reference/functions/register_activation_hook/
    https://developer.wordpress.org/reference/functions/get_sites/

    Plugin Author crossi72

    (@crossi72)

    Many thank for the code snippet and the complete explanation!

    I’ll check the code and integrate it into the next version, actually I’m working on a new feature to rename image from associated post, I hope to release it in a week, if possibile (my spare time is limited) with your code already integrated.

    C.

    Plugin Author crossi72

    (@crossi72)

    Hi @rinatkhaziev,
    I just released version 3.1.0 that implement the logic you proposed, feel free to suggest other improvement (or correction).

    Many thanks
    C.

    Thread Starter Rinat Khaziev

    (@rinatkhaziev)

    Thank you so much! I marked this as resolved.

Viewing 5 replies - 1 through 5 (of 5 total)
  • You must be logged in to reply to this topic.