Content: Has taxonomy term ID (with depth) query performance

Just update status so it will not be missed.
In general improving this query would be really cool. Sadly it's too late for me to take a proper review but i would like it
if you explain what you do in the code. This would be a huge plus for maintainability.

While removing those joins is a good thing, the sub query at the end is still bad. It means that the database will likely still fetch a bunch of rows it won't need for the result set.

This is what I've observed on a database with over 700,000+ records in the node table and 1,000,000+ records in the taxonomy_index table.

So finally, it would be better to do an INNER JOIN than a subquery to directly limit the amount of rows fetched to exactly what the query wants back.

My patch increased my queries from 6 seconds with the previous in #3 to 2.20ms.

Triggering the testbot.

Puh it would be good to have an issue summary here, as this is not that easy to understand.

This is a real issue.. I have also performance problems using taxonomy term ID (with depth) query.

change code section

Looks like this is also a problem in views_handler_filter_term_node_tid_depth.inc - patch on its way.

Added a structured summary.

I have tested both patch #3 and #4.
Patch #4 seems to be faster than #3 but the results are incorrect.
I have patched views_handler_filter_term_node_tid_depth.inc and merged with patch #3.

Just changed the name of the file

I've reviewed the patch and it looks good. I've also benchmarked a sample query (attached) that uses the term with depth filter in mysqlslap with the following results (iterations set to 5 for every test):

*Query from prosper block. Attached.

Un-cached:
Average number of seconds to run all queries: 18.383 seconds
Minimum number of seconds to run all queries: 0.009 seconds
Maximum number of seconds to run all queries: 91.879 seconds
Number of clients running queries: 50
Average number of queries per client: 1

Query cached:
Average number of seconds to run all queries: 0.008 seconds
Minimum number of seconds to run all queries: 0.007 seconds
Maximum number of seconds to run all queries: 0.009 seconds
Number of clients running queries: 50
Average number of queries per client: 1

Patched version (un-cached):
Average number of seconds to run all queries: 0.027 seconds
Minimum number of seconds to run all queries: 0.009 seconds
Maximum number of seconds to run all queries: 0.098 seconds
Number of clients running queries: 50
Average number of queries per client: 1

Patched version (cached):
Average number of seconds to run all queries: 0.007 seconds
Minimum number of seconds to run all queries: 0.007 seconds
Maximum number of seconds to run all queries: 0.009 seconds
Number of clients running queries: 50
Average number of queries per client: 1

The pre-patched query was very slow when there were a lot of concurrent requests and if the query has not been cached.

This patch does add to the total queries made for the view when building the nid list for the IN operator but I believe these will be fast queries via API calls.

I really fear that this might break something for people and honestly I have no clue at all how this file really works.

Can we maybe introduce a new handler so people can choose that explicit in the UI, so people can use it, if wanted?

@Josh the taxonomy_index can have more than one row with the same nid (a node with multiple terms attached to it) so if you are joining it onto the node table you could potentially have the same node multiple times in the result set. Yes, we are filtering by tid but the node could have a parent and child term attached to it (both in the list of tids) so it would appear twice.

@Josh this might be expected behaviour from the users point of view. They could add a group by if they want unique nodes.

@dawehner we have just released this on a site with some major traffic. We can monitor its performance for now. Adding another handler that does exactly the same thing as an existing one but just does it a different way is bound to confuse users. I think more testing and another review is probably the way to go. I think the focus needs to be on the tid list that is now build to be inserted into the query. Could there be a variation between this and whats returned by the old select sub query?

The query can be further optimized if this

AND (node.nid IN (
  SELECT tn.nid AS nid
  FROM taxonomy_index tn
  WHERE tn.tid = '530'
))

Is transformed into an inner join like this

INNER JOIN taxonomy_index AS tn 
  ON tn.nid = node.nid
  AND tn.tid = '530'

Original Query: 0.5817 sec

SELECT 
  node.title AS node_title, 
  node.nid AS nid, 
  'taxonomy_term:feed' AS view_name
