WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 4 years ago

#28410 closed defect (bug) (duplicate)

Walking too many nested comments is inefficient and can break sites

Reported by: dllh Owned by:
Milestone: Priority: normal
Severity: normal Version:
Component: Comments Keywords: needs-patch
Focuses: performance Cc:

Description

Repro:

  1. Create a post with many thousands of hierarchical comments on a single post.
  2. Make sure the option to show nested comments is toggled on.
  3. View the post.

Expected: The post will display in a timely fashion.

Actual: The page takes a long time to load (if it loads) and displays way too many comments. In extreme cases, the site can just time out.

This happens because in comments_template() we select all comments and then walk them to establish parent/child relationships. For a very popular post, this can be catastrophic to the site, especially if/as more and more people pile on to view the site and cause cascading load issues.

Even in cases in which there's only a handful of top-level comments, if there's a lot of nested discussion, the issue manifests.

Potential fixes include the following:

  1. Load just the first 10 (or however many settings specify) un-parented comments for the post and then add some UX that'll fetch child comments in via ajax so that we're only ever fetching the comments we need.
  2. Do some kind of recursive logic to fetch only the top 10, then to fetch any children of those, and then any children of those, ad infinitum. This could get a little ugly and could wind up just shifting the problem by trading many smaller queries for one big, bad one.

I've attached a sample import file with 10k comments on one post and random hierarchical assignments. It demonstrates the issue nicely with nesting set to 5 levels.

Attachments (1)

export-10kcomments-nested.xml.gz (2.4 MB) - added by dllh 5 years ago.
Export file with 1 post, 10k comments, random parentage.

Change History (5)

@dllh
5 years ago

Export file with 1 post, 10k comments, random parentage.

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


4 years ago

#2 @jorbin
4 years ago

  • Keywords needs-patch added

This may be related to #31188. @Stevenkword is going to investigate a bit. We need to figure out if it is the query that is slow, and if so how to speed it up, or if it is something else.

#3 @stevenkword
4 years ago

In this case, the delay is being caused by the walker. The query itself is only taking about 300ms in my vagrant.

Steps to reproduce:

  1. Import export-10kcomments-nested.xml.gz using the importer
  2. Comment out $output = $walker->paged_walk( $_comments, $r['max_depth'], $r['page'], $r['per_page'], $r ); in the function wp_list_comments() found in /wp-includes/comment-template.php
  3. The page should load in in under a second and the $_comments array is populated by the query results.

We need to take a better look at the walker methods to resolve the reported issue.

Last edited 4 years ago by stevenkword (previous) (diff)

#4 @wonderboymusic
4 years ago

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

I have a patch on #8071 that does what is described in this ticket and will focus the efforts there

Note: See TracTickets for help on using tickets.