Make WordPress Core

Opened 10 years ago

Closed 10 years ago

#32144 closed defect (bug) (fixed)

get_terms with child_of

Reported by: shifty51's profile shifty51 Owned by: boonebgorges's profile boonebgorges
Milestone: 4.2.2 Priority: normal
Severity: normal Version: 4.2
Component: Taxonomy Keywords: fixed-major
Focuses: performance Cc:

Description

Hello, I have a problem with the "get_terms" when I use "child_of" since I installed version 4.2 of wordpress. The error is as follows

Fatal error: Maximum execution time of 30 seconds exceeded in
mywebsite.com/wp-includes/taxonomy.php on line 3958.

The term used in 'child of "a lot of childs (More than 800) and up to 2 levels and is itself a child of another term.
The line in question is in /wp-includes/taxonomy.php this:

// Do not recurse if we've already APPROBATION the term as a child - this Indicates a loop.
if (in_array ($ term-> term_id, $ ancestors)) {
continue;
}

I tried disabling all plugins and activating the twentyfifteen theme, simply by adding taxonomy declaration in the function.php file of the theme:

/ *
 * TAXONOMIES
 * /
add_action ('init', 'create_dossier_taxonomies', 0);

create_dossier_taxonomies function () {
  // Add new taxonomy, make it hierarchical (like categories)
  $ Labels = array (
    'Name' => _x ('Folders', 'general taxonomy name'),
    'Singular_name' => _x ('File', 'taxonomy singular name'),
    'Search_items' => __ ('Search Folder')
    'All_items' => __ ('All files')
    'Parent_item' => __ ('directory')
    'Parent_item_colon' => __ ('Parent Folder:')
    'Edit_item' => __ ('Edit File')
    'Update_item' => __ ('Update folder')
    'Add_new_item' => __ ('Add Folder')
    'New_item_name' => __ ('New folder name')
    'Menu_name' => __ ('File')
  );

  $ Args = array (
    'Hierarchical' => true,
    'Label' => $ labels
    'Show_ui' => true,
    'Show_admin_column' => true,
    'Query_var' => true,
    'Rewrite' => array ('slug' => 'file')
  );

  register_taxonomy ('file', array ('post', 'short'), $ args);
  // Add new taxonomy, make it hierarchical (like categories)

}

When I commented these lines I have no problem. Looking in the version 4.1.3, I feel that the function in question has been rewritten. It would there be a problem with "in_array"?

Sorry for my english...

Attachments (1)

32144.diff (2.0 KB) - added by boonebgorges 10 years ago.

Download all attachments as: .zip

Change History (10)

#1 @boonebgorges
10 years ago

If you're experiencing memory issues with this in_array() check, it suggests that you have a very large number of taxonomy terms, arranged in very deep hierarchies. Is this right? Can you give some numbers here - about how many folders do you have, and are they very very nested?

I can see a change we could make that should improve performance here (switch from in_array() to isset(), and flip the array), but before making this change, I want to be sure that this isn't a symptom of a deeper problem.

#2 follow-up: @shifty51
10 years ago

Ok, first thank you for your reply! The term in question is at level 1 of the taxonomy. It contains more than 800 childs (889).Not very nested. Some of the child have one lower level with a few children (10 at most). I tried to replace in_array by isset. The site no longer crashes but all terms are not apparent.
The structure :
Dossier (taxonomy)
--artiste (the term with 800 childs)
--localisation (less childs, no problem)

Last edited 10 years ago by shifty51 (previous) (diff)

@boonebgorges
10 years ago

#3 in reply to: ↑ 2 ; follow-up: @boonebgorges
10 years ago

Replying to shifty51:

Ok, first thank you for your reply! The term in question is at level 1 of the taxonomy. It contains more than 800 childs (889).Not very nested. Some of the child have one lower level with a few children (10 at most). I tried to replace in_array by isset. The site no longer crashes but all terms are not apparent.
The structure :
Dossier (taxonomy)
--artiste (the term with 800 childs)
--localisation (less childs, no problem)

Thanks for the additional details. When you said you tried to replace in_array by isset, did you try something like 32144.diff? If not, would you mind giving it a spin to see if it solves your problem? If so, I'll try to run some more precise benchmarks.

#4 in reply to: ↑ 3 @shifty51
10 years ago

Replying to boonebgorges:

Replying to shifty51:

Ok, first thank you for your reply! The term in question is at level 1 of the taxonomy. It contains more than 800 childs (889).Not very nested. Some of the child have one lower level with a few children (10 at most). I tried to replace in_array by isset. The site no longer crashes but all terms are not apparent.
The structure :
Dossier (taxonomy)
--artiste (the term with 800 childs)
--localisation (less childs, no problem)

Thanks for the additional details. When you said you tried to replace in_array by isset, did you try something like 32144.diff? If not, would you mind giving it a spin to see if it solves your problem? If so, I'll try to run some more precise benchmarks.

I tried just to change this part of the code.

if ( isset( $ancestors[ $term->term_id ] ) )

With all of your changes, it works!
Thank you! Will it is likely that this change to take effect in future versions?

Last edited 10 years ago by shifty51 (previous) (diff)

#5 @boonebgorges
10 years ago

  • Milestone changed from Awaiting Review to 4.2.2

Thanks, shifty51!

I've run some basic tests, and it looks like the performance benefit of switching to a keyed array + isset are very dramatic. For sites with a large number of terms in a taxonomy hierarchy with three or more terms, the difference can be several orders of magnitude. I created about 6000 terms, made 3000 of them a child of $foo, and then another 1000 a child of one of $foo's children. Then I ran get_terms( array( 'child_of' => $foo, 'hide_empty' => false ) ). Results:

without 32144.diff: 26s
with 32144.diff: ~0.4s

Because this problem has a potential to be pretty big on certain sorts of sites, and because it was introduced in 4.2, I'm moving to 4.2.x consideration.

#6 @boonebgorges
10 years ago

  • Owner set to boonebgorges
  • Resolution set to fixed
  • Status changed from new to closed

In 32326:

Improve performance of loop detection in _get_term_children().

Using an array keyed by term_id allows us to use isset() rather than the
slower in_array(). In addition, it lets us avoid the use of wp_list_pluck()
on large arrays, and helps us to avoid arrays that are unnecessarily large due
to duplicate entries.

Fixes #32144 for trunk.

#7 @boonebgorges
10 years ago

  • Component changed from General to Taxonomy
  • Focuses performance added
  • Keywords fixed-major added
  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening for 4.2.2.

#8 @SergeyBiryukov
10 years ago

#32245 was marked as a duplicate.

#9 @boonebgorges
10 years ago

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

In 32383:

Improve performance of loop detection in _get_term_children().

Using an array keyed by term_id allows us to use isset() rather than the
slower in_array(). In addition, it lets us avoid the use of wp_list_pluck()
on large arrays, and helps us to avoid arrays that are unnecessarily large due
to duplicate entries.

Fixes #32144 for 4.2 branch.

Note: See TracTickets for help on using tickets.