FROM node AS node
WHERE (node.status = 1 OR (node.uid = 2 AND 2 <> 0 AND 1 = 1) OR 1 = 1) 
AND (node.type IN ('basic_information')) 
AND (node.nid IN (
  SELECT tn.nid AS nid
  FROM taxonomy_index AS tn
  LEFT OUTER JOIN taxonomy_term_hierarchy AS th ON th.tid = tn.tid
  LEFT OUTER JOIN taxonomy_term_hierarchy AS th1 ON th.parent = th1.tid
  LEFT OUTER JOIN taxonomy_term_hierarchy AS th2 ON th1.parent = th2.tid
  LEFT OUTER JOIN taxonomy_term_hierarchy AS th3 ON th2.parent = th3.tid
  LEFT OUTER JOIN taxonomy_term_hierarchy AS th4 ON th3.parent = th4.tid
  LEFT OUTER JOIN taxonomy_term_hierarchy AS th5 ON th4.parent = th5.tid
  LEFT OUTER JOIN taxonomy_term_hierarchy AS th6 ON th5.parent = th6.tid
  LEFT OUTER JOIN taxonomy_term_hierarchy AS th7 ON th6.parent = th7.tid
  LEFT OUTER JOIN taxonomy_term_hierarchy AS th8 ON th7.parent = th8.tid
  LEFT OUTER JOIN taxonomy_term_hierarchy AS th9 ON th8.parent = th9.tid
  WHERE tn.tid = '530' 
  OR th1.tid = '530' 
  OR th2.tid = '530' 
  OR th3.tid = '530' 
  OR th4.tid = '530' 
  OR th5.tid = '530' 
  OR th6.tid = '530' 
  OR th7.tid = '530' 
  OR th8.tid = '530' 
  OR th9.tid = '530'
))
ORDER BY node_title ASC

Patched Query: 0.0510 sec

SELECT 
  node.title AS node_title, 
  node.nid AS nid, 
  'taxonomy_term:feed' AS view_name
FROM node AS node
WHERE (node.status = 1 OR (node.uid = 2 AND 2 <> 0 AND 1 = 1) OR 1 = 1) 
AND (node.type IN ('basic_information')) 
AND (node.nid IN (
  SELECT tn.nid AS nid
  FROM taxonomy_index tn
  WHERE tn.tid = '530'
))
ORDER BY node_title ASC

Better Optimized Query: 0.0092 sec

SELECT 
  node.title AS node_title, 
  node.nid AS nid, 
  'taxonomy_term:feed' AS view_name
FROM node AS node
INNER JOIN taxonomy_index AS tn 
  ON tn.nid = node.nid
  AND tn.tid = '530'
WHERE (node.status = 1 OR (node.uid = 2 AND 2 <> 0 AND 1 = 1) OR 1 = 1) 
AND (node.type IN ('basic_information')) 
ORDER BY node_title ASC

Query results are the same for all 3 queries; but the last query is faster and doesn't do a table scan (55k rows) like the previous 2

Patch in #11 changed so that it uses an inner join with a condition.

Hi Mikeytown2

I tried your patch. It seems that the results are showing twice as many articles.

I am attaching the snapshot of the last page of a view using the patch #11 and patch #21.

Also, attaching the snapshot of Page load time (using NewRelic).
On this graph you will see 3 sets of spikes, as I am loading page 1 and page 2 of the same view.
The first set of spikes is without patches.
Second set of spikes is using patch #11.
Third set of spikes is using patch #21.

@marcelovani
I'm guessing the extra load in your case is due to twice as many results being returned. I would bet that fixing the extra results would solve the increased load issue.

Any chance you are able to capture the queries generated by patch #11 & patch #21? That would help me debug and fix this.

Notes for me: http://stackoverflow.com/questions/3088686/how-to-make-a-distinct-join-w...

Patch #11
Query: 215 ms

SELECT node.sticky                                                AS node_sticky
       ,
       nodequeue_nodes_node.position                              AS
       nodequeue_nodes_node_position,
       field_data_field_published_date.field_published_date_value AS
       field_data_field_published_date_field_published_date_value,
       node.nid                                                   AS nid
FROM   node node
       LEFT JOIN nodequeue_nodes nodequeue_nodes_node
              ON node.nid = nodequeue_nodes_node.nid
                 AND nodequeue_nodes_node.qid = :views_join_condition_
       LEFT JOIN field_data_field_child_page field_data_field_child_page
              ON node.nid = field_data_field_child_page.field_child_page_nid
       LEFT JOIN field_data_field_child_items
                 field_data_field_child_items_field_data_field_child_page
              ON field_data_field_child_page.entity_id =
