Avoid loading all terms on the taxonomy overview form

You can work around this with modules such as taxonomy manager. Downgrading to normal.

1: drupal-8.x-reduce_taxonomy_memory_consumption-2183565-1.patch queued for re-testing.

Re-rolling. Interdiff didn't make sense as most files have different paths now.

Discovered a few more cases where the results of the loadTree can be cleaned. I think the results of this patch are probably only useful for those using the forum module and several thousand taxonomy terms.

There is quite an extensive documentation available about profiling here: https://www.drupal.org/contributor-tasks/profiling . If you have any further questions please feel free to ping me on IRC! :)

I rerolled the patch so it applies to 8.2.x-dev so i will add it here. For me however, i still have memory issues even after this patch.

I think we need to reroll patch for 8.4.x-dev, then I can test on vocabulary with 40k terms.

Adding reroll for 8.4.x-dev, doubt it will work for a vocabulary with 40k items but is good to try.

Still exhausting 256MB on a vocabulary overview page containing 15000 terms using patch from #25.

A re-roll to follow core changes.

These unset calls aren't likely to help anyone much. PHP is already going to free that memory when the $tree variable de-scopes. The unset will help in the scenario where you are just at the edge of your memory limit and the next thing within the same function that defined the $tree variable pushes you over.

On a site with 10714 terms

Without the patch:

Total Incl. MemUse (bytes): 235,988,560 bytes
Total Incl. PeakMemUse (bytes): 242,675,400 bytes

With the patch:

Total Incl. MemUse (bytes): 236,111,760 bytes
Total Incl. PeakMemUse (bytes): 242,864,088 bytes

Patches are still not useful. I think there is no solution yet.
Does anyone have a different approach?

Hi! I'm on Drupal 8.8.1 Php version 7.3 Mariadb 5.5.5-10.0.38-MariaDB-0ubuntu0.16.04.1 Apache web server. I imported 23,855 taxonomy terms. It seems I'm getting same error. I applied the patches in comment #28 and #25 it seems the files in the patch are not the same with Drupal installation.
Below is what I'm getting when applying the patch:

git apply -v 2183565-28.patch
Checking patch core/modules/forum/src/Form/ForumForm.php...
Hunk #1 succeeded at 144 (offset 1 line).
Hunk #2 succeeded at 156 (offset 1 line).
Checking patch core/modules/forum/src/ForumManager.php...
Hunk #1 succeeded at 438 (offset 22 lines).
Checking patch core/modules/node/tests/src/Functional/NodeAccessPagerTest.php...
error: while searching for:
$tree = \Drupal::entityManager()->getStorage('taxonomy_term')->loadTree($vid, 0, 1);
$tid = reset($tree)->tid;
$this->assertTrue($tid, 'General discussion term is found in the forum vocabulary.');

// Create 30 nodes.
for ($i = 0; $i < 30; $i++) {

error: patch failed: core/modules/node/tests/src/Functional/NodeAccessPagerTest.php:77
error: core/modules/node/tests/src/Functional/NodeAccessPagerTest.php: patch does not apply
Checking patch core/modules/taxonomy/src/Form/OverviewTerms.php...
error: while searching for:
$current_page[$key] = $term;
} while (isset($tree[++$tree_index]));

// Because we didn't use a pager query, set the necessary pager variables.
$total_entries = $before_entries + $page_entries + $after_entries;
$pager_total_items[0] = $total_entries;

error: patch failed: core/modules/taxonomy/src/Form/OverviewTerms.php:200
error: core/modules/taxonomy/src/Form/OverviewTerms.php: patch does not apply
Checking patch core/modules/taxonomy/src/Plugin/EntityReferenceSelection/TermSelection.php...
error: while searching for:
foreach ($terms as $term) {
$options[$vocabulary->id()][$term->id()] = str_repeat('-', $term->depth) . Html::escape($this->entityManager->getTranslationFromContext($term)->label());
}
}
}
}

