Make WordPress Core

Opened 9 years ago

Last modified 4 years ago

#32366 reopened defect (bug)

wp_count_comments() is hardcoded into /wp-admin/menu.php and queries all comments every single page load within the admin area, regardless of if edt_posts is removed from top menu or not

Reported by: justindocanto's profile justindocanto Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 4.2
Component: Comments Keywords: has-patch close
Focuses: administration, performance Cc:

Description

wp_count_comments() is hardcoded into /wp-admin/menu.php and queries all comments every single page load within the admin area, regardless of if edt_posts is removed from top menu or not.

This is reproducible with 0 plugins and any core theme, as long as you have a large number of comments.

Currently working on a site with 5,000,000+ comments and unless we comment out the following code in /wp-admin/menu.php, the server gets hammered. With multiple people in the admin area at the same time, it's even worse, since it loads on every single page load.

The code: /wp-admin/menu.php - Line 94-97 (4.2.2)

$awaiting_mod = wp_count_comments();
$awaiting_mod = $awaiting_mod->moderated;
$menu[25] = array( sprintf( __('Comments %s'), "<span class='awaiting-mod count-$awaiting_mod'><span class='pending-count'>" . number_format_i18n($awaiting_mod) . "</span></span>" ), 'edit_posts', 'edit-comments.php', '', 'menu-top menu-icon-comments', 'menu-comments', 'dashicons-admin-comments' );
unset($awaiting_mod);

it's wp_count_comments that causes the large queries, when you have a lot of comments, but I comment out his entire section so that there isnt any errors with removing the creation of $awaiting_mod since it's being called on the next couple lines, before it is unset.

Before I found the source, and saw that it was hard coded, I was hoping for some sort of hook to disable this entire function and/or a way for the wp_count_comments() to not be called if the item was removed... but thats not currently possible.

I'm new to working on core, so i'm not sure on the best way to attack this issue. I figured it was best to bring up the issue and learn how you guys tackle the code. Open to coding a solution myself if somebody can point me in the right direction. Thanks.

Attachments (1)

32366.diff (1.1 KB) - added by chacha102 8 years ago.
Introduce 'disable_admin_menu_comment_count' filter

Download all attachments as: .zip

Change History (20)

#1 @justindocanto
9 years ago

Just realize 'hardcoded' was a poor choice of words, but i think the point is there. wp_count_comments() is called with no way to disable it. I'm new to helping with core, sorry. :)

#2 follow-up: @chriscct7
9 years ago

  • Keywords needs-patch dev-feedback added
  • Version changed from 4.2.2 to 4.2

There are a couple of options here that I've thought of thus far:

  • Disable counts for comments once it gets above a certain number (solves the problem but loses ui).
  • Try to make the comment count query more efficient (unlikely to make that much of an improvement though).
  • Provide a filter to turn off the counts
  • We could store the counts in options, and when a comment is added or removed increment or decrement the options and then pull the numbers from the options instead of querying the comments table

#3 @chriscct7
9 years ago

Perhaps we should split off comment:2 into a separate ticket. Provide the ability to turn off the comments here, and then work on improving the comment count function in a separate ticket?

#4 follow-up: @johnbillion
9 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to duplicate
  • Status changed from new to closed

Thanks for the report justindocanto. This is a duplicate of an existing ticket: #19372

#5 in reply to: ↑ 2 @justindocanto
9 years ago

Replying to chriscct7:

  • We could store the counts in options, and when a comment is added or removed increment or decrement the options and then pull the numbers from the options instead of querying the comments table

This reminds me of how wp_term_taxonomy has counts of the total amount of posts for each term. Calling 1 option from the wp_options table to get the total comment count would be much less resource usage than counting all the comments every single time. We'd have to build in logic in the add/delete comment functions (for functions used in the front-end and back-end) to get an accurate count, but it would do the job.

We could also use this count on the 'Right Now' widget on the dashboard, since it shows a total amount of comments there. I've had it disabled for so long, i forgot about it.

#6 follow-up: @johnbillion
9 years ago

Also note that caching the comment count can actually decrease performance on sites which get a very high number of comments, because the cache value will constantly get flushed. This was mentioned in another ticket which I can't immediately find.

#7 in reply to: ↑ 4 @justindocanto
9 years ago

Replying to johnbillion:

Thanks for the report justindocanto. This is a duplicate of an existing ticket: #19372

This is not the same issue. The 'edit_posts' solution does not solve the problem mentioned in this ticket.

#8 in reply to: ↑ 6 @justindocanto
9 years ago

Replying to johnbillion:

Also note that caching the comment count can actually decrease performance on sites which get a very high number of comments, because the cache value will constantly get flushed. This was mentioned in another ticket which I can't immediately find.

Bummer. Good to know.

I'm wondering if there's a way we can stop the menu item from being created and the query from ever happening via some sort of filter or option. Everything else has some sort of way to stop the querying of the entire comments table.

The total comment count in the 'Right Now' dashboard widget can be disabled and the query prevented by unsetting the 'right now' meta box using the 'wp_dashboard_setup' action.