field_data_field_child_items_field_data_field_child_page.field_child_items_value
LEFT JOIN node node_field_data_field_child_items_field_data_field_child_page
       ON field_data_field_child_items_field_data_field_child_page.entity_id =
          node_field_data_field_child_items_field_data_field_child_page.nid
LEFT JOIN field_data_field_published_date field_data_field_published_date
       ON node.nid = field_data_field_published_date.entity_id
          AND ( field_data_field_published_date.entity_type =
                :views_join_condition_1
                AND field_data_field_published_date.deleted =
                    :views_join_condition_2 )
WHERE  ( ( ( node.status = :db_condition_placeholder_0 )
           AND ( node.nid IN (SELECT tn.nid AS nid
                              FROM   taxonomy_index tn
                              WHERE  (( tn.tid = :db_condition_placeholder_1 )))
               ) )
         AND (((
       node_field_data_field_child_items_field_data_field_child_page.nid IS
       NULL ))
             ) )
ORDER  BY node_sticky DESC,
          nodequeue_nodes_node_position DESC,
          field_data_field_published_date_field_published_date_value DESC
LIMIT  10 offset 0

Patch #21
Query: 700 ms

SELECT node.sticky                                                AS node_sticky
       ,
       nodequeue_nodes_node.position                              AS
       nodequeue_nodes_node_position,
       field_data_field_published_date.field_published_date_value AS
       field_data_field_published_date_field_published_date_value,
       node.nid                                                   AS nid
FROM   node node
       LEFT JOIN nodequeue_nodes nodequeue_nodes_node
              ON node.nid = nodequeue_nodes_node.nid
                 AND nodequeue_nodes_node.qid = :views_join_condition_
       LEFT JOIN field_data_field_child_page field_data_field_child_page
              ON node.nid = field_data_field_child_page.field_child_page_nid
       LEFT JOIN field_data_field_child_items
                 field_data_field_child_items_field_data_field_child_page
              ON field_data_field_child_page.entity_id =
field_data_field_child_items_field_data_field_child_page.field_child_items_value
LEFT JOIN node node_field_data_field_child_items_field_data_field_child_page
       ON field_data_field_child_items_field_data_field_child_page.entity_id =
          node_field_data_field_child_items_field_data_field_child_page.nid
INNER JOIN taxonomy_index tn
        ON node.nid = tn.nid
           AND tn.tid = :views_join_condition_1
LEFT JOIN field_data_field_published_date field_data_field_published_date
       ON node.nid = field_data_field_published_date.entity_id
          AND ( field_data_field_published_date.entity_type =
                :views_join_condition_2
                AND field_data_field_published_date.deleted =
                    :views_join_condition_3 )
WHERE  ( (( node.status = :db_condition_placeholder_0 ))
         AND (((
       node_field_data_field_child_items_field_data_field_child_page.nid IS
       NULL ))
             ) )
ORDER  BY node_sticky DESC,
          nodequeue_nodes_node_position DESC,
          field_data_field_published_date_field_published_date_value DESC
LIMIT  10 offset 0

I've updated the patch to use joins – it seems to be performing as well as the sub query plus it will take advantage of the MySQL query cache. See https://dev.mysql.com/doc/refman/5.7/en/query-cache-operation.html

I had to add $this->query->distinct = TRUE; to views_handler_argument_term_node_tid_depth in order to prevent duplicates - I left this out of views_handler_filter_term_node_tid_depth as there is the option to reduce duplicates when adding the filter.

Updated patch with uppercase LEFT

@davidgrayston
Can you show the before/after of what a query will look like with the patch?

Patch updated to use the correct auto generated table alias

