WordPress.org

Make WordPress Core

Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#7147 closed defect (bug) (fixed)

Problems with 700+ categories

Reported by: bi0xid Owned by:
Milestone: 2.6 Priority: normal
Severity: normal Version: 2.5.1
Component: Optimization Keywords:
Focuses: Cc:

Description

When having a large amount of categories in WordPress, it lasts *minutes* to show a page (any page, includin admin pages).

It doesn't happen with large amount of tags, posts or pages.

Looking for the problem.

Attachments (1)

7147.diff (2.6 KB) - added by DD32 7 years ago.

Download all attachments as: .zip

Change History (19)

comment:1 @bi0xid7 years ago

It seems the problem is in class Walker_Category.

When using it in wp_list_categories, it gets the database rows one by one. It's more than 2000 queries in my system!

I must use another way to categorize information instead of categories. Any ideas?

comment:2 @DD327 years ago

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

I must use another way to categorize information instead of categories. Any ideas?

Tags?

See #6561 (handle thousands of pages)

comment:3 @bi0xid7 years ago

  • Resolution duplicate deleted
  • Status changed from closed to reopened

I cannot categorize tags like a tree.

You have closed the ticket because #6561 exists. But it talks about pages, not categories.

I'm going to reopen it, they are not the same.

comment:4 @DD327 years ago

  • Milestone set to 2.6

You have closed the ticket because #6561 exists. But it talks about pages, not categories.

oops, Sorry, Mind hadnt fully woken up; I confused the Posts ticket with the closed category ticket.

On my local system, I just setup 1000 categories, and assigned them all to a post. It does indeed take about 10 seconds to show the entire tree.

There are a total of 2 queries made which used 60ms total.

Using the PHP Trace logs, i've come up with this:

wp_widget_categories: self: 3052ms; total: 6529ms
  wp_list_categories: self: 0.5ms; total: 3477ms
    get_categories: total: 2265ms
      get_terms: total:1205ms
         _get_term_children: 1179ms and calls itself a few thousand times each taking 0.2~30ms which adds up to the 1179ms
    walk_category_tree: total: 2256ms
      Walker::walk 2265ms
        Walker::display_element 0.1~60ms called

Looking at the functions called:

Walker:display_element
Average: 3.8ms
Total time: 3840ms
Calls: 1019

get_category_link
Average: 0.4ms
Total time: 471ms
Calls: 2009

Walker::start_el
Average: 0.4ms
Total time: 380ms
Calls: 1000

It seems that quite a lot of the time is simple Walker::display_element taking 10+ms on some items.

comment:5 @bi0xid7 years ago

I have 2000 categories with 1020 posts (multi-categories to show a tree with the information).

When showing categories.php, for example, or index.php, which has wp_list_categories(), the page charges more than 692KB, stucking 50 secs later. Walker doesn't work properly with my system.

Apache/1.3.39 (Unix) mod_python/2.7.11 Python/2.4.4 mod_perl/1.29 PHP/5.2.6 MySQL/4.1.22

Is there any problem reported with this versions?

comment:6 @DD327 years ago

I'd say the main reason for the massive ammount of time is the fact your web server is slower than my local machine.

I noticed sizeof() was being called 200,000 times.. I've made some modifications which has pulled wp_list_categories() down from 36 seconds to 18 seconds. modifying _get_term_children() pulled off another 0.5seconds (I suspect that was just the pass-by-reference call)

@DD327 years ago

comment:7 follow-up: @ryan7 years ago

(In [8111]) Don't do sizeof() in a loop. Props DD32. see #7147

comment:8 follow-up: @ryan7 years ago

Applied the sizeof() fix. We definitely shouldn't be doing that in a loop.

Passing $terms by ref to _get_term_children() looks like it would cause a Call-time pass-by-reference warning since the functions declaration wasn't changed to accept pass by ref. Also, an array passed by ref keeps it internal array pointer. Recursive calls to _get_term_children() would be advancing the pointer on the same copy of the terms array, thus the location of where the calling function left off would be lost. Using for() instead of foreach() can get around this.

comment:9 @ryan7 years ago

By the way, categories.php is paged in 2.6, which helps a lot. Testing on a blog with 15k categories reduced the time to load categories.php from many minutes to about 8 seconds. Printing 20 categories per page is much faster than printing 15k. :-)

comment:10 @DD327 years ago

Passing $terms by ref to _get_term_children() looks like it would cause a Call-time pass-by-reference

Ah... The slight time decrease that that added was probably because it skipped over a few terms here and there.

categories.php is paged in 2.6, which helps a lot.

Hepls a lot, But still has a huge performance hit on the page mainly because the Add Category parent still has to list them all. It does help if you just want to access a category though.

comment:11 @ryan7 years ago

(In [8112]) Make the args for the category drop down query the same as the category table query so that the cache is used. Eliminates a query. see #7147

comment:12 @ryan7 years ago

The exact same query was being run for the dropdown and the table. This is because one had hierarchial set to true and the other set it to 1. Lame. We need to make the cache keying more robust. Worked around it for now by making both use true.

comment:13 in reply to: ↑ 7 @bi0xid7 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

Replying to ryan:

(In [8111]) Don't do sizeof() in a loop. Props DD32. see #7147

It works perfectly.

Thanks :)

comment:14 in reply to: ↑ 8 ; follow-up: @Otto427 years ago

Replying to ryan:

Applied the sizeof() fix. We definitely shouldn't be doing that in a loop.

Same problem exists in the walk() function on line 513.

comment:15 @westi7 years ago

(In [8118]) Move another sizeof out of the loop. See #7147.

comment:16 in reply to: ↑ 14 ; follow-up: @westi7 years ago

Replying to Otto42:

Replying to ryan:

Applied the sizeof() fix. We definitely shouldn't be doing that in a loop.

Same problem exists in the walk() function on line 513.

Ok. I have made a similar change. Don't have time to fully test.

Would be nice if someone could make sure I've not broken anything :-)

comment:17 in reply to: ↑ 16 @DD327 years ago

Replying to westi:

Ok. I have made a similar change. Don't have time to fully test.

Would be nice if someone could make sure I've not broken anything :-)

I made a few changes like that here and there on the local system, and everything seemed in order, Your commit looks ok too :)

That one didnt really affect performance though i dont think, I still havnt found a way to slim any more time off the category loop stuff :)

comment:18 @ryan7 years ago

I think we can do some pass by ref as long as we do a counted for loop rather than foreach. Passing by ref can really shave time with big arrays.

Note: See TracTickets for help on using tickets.