The pending comment count column can be disabled and the query prevented on the /wp-admin/edit.php screen by unsetting the 'comments' column in the 'manage_posts_columns' filter

As mentioned before, the 'edit_posts' solution on #19372, for people who dont need to see it, does not allow for the disabling of this query entirely, which is needed for larger sites.

Last edited 9 years ago by justindocanto (previous) (diff)

#9 @boonebgorges
8 years ago

  • Keywords dev-feedback removed
  • Milestone set to Future Release
  • Resolution duplicate deleted
  • Status changed from closed to reopened

@chacha102
8 years ago

Introduce 'disable_admin_menu_comment_count' filter

#10 @chacha102
8 years ago

  • Keywords has-patch added; needs-patch removed

Given that it is in the base of a file that you can't stop from loading, I think a filter is going to be the best approach. Plus, the menu item is still accessible for administrators. Just the comment count is removed.

This does not, however, do anything to allow you to temporarily cache the value or anything else. Just remove the comment count.

#11 @rachelbaker
8 years ago

Related #35060, which adds a filter to bail out of updating the comment count.

#12 @rachelbaker
8 years ago

  • Keywords close added

With the introduction of the wp_count_comments in #35060 you can skip the comment count query from running. I don't think we need another filter.

To skip the comment counts for all posts (including the admin_bar and menu), a function like this should do the trick:

<?php
add_filter( 'wp_count_comments', 'skip_comment_count_query', 10, 2 );
function skip_comment_count_query( $count, $post_id ) {
        if ( 0 === $post_id ) {
                $stats = array(
                        'approved'       => 0,
                        'moderated'      => 0,
                        'spam'           => 0,
                        'trash'          => 0,
                        'post-trashed'   => 0,
                        'total_comments' => 0,
                        'all'            => 0,
                );

                return (object) $stats;
        }
}

#13 @rachelbaker
8 years ago

  • Milestone Future Release deleted
  • Resolution set to wontfix
  • Status changed from reopened to closed

#14 @lkraav
5 years ago

  • Resolution wontfix deleted
  • Status changed from closed to reopened

With the introduction of the wp_count_comments in #35060 you can skip the comment count query from running. I don't think we need another filter.

@rachelbaker above conclusion seems to be a solid fit only for simple scenarios.

When multiple plugins add_filter wp_count_comments, each with their own logic, possibly making heavy queries, situation becomes significantly more complex.

It is not possible to safely unhook this filter from everyone, because in-depth knowledge about why things are done the way they are may not be(come) available.

Because comments model is being used for a variety of non-UI utilities (Action Scheduler, Sensei LMS), it is not reasonable anymore (probably for a long time already) to assume menu.php firing off wp_count_comments() is safe and performant everywhere.

I vote to reopen and rearchitect "Comments" admin menu item display, because the performance hit can be very significant (admin page load times increase by a factor of 10+, depending on comment db size), for trivial gains (menu item badge).

Example: https://github.com/Automattic/sensei/blob/version/1.12.2/includes/class-sensei.php#L894

Version 0, edited 5 years ago by lkraav (next)

#15 @iCaleb
5 years ago

When multiple plugins add_filter wp_count_comments, each with their own logic, possibly making heavy queries, situation becomes significantly more complex.

This is key to the problem here. I'm looking at a site right now where there are filters on wp_count_comments coming from WooCommerce, WC Memberships, and Action Scheduler as they need to remove their own comment types from being returned. Each of these queries becoming significantly heavier as the site size increases. What's perhaps even worse is that the site has comments completely disabled, and doesn't even use them in the traditional post-reply sense.

The filter inside of wp_count_comments is simply not sufficient because it doesn't grant enough context. There are two places in the admin where comments are counted on every page:

The later can be removed by unhooking it from admin_bar_menu. And a bonus location on the dashboard where you can also remove the wp_count_comments() call by unsetting the "At a Glance" widget.


While I would still love a way to unhook the call from menu.php, it might be worth also taking a step back and see what the underlining issue is here.

The three plugins I mentioned are all doing the same thing on the wp_count_comments filter. They re-query and remove their custom comment_type's from the response. So maybe the better answer is an optional, filter-powered query clause in get_comment_count() where plugins can pass a list of comment_types that need to be ignored? It will slow the query down, but it's 1000x better than having to query the whole thing again to remove just one type at a time.

#16 @SergeyBiryukov
5 years ago

  • Milestone set to Future Release

#17 follow-up: @lkraav
5 years ago

Turns out this might be a duplicate of #19901?

#18 in reply to: ↑ 17 @justindocanto
5 years ago

Replying to lkraav:

Turns out this might be a duplicate of #19901?

#19901 is suggesting a speed up while this ticket (#32366) is suggesting & working on a way to bypass/disable/filter the function entirely. Similar reasons for both, but 2 different end results being worked on. I would love to see it sped up AND a way to disable it.

Note: See TracTickets for help on using tickets.