Make WordPress Core

Opened 4 years ago

Last modified 9 months ago

#50568 new defect (bug)

Improve WP_Term's sanitization calls

Reported by: cybr's profile Cybr Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 5.5
Component: Taxonomy Keywords: has-patch needs-unit-tests early
Focuses: performance Cc:

Description

Akin to WP_Post::filter(), I think WP_Term::filter() should have a bypass, too. This will significantly improve performance wherever terms are used (list edit screens, category widgets, blocks) because the bypass prevents redundant re-sanitization.

The attached patch will shave off a 30% load time at wp-admin/edit.php after #50567's implementation. This patch tests for the WP_Term object's ::$filter state, and only re-sanitizes the term when the state differs from the input argument.

You'll also find that sanitize_term() now looks a lot like sanitize_post()--the same goes for get_term()'s resemblance to get_post(). Overall, it seems posts have received a lot more love over the years, and this patch steals some of that.

There are a few issues with terms, however. For example, update_term_cache() caches terms regardless of being shared, while WP_Term::get_instance() tries to prevent that. The $filter = 'display' argument is used in contexts where raw would do fine, too (e.g. @ WP_Terms_List_Table::single_row()). If we iron those issues out, we can fully phase out the re-sanitization of terms.

Attachments (1)

wp-includes.patch (5.1 KB) - added by Cybr 4 years ago.

Download all attachments as: .zip

Change History (11)

@Cybr
4 years ago

#1 @SergeyBiryukov
4 years ago

  • Milestone changed from Awaiting Review to 5.6

This ticket was mentioned in Slack in #core by hellofromtonya. View the logs.


4 years ago

#3 @hellofromTonya
4 years ago

  • Keywords needs-unit-tests early added
  • Milestone changed from 5.6 to 5.7

From today's 5.6 core scrub, Sergey:

This could probably use some unit tests. Both #50567 and this one are nice performance enhancements, but also seem like early tickets, should probably be moved to 5.7.

Moving this ticket to early 5.7 milestone along with #50567.

If any maintainer or committer feels this can be resolved in time, or wishes to assume ownership during a specific cycle, feel free to update the milestone accordingly.

This ticket was mentioned in Slack in #core by hellofromtonya. View the logs.


4 years ago

This ticket was mentioned in Slack in #core by lukecarbis. View the logs.


4 years ago

This ticket was mentioned in Slack in #core by metalandcoffee. View the logs.


4 years ago

This ticket was mentioned in Slack in #core by hellofromtonya. View the logs.


4 years ago

#8 @hellofromTonya
4 years ago

  • Milestone changed from 5.7 to Future Release

This ticket is marked for early in the alpha cycle. We are now in the Beta cycle and long past early. Moving this ticket and #50567 to Future Release. However, would be great to get both committed early in 5.8 alpha cycle if possible.

If any maintainer or committer feels this can be resolved in time, or wishes to assume ownership during a specific cycle, feel free to update the milestone accordingly.

#9 @mukesh27
2 years ago

@Cybr would you mind creating a PR on github.com/wordpress/wordpress-develop? #50567 is already merge so we can target this for next milestone? cc. @peterwilsoncc

#10 @Cybr
9 months ago

Here's a snippet of a bypass I've been using for the past 4 years.
I put in a collective fix-up plugin called "WP Fix: Unified Core Kit," which I might publish here one day.

add_filter(
        'get_term',
        /**
         * Properly primes term cache. Otherwise, terms get resanitized every time they're called.
         *
         * @since 1.0.0
         * @link <https://core.trac.wordpress.org/ticket/50568>
         *
         * @param WP_Term $term     Term object.
         * @param string  $taxonomy The taxonomy slug.
         */
        function( $term, $taxonomy ) {

                $_term = wp_cache_get( $term->term_id, 'terms' );

                if ( $_term && empty( $_term->filter ) ) {
                        $term = sanitize_term( $term, $taxonomy, 'raw' );
                        wp_cache_replace( $term->term_id, $term, 'terms' );
                }

                return $term;
        },
        -100, // Low, for others might add data to the term object, which we don't want to filter out...
        2
);

Saves 2 redundant database requests on my tiny homepage (running WP 6.4.3).

Note: See TracTickets for help on using tickets.