Before (sub query #11):

SELECT node.sticky AS node_sticky, node.nid AS nid
FROM 
node node
WHERE (( (node.status = '1') AND (node.nid IN  (SELECT tn.nid AS nid
FROM 
taxonomy_index tn
WHERE ( (tn.tid IN  ('18', '650', '1456', '97', '66', '629', '406', '63', '467', '1963', '65')) ))) ))
ORDER BY node_sticky DESC
LIMIT 20 OFFSET 0

After (joins #29):

SELECT DISTINCT node.sticky AS node_sticky, node.nid AS nid
FROM 
node node
LEFT JOIN taxonomy_index taxonomy_index ON node.nid = taxonomy_index.nid
WHERE (( (node.status = '1') AND (taxonomy_index.tid IN  ('18', '650', '1456', '97', '66', '629', '406', '63', '467', '1963', '65')) ))
ORDER BY node_sticky DESC
LIMIT 20 OFFSET 0

I've been trying the join approach out on a few views with varying performance.

The sub query approach tends to be faster – an example using sub query that took 0.20 sec, is taking 0.81 sec with the left join.

The join approach seems to degrade quickly as you add more tids to taxonomy_index.tid IN ()

I think this is because the sub query is able to use an index for tn.tid IN (), whereas the join approach doesn't use an index containing a tid (term_node,nid available, but chooses nid in my case).

I tried a few things to speed up the join query:

1. I added a new tid_nid index on taxonomy_index, which made the join query run in 0.20 seconds – CREATE INDEX tid_nid ON taxonomy_index (tid, nid)
2. I played around with the query away from views and was able to get the join query down to 0.11 sec, by forcing the join to use the term node index with the hint FORCE INDEX FOR JOIN (`term_node`) – Unfortunately there isn't any easy/clean way to alter the view query in this way, and perhaps this is pushing the use of views too far.

Alternative patch that adds a new handler "Has taxonomy term ID with depth (using joins)".

This is essentially a copy of the existing handler "Has taxonomy term ID (with depth)", with a few adjustments.

Looking at #30

What if the query was this (move the tid into the join condition)

SELECT DISTINCT node.sticky AS node_sticky, node.nid AS nid
FROM node node
LEFT JOIN taxonomy_index taxonomy_index 
  ON node.nid = taxonomy_index.nid
  AND taxonomy_index.tid IN ('18', '650', '1456', '97', '66', '629', '406', '63', '467', '1963', '65')
WHERE node.status = '1'
ORDER BY node_sticky DESC
LIMIT 20 OFFSET 0

We'd need to make it an inner join so that it filters the nodes like this:

SELECT DISTINCT node.sticky AS node_sticky, node.nid AS nid
FROM node node
INNER JOIN taxonomy_index taxonomy_index
  ON node.nid = taxonomy_index.nid
  AND taxonomy_index.tid IN ('18', '650', '1456', '97', '66', '629', '406', '63', '467', '1963', '65')
WHERE node.status = '1'
ORDER BY node_sticky DESC
LIMIT 20 OFFSET 0

I'll try this method out on a larger data set to see if it improves performance

Here's a patch in the meantime that adds a new handler and uses an inner join (pretty similar to your earlier patch)

If we know the conditions in which one way is faster than the other way we could pick the best way given what we know. Also allow for this to be overridden so the behavior can be changed if we get it wrong.

Their performance is very similar and the INNER JOIN seems like the correct implementation – If there are any adverse effects with larger data sets (with either), I'll post an update here.

Wanted to share some real life testing results.

I've compared patches #11, #21 and #29 locally with one of the views that was taking the most time (3 runs each):

* No patch: 3645 ms
* Patch #11: 1640 ms
* Patch #21: 1646 ms
* Patch #29: 1681 ms

Then I tested at a larger scale on a home page with many (about 10) complex views, most using "Content: Has taxonomy term ID (with depth)" contextual filter:

* No patch: 16367 ms
* Patch #11: 7842 ms
* Patch #21: 6116 ms
* Patch #29: 6052 ms

We decided to go ahead with #21. It has been out in the wild (production) for about a week now serving 26 radio station websites on Acquia SiteFactory (multisite).
The number of slow queries went down 2 times since the patch deployment - http://take.ms/0kC1t

No adverse effects of the patch have been noticed so far.

Note: most of the sites have taxonomy depth of 2, some may have 3, but that is not common.

#35 works great for me (query went down to 300ms from 1s) and nevertheless it is good to have an alternative.
It is also in a new handler like @dawehner requested, so, lets get this in

We've recently switched our testing from the old qa.drupal.org to DrupalCI. Because of a bug in the new system, #2623840: Views (D7) patches not being tested, older patches must be re-uploaded. On re-uploading the patch, please set the status to "Needs Review" so that the test bot will add it to its queue.

If all tests pass, change the Status back to "Reviewed & tested by the community". We'll most likely commit the patch immediately without having to go through another round of peer review.

We apologize for the trouble, and appreciate your patience.

Re-uploading the patch, let's see if it works

Back to RTBC

+++ b/modules/taxonomy/views_handler_argument_term_node_tid_depth_join.inc
@@ -0,0 +1,191 @@
+    // TODO review text
+    return t('No name');

Let's figure out what we're doing with this part, or remove the TODO if it's fine as-is.

+++ b/modules/taxonomy/views_handler_argument_term_node_tid_depth_join.inc
@@ -0,0 +1,191 @@
+    // TODO review text
+    return t('No name');

I propose returning empty string ''. ctools_term_description_content_type_render() does that, and ds_field_formatter_view() does that.

Yet that @todo looks like it was copied from views_handler_argument_term_node_tid_depth.inc and views_handler_argument_taxonomy.inc

Anybody +1 that idea and I'll roll it into a patch?

You're probably right http://cgit.drupalcode.org/views/tree/modules/taxonomy/views_handler_arg...
I would leave the 'No name' and remove the TODO

manually edited

This looks pretty good and it safes a hell lot of time.

Now we need to forward port it to D8 and add tests as well.

thanks! quick porting to d8, would be nice to also have it in soon

Maybe related: #2133215: Fix IndexTidDepth views argument plugin and TaxonomyIndexTidDepth views filter plugin performance

Patch 47 no longer applies. Needs reroll.
Should make followup issue in the views project queue.

Unfortunately patch #55 also no longer applies with drupal-8.3.5 and fails after manual changes made in core/modules/taxonomy/taxonomy.views.inc and core/modules/views/views.module. Checked with php 7.0.14-1~dotdeb+8.1

this was committed to views 7.x quite a while back
http://cgit.drupalcode.org/views/commit/?id=4c4cedfc0e9177cb7d8820e6a8c3...
author rootatwc 2016-02-25 08:27:23 +0100
committer Daniel Wehner 2016-02-25 08:27:23 +0100

so the 7.x version is done.

Just have to reroll the 8.x version

Please reroll #55

Is the D7 commit still valid? My D7 is still spewing out suicidal queries like this one:

SELECT tn.nid AS nid 
FROM   taxonomy_index tn 
       LEFT OUTER JOIN taxonomy_term_hierarchy th 
                    ON th.tid = tn.tid 
       LEFT OUTER JOIN taxonomy_term_hierarchy th1 
                    ON th.parent = th1.tid 
       LEFT OUTER JOIN taxonomy_term_hierarchy th2 
                    ON th1.parent = th2.tid 
       LEFT OUTER JOIN taxonomy_term_hierarchy th3 
                    ON th2.parent = th3.tid 
       LEFT OUTER JOIN taxonomy_term_hierarchy th4 
                    ON th3.parent = th4.tid 
WHERE  ( ( tn.tid = '19' ) 
          OR ( th1.tid = '19' ) 
          OR ( th2.tid = '19' ) 
          OR ( th3.tid = '19' ) 
          OR ( th4.tid = '19' ) ); 

998 rows in set (22.50 sec)

After adjusting depth to one, this comes up:

SELECT          tn.nid AS nid 
FROM            taxonomy_index tn 
LEFT OUTER JOIN taxonomy_term_hierarchy th 
ON              th.tid = tn.tid 
LEFT OUTER JOIN taxonomy_term_hierarchy th1 
ON              th.parent = th1.tid 
WHERE           ( ( 
                                                tn.tid = '19') 
                OR              ( 
                                                th1.tid = '19') ))

998 rows in set (0.77 sec)

And this is from a taxonomy_index of just 137322 rows and a taxonomy_term_hierarchy of 2100 rows.

The taxonomy system is the "Drupal way" of categorizing content, so it really shouldn't take 0.77 seconds to ask the database for a simple list of id's in a certain category. In this example, tid 19 has 8 sub-terms, so it should fetch these 8 first, and then check for children WHERE parent = tid of any of these eight. This can be done with a subquery or multiple queries. The current JOIN however is flawed because it has to fetch every term in the database first, and then fetch each parent of each of term to see if it belongs to tid 19, which is the wrong way to do it.

Re-rolled the patch in #55 for Drupal 8.

I also updated the array literal syntax and adjusted the join to be an inner join (to match what was done in D7). Interdiff included.

Sorry--the patch in 66 had a syntax error.

Updated Functional tests, resolved coding standards issues & fixed test failures from #67

Patch applies cleanly without any errors.
If Code review is good this can be moved to RTBC

Fixing the spelling errors.

This is used on a few sites in production for a client of mine. The sites are moderately large - 30,000+ nodes - 10,000+ terms and this patch makes views based on terms with depth much more performant. I also think the fact that Views 3 for Drupal 7 has this change in means we should consider it for Drupal 9.

One concern I have is how is a new Drupal user supposed to know whether they should use 'Taxonomy term ID with depth (using joins)' or 'Taxonomy term ID with depth'

I think we should reconsider #13 and change the existing handlers to use this method. I think the benefit for large sites outweigh the potential costs of doing 2 queries (one to get the taxonomy terms and one to get the nodes).

Tests were added in #55.

The tests weren't actually tests what we thought they were because of their field values - they were using the current core plugins.

This patch also includes some work to only load the terms when absolutely necessary. I still think that we should improve the current plugins rather than add new ones because I have no idea how to explain the difference in the UI.

And a patch... :)

Moved tests to kernel tests for speed. Made the argument test more like the filter test so we have better coverage and optimised the filter and argument plugins for less taxonomy loading and less looping. Also added test coverage of providing multiple terms via argument or filter config.

And here's the consolidation so we don't have two plugins doing the same thing. We have better tests and in fact if you use the old plugin one of the test cases fail due to sorting - that said the sort on the test view is kinda meaningless because all nodes are created with the same time because they use all the same request time.

And now with service injection.

Performance tests:

1. Install standard
2. Install drush (composer require drush.drush)
3. Install devel_generate (composer require drupal/devel && drush en devel_generate)
4. Create 50 terms (drush devel-generate:terms --bundles=tags)
5. Create 50 articles (drush devel-generate:content --bundles=article)
6. Create a view on node teasers using the taxonomy depth argument - use a depth of 4
7. Enables views performance statistics and query output on preview

50 terms & 50 nodes

Without patch

Query build time 2.56 ms
Query execute time 3.34 ms
View render time 11.63 ms

With patch

Query build time 4 ms
Query execute time 1.94 ms
View render time 10.89 ms

As expected the build time is a bit longer but the query time is quicker... so for small data sets it is basically the same.

50050 terms & 10050 nodes

If you generate 50000 more terms and 10000 more nodes this changes...