error: patch failed: core/modules/taxonomy/src/Plugin/EntityReferenceSelection/TermSelection.php:65
error: core/modules/taxonomy/src/Plugin/EntityReferenceSelection/TermSelection.php: patch does not apply
Checking patch core/modules/taxonomy/src/Plugin/views/filter/TaxonomyIndexTid.php...
error: while searching for:
$choice->option = [$term->id() => str_repeat('-', $term->depth) . \Drupal::entityManager()->getTranslationFromContext($term)->label()];
$options[] = $choice;
}
}
}
else {

error: patch failed: core/modules/taxonomy/src/Plugin/views/filter/TaxonomyIndexTid.php:185
error: core/modules/taxonomy/src/Plugin/views/filter/TaxonomyIndexTid.php: patch does not apply
Checking patch core/modules/taxonomy/src/TermForm.php...
Hunk #1 succeeded at 50 (offset 2 lines).
Hunk #2 succeeded at 65 (offset 2 lines).
Checking patch core/modules/taxonomy/src/Tests/TermTest.php...
error: core/modules/taxonomy/src/Tests/TermTest.php: No such file or directory
Checking patch core/modules/taxonomy/taxonomy.module...
error: while searching for:
$hierarchy = VocabularyInterface::HIERARCHY_SINGLE;
}
}
if ($hierarchy != $vocabulary->getHierarchy()) {
$vocabulary->setHierarchy($hierarchy);
$vocabulary->save();

error: patch failed: core/modules/taxonomy/taxonomy.module:193
error: core/modules/taxonomy/taxonomy.module: patch does not apply
Checking patch core/modules/taxonomy/tests/src/Kernel/TermKernelTest.php...
Hunk #1 succeeded at 109 (offset -4 lines).
Hunk #2 succeeded at 123 (offset -4 lines).

Thanks in advance!

Same for D9.2

$tree = $this->storageController->loadTree($taxonomy_vocabulary->id(), 0, NULL, TRUE);

Loads all entities of vocabulary (no pager value used), but why?

Patch #28 can not be applied to core 9.2.7 and to the latest 9.2.x (1a5cc0d9ab68d8c7b22143c4642a691365171cd1 2021-10-25 23:15:55 +0100). Which makes sense because patch #28 was created for 8.5.x.

Thanks for the reroll.
I think the unset() not going to solve this memory problem. PHP's garbage collector does a better job than we can do manually. I think the unset also pointless in tests files where there is only a few amount of terms (less than 10).

I have a vocabulary with >34000 terms without hierarchy and I can't view the /admin/structure/taxonomy/manage/my_voc_01/overview page with memory limit 512MB.

The attached patch solves the problem, but I am not sure if this is the right direction.
However the original code also uses expressions like these: $term->depth and $term->parents

• https://git.drupalcode.org/project/drupal/-/blob/1a5cc0d9ab68d8c7b22143c...
• https://git.drupalcode.org/project/drupal/-/blob/1a5cc0d9ab68d8c7b22143c...

Not sure why the "PHP 7.3 & MySQL 5.7" failed. I retested using PHP 7.4and it passed fine.

I agree with Sweetchuck I don't see unset adding much benefit to the patch. This appears to lower the load in manual testing.

This creates other sort issues:

1. Create a vocabulary that has parent/child terms.
- If you go to the overview page you will not be able to see the tree just the flat list
- If you try to update the tree and save it (on the overview page), you will get a bunch of notices.

Re-roll to address issues in 43

I can confirm the patch #45 solves the issue. I just applied this patch on our dev instance of a large newspaper site and it works great! Kudos! Not only does the page load (right now we are not able to look at the taxonomy overview page because we used to run out of memory) but it loads and works super fast. This was tested on the taxonomy overview for a large site with over 56000 tags. +1 on this.

Tested the patch #45 o newer Drupal (the previous test was done on a Drupal 8.6.18. Surprisingly the patch applied cleanly!). I tested now on a Drupal 9.3.12 and also worked fine. It was running out of memory with PHP 7.4 and 256M of memory limit. The taxonomy had around 39000 tags. With the latest patch it worked perfectly. Also tested on much smaller taxonomies with terms of depth 2 and it worked fine. Probably a someone should test on something with deeper taxonomies. The ones I have are large but have depth = 1. Still looking very promising in my opinion. Great work everybody!

Patch #45 does not apply on D9.4.x.

Hey @super_romeo! I created a patch manually since its just a couple of lines. Please test it. It should apply cleanly to D9.4.x.

Works fine with D9.4.x
Also the issue reported in #43 is fixed too.

This is not a patch to reduce memory consumption, but it is closely related and may fix the issue for some.
For this patch to work, you need to set the memory per vocabulary on your settings.php file.

$config['taxonomy_load_tree_memory']['big_vocabulary'] = '750M';
$config['taxonomy_load_tree_memory']['big_vocabulary_2'] = '1250M';`

Retitling since this is changing the taxonomy overview controller, not the ::loadTree() method itself.

The patch in #49 looks good, however we should add an inline comment explaining that we're avoiding loading all terms, and that the ones shown on the current page get loaded later.

This patch doesn't work for me with 4000 child terms on a single parent (502 error).

The issue still seems to exist in D10 as well. Attached is patch #49 from branch 10.1.x.

This is an extension of the #73 patch which fixes memory leak error not only on the terms overview page but also on a term edit page.

It allows to reduce number of loaded terms by reflecting pager "page" parameter into the sql query range().

Pager still works the same with one exception for the "Last" button which moves by 10 pages forward while clicking on it.

If you like me have an insane number of terms / pager pages then the "page" parameter in the browser address bar can be edited directly to get you to the very end of the pager.

We just ran into this issue migrating a Drupal 7 website with a large taxonomy to Drupal 10. So this is still important to fix, as it breaks large sites. Reading the comments above, I think we still need a plan on how to fix this deep underlying issue, before we invest more time in code here?

At least with pagination, this should not break:
drush config:set taxonomy.settings terms_per_page_admin 100

But a written above, changing the page size has no effect on this error.

Is any of the above approaches at least an improvement from the Core maintainers perspective? Who could have a look?

Not sure why this issue derailed. The patch in #45 worked for me in several tests/sites with large taxonomies and had the blessing by Catch. All that remained was to add a comment. In all honesty I didn't understand exactly what the comment was but I believe once that comment is added this was RTBC.

There is also a version for 10.x. in #54. I recommend you test that version and leave a comment if it works.

I don't recommend the solutions that try to increase the memory size as suggested as they will eventually break. The proposed solution worked for me on two large sites I tested.

Patch #54 works well with the latest Drupal 10.0.9. Thanks.

Created MR and patch against 11.x based on #49and including suggestions by catch in #52

As a bug will need a test case showing the issue please

Thanks!

Thank you for the heads-up smustgrave.

Attaching new version of the patch including tests and improving the load terms logic for the submit handelr as well.

New patch version fixing typo.

Giving credit to bbrala for pointing me out to a good way for doing a profile using ddev and phpstorm.

Using devel generate created 1000 terms and went to the taxonomy overview page of my vocabulary

Attaching before/after

Before

After

So seeing a slight faster render.

Patch 2183565-69.patch is not working for me. Still getting:
Fatal error: Allowed memory size of 805306368 bytes exhausted (tried to allocate 4096 bytes) in /var/www/web/modules/contrib/taxonomy_menu/src/TaxonomyMenuHelper.php on line 104
Fatal error: Allowed memory size of 805306368 bytes exhausted (tried to allocate 24576 bytes) in Unknown on line 0
Could it be because I'm also using Taxonomy menu module?
Update: yes, when disabling TM it works well.

Hi :)

Since 9.4 all my taxo more big 2000 is block, I put this patch and works now:
https://www.drupal.org/project/drupal/issues/3327118#comment-15252799

But, all big content is still block: Error Vanish cache server.

I can not make Sitemap too for big content, Drupal 9.4 break all web site... :(

Moving back to RTBC (per #71) - there was a random failure.

Re #73: I have tested the patch #69 and is working for us on a test site. Taxonomy with 2000+ terms: without the patch - out of memory, after patch is applied: the page loaded without any problems.

+++ b/core/modules/taxonomy/src/Form/OverviewTerms.php
@@ -520,9 +527,10 @@ public function submitForm(array &$form, FormStateInterface $form_state) {
 
     // Build a list of all terms that need to be updated on previous pages.
     $weight = 0;
-    $term = $tree[0];
-    while ($term->id() != $form['#first_tid']) {
-      if ($term->parents[0] == 0 && $term->getWeight() != $weight) {
+    $raw_term = $tree[0];
+    while ($raw_term->tid != $form['#first_tid']) {
+      if ($raw_term->parents[0] == 0 && $raw_term->weight != $weight) {
+        $term = $this->storageController->load($raw_term->tid);
         $term->setWeight($weight);

I think it would be worth building the list of term IDs in advance, and then using ::loadMultiple() here.

Similar in the next hunk of the MR too.

Otherwise looks good though.

Good point.

Added new version of the patch storing in an index all the entities and their weight to load all of them in a single call afterwards.

Add missing interdiff.

There are no test failures in the patch #77, but I am curious how is the while loop supposed to work now, as the $raw_term variable is not being changed at all?

@@ -526,14 +533,13 @@ public function submitForm(array &$form, FormStateInterface $form_state) {
     $weight = 0;
+    $raw_term = $tree[0];
+    $term_weights = [];
+    while ($raw_term->tid != $form['#first_tid']) {
+      if ($raw_term->parents[0] == 0 && $raw_term->weight != $weight) {
+        $term_weights[$raw_term->tid] = $weight;
       }
       $weight++;
-      $term = $tree[$weight];
     }

Good catch!

That part of the code is only affected if we are loading a page that's not the first and root terms weight is not starting by 0.

Added extra tests to cover that case.

While working on this, I found out there are no tests to ensure that overview terms form submit callback behaves as expected. Should they be added here or maybe better to open a follow up issue for that?

Thanks!

Fix typo in comment.

Failed asserting that actual size 11 matches expected size 5.

Ran new tests to make sure they failed and clearly they did.

Seems feedback has been updated.

1. +++ b/core/modules/taxonomy/tests/modules/taxonomy_test/taxonomy_test.module
   @@ -46,3 +46,16 @@ function taxonomy_test_form_taxonomy_term_form_alter(&$form, FormStateInterface
   +  $value = \Drupal::state()->get(__FUNCTION__);
   +  if (isset($value)) {
   +    foreach ($entities as $entity) {
   +      $value[$entity->id()] = $entity;
   +    }
   +    \Drupal::state()->set(__FUNCTION__, $value);
   +  }
   

   I don't think we should be save the term objects to state.

   I think this could be

   $value = \Drupal::state()->get(__FUNCTION__);
   // Only record loaded terms is the test has set this to an empty array.
   if (is_array($value)) {
     $value = array_merge($value, array_keys($entities));
     
     \Drupal::state()->set(__FUNCTION__, array_unique($value));
   }
   

2. +++ b/core/modules/taxonomy/tests/src/Functional/TaxonomyTermPagerTest.php
   @@ -73,4 +73,48 @@ public function testTaxonomyTermOverviewPager() {
   +  public function testTaxonomyTermOverviewTermLoad() {
   +    // Set limit to 3 terms per page.
   +    $this->config('taxonomy.settings')
   +      ->set('terms_per_page_admin', '3')
   +      ->save();
   +
   +    $state = $this->container->get('state');
   +    $state->set('taxonomy_test_taxonomy_term_load', []);
   

   The first state->set() is not required.

I've attached a patch that does the above. It also adjusted things so that we use loadMultiple() in the buildForm - this is very similar to what @catch requested in for the submit in #76

Ah actually we already have a list of tids to load :) no need to built yet-another-list-of-tids...

Updated the MR with all the changes from the issue after the last commit to it. Hiding all the files.

Seems you addressed your own feedback :)

This is not a deep performance benchmark, but I can confirm that the MR !4110 is working on few our sites with thousands of taxonomy terms. Before changes from MR were applied, there was an OOM error on the taxonomy overview page (admin/structure/taxonomy/manage/vocabulary/overview). After applying the changes, the page is working again.

I think a more detailed benchmark could help for this to be committed (for example memory usage on that page before/after, etc..), as there is a "needs profiling" tag, but otherwise I think this works pretty well.

I think it should be possible to write a performance test for this using #3352851: Allow assertions on the number of database queries run during tests - I wouldn't block it on that, this issue has existing for many years longer than that capability, but I'll open a spin-off and try to do a before/after comparison if I can and then we can commit the test coverage in its own issue.

I opened #3410092: Performance tests for the taxonomy overview page and wrote the test, then merged the branch here in and ran the test again - and as I was writing the test coverage, realised we multiple load taxonomy terms in taxonomy get tree, which will also multiple get the cache items, and multiple set the cache items too. Given that, we can't detect the performance improvement here with PerformanceTestBase and the test run after the merged branch confirmed that - we're exchanging a massive database query for a smaller one, and nothing runs per-term.

Bit of a shame that the tests can't detect it, but also if it had detected a difference, there'd be something wrong with multiple loading, it the test would have least have probably detected the multiple load regression fixed in #84 though so worth adding anyway

Committed/pushed to 11x and cherry-picked to 10.2.x, thanks!

PS this issue impacts not just the taxonomy overview page, but taxonomy edit, batch update etc etc. (using Drupal 10.2.0 and php 8.1)
I have a large location based taxonomy with continents -> countries -> cities and unable to make edits, bulk update url aliases due to 500 timeout errors as Drupal is trying to load all terms...

Moved to https://www.drupal.org/project/drupal/issues/3493621