Without patch

Query build time 1.93 ms
Query execute time 116.73 ms
View render time 122.79 ms

With patch

Query build time 715.26 ms
Query execute time 3.17 ms
View render time 723.78 ms

What's interesting here is that the performance with the patch is worse. I've definitely seen this patch improve things but maybe it's not as good as it looks? Or maybe its the shape of the random data. This needs more investigation.

What's interesting here is that the performance with the patch is worse. I've definitely seen this patch improve things but maybe it's not as good as it looks?

I'm pretty sure I know why this is. The sites I've seen this make quite large performance improvements on use memcache or something similar for entity caching. This patch results in load being moved off the database server to memcache for loading the taxonomy terms. And this results in a performance improvement. It feels as though this is going to be a very tricky patch to land as it is in core.

Maybe a way forward is to make the plugins work for both strategies and allow the user to choose which as via plugin configuration. This would get around the UI issue of having multiple plugins and allow us to explain why the option exists.

I've been working on a different approach that uses unions and should result in performance improvements for everyone - see #2133215: Fix IndexTidDepth views argument plugin and TaxonomyIndexTidDepth views filter plugin performance

Hi @everyone,

Taxonomy performance sandbox module creating custom table & Index the taxonomy_term_hierarchy & It give fast query performance for level based terms. I checked this module with depth 10, It also create dynamic table fields when cross depth 10. but views argument maximum depth filter value is 10.

@Jeya sundhar I don't think an additional module is necessary here. It means more tables to maintain and keep in sync. These things don't come for free. As this issue and #2133215: Fix IndexTidDepth views argument plugin and TaxonomyIndexTidDepth views filter plugin performance there is quite a lot that can be done to optimise this query. That I said I think that #2133215: Fix IndexTidDepth views argument plugin and TaxonomyIndexTidDepth views filter plugin performance clearly is the way to go. It results in the same number of queries being sent to the server as HEAD (yes there are more queries on the server because it uses unions) but it is using a much more performant join strategy. This issue also results in good performance on real world sites but it also increases the work done to build the query because it has to load taxonomy entities in order to build the list of terms to select from. This might be cheap when they are cached in memcache or redis but that's not always the case.

I think we should consider marking this issue a duplicate of #2133215: Fix IndexTidDepth views argument plugin and TaxonomyIndexTidDepth views filter plugin performance. I'll do that once that issue has more reviews and more real world testing.

Patch at #80 needs to be re-rolled... We find this patch not applying for 9.3.x for php 7.4

Adding patch for 9.3.x compatibility for PHP 7.3.x
I have re-rolled patch #70 for this as that was the last patch in this thread applying to 9.3.x

Tried to fix custom command failure at #89

Per @alexpott in #85:

I think we should consider marking this issue a duplicate of #2133215: Fix IndexTidDepth views argument plugin and TaxonomyIndexTidDepth views filter plugin performance. I'll do that once that issue has more reviews and more real world testing.

Moving this to postponed so people don't continue to work on it.

I'm sure such a long issue needs work for all tags

Above patch doesn;t contain new files.
Updating patch with new files.
Patch #89 breaks with 9.4 version
Sql query is forming Inner join with 'IN' condition where it '=' in 9.3

Re-rolled patch for D11

Thanks but fixes need to be in MRs vs patches now.

Also was previously tagged for steps which appear to be missing, if those could be added before review

Thanks.

I like performance issues, though alexpott's profiling shows this doesn't do much and #2133215: Fix IndexTidDepth views argument plugin and TaxonomyIndexTidDepth views filter plugin performance may have already eaten most of its value... that said, I moved #100 into an MR and going to make it work so I can test out this and try to repro his results

Switched to DISTINCT subquery rather than the raw table join from patch #100. Needed for correct results but harder to compare against the raw query join used in the earlier profiling.

Benchmarked the MR against HEAD on main branch using devel_generate data: 50,000 terms, 10,000 nodes, 19,771 taxonomy_index rows. The view has a depth 4 contextual filter on a term with 119 descendants. Numbers are averages of 10 runs. Static entity caches were reset between runs. The benchmark views include a nid tiebreak sort because devel-generated nodes share identical created timestamps.

Patch #100 called TermStorage::loadTree(). That method loads the full vocabulary to extract one subtree. It cost ~820 ms here. This matches the 715 ms build time reported in #83.

The MR now expands the hierarchy with one query on taxonomy_term__parent per depth level. Each query touches only the subtree. Build time drops to 5.3 ms. The join subquery selects DISTINCT nids, so nodes tagged with several matching terms are not duplicated. Both handlers return identical result sets.

Attached scripts to reproduce the tests can be run (after file renames remove _.txt and put in a bench/ directory) and assuming you have drush and already have devel module installed for devel_generate to generate the nodes and terms.

Careful with the --kill flag on a real site that will delete all your tags or content!!

drush scr bench/setup.php
drush devel-generate:terms 50000 --bundles=tags --max-depth=4 --kill
drush devel-generate:content 10000 --bundles=article --kill
drush scr bench/run.php

The views also have page displays at /bench-depth/% and /bench-depth-join/% for profiling with xhprof which is in ddev if you're using it.

AI Disclosure: AI assisted in the reading through the comments, collecting the important ones(since there are 100 comments here). I setup the branch with the patch in #100 to kick it off. AI fixed the code to work with D12 on the main branch, created the scripts to profile matching alexpott's earlier benches, then came up with the changes to get a better result. I read through all its output in order to understand it.

Fresh numbers, the earlier depth-4 union in #104 was not a huge diff (could be just my machine on battery for a moment, or other things eating my CPU). Updated picture:

Let's reframe this... at depth 4 the two strategies are effectively the same, the join variant trades ~1 ms more build for ~1.5 ms less execute, a wash within noise. The real win is the depth 9 result (which might not be practical): ~40× faster execute, reproduced across runs with identical result sets.

Because of the age of this issue, and the history, and #2133215: Fix IndexTidDepth views argument plugin and TaxonomyIndexTidDepth views filter plugin performance does most of the fixing for this issue. And because I don't want to mess with the issue more than I have. I will close this as outdated and open a new issue with the proposal.