Problem/Motivation

Follow-up from #3551308: Combine single cardinality fields into a single database query when loading entities.

We can apply the same treatment to multiple cardinality fields as single cardinality fields to save even more database queries.

The UNION ALL in the issue summary of that issue doesn't look necessary, just needs a left join.

Steps to reproduce

Proposed resolution

Load all multiple cardinality fields at once. Looks like:

 EXPLAIN SELECT node_field_data.nid AS nid, node_field_data.langcode AS langcode, node__field_ingredients.field_ingredients_value AS field_ingredients_value, node__field_ingredients.delta AS field_ingredients_delta, node__field_recipe_category.field_recipe_category_target_id AS field_recipe_category_target_id, node__field_recipe_category.delta AS field_recipe_category_delta, node__field_tags.field_tags_target_id AS field_tags_target_id, node__field_tags.delta AS field_tags_delta, node__layout_builder__layout.layout_builder__layout_section AS layout_builder__layout_section, node__layout_builder__layout.delta AS layout_builder__layout_delta FROM node_field_data node_field_data LEFT OUTER JOIN node__field_ingredients node__field_ingredients ON node__field_ingredients.entity_id = node_field_data.nid AND node__field_ingredients.langcode = node_field_data.langcode AND node__field_ingredients.deleted = 0 LEFT OUTER JOIN node__field_recipe_category node__field_recipe_category ON node__field_recipe_category.entity_id = node_field_data.nid AND node__field_recipe_category.langcode = node_field_data.langcode AND node__field_recipe_category.deleted = 0 LEFT OUTER JOIN node__field_tags node__field_tags ON node__field_tags.entity_id =
node_field_data.nid AND node__field_tags.langcode = node_field_data.langcode AND node__field_tags.deleted = 0 LEFT OUTER JOIN node__layout_builder__layout node__layout_builder__layout ON node__layout_builder__layout.entity_id = node_field_data.nid AND node__layout_builder__layout.langcode = node_field_data.langcode AND node__layout_builder__layout.deleted = 0 WHERE (node_field_data.nid IN (1)) AND (node_field_data.langcode IN ("en", "es", "und", "zxx"));
+----+-------------+------------------------------+------------+------+----------------------------------------------+--------------------------------------+---------+-------------+------+----------+--------------------------+
| id | select_type | table                        | partitions | type | possible_keys                                | key                                  | key_len | ref         | rows | filtered | Extra                    |
+----+-------------+------------------------------+------------+------+----------------------------------------------+--------------------------------------+---------+-------------+------+----------+--------------------------+
|  1 | SIMPLE      | node_field_data              | NULL       | ref  | PRIMARY,node__id__default_langcode__langcode | node__id__default_langcode__langcode | 4       | const       |    2 |    40.00 | Using where; Using index |
|  1 | SIMPLE      | node__field_ingredients      | NULL       | ref  | PRIMARY                                      | PRIMARY                              | 5       | const,const |   26 |   100.00 | Using where              |
|  1 | SIMPLE      | node__field_recipe_category  | NULL       | ref  | PRIMARY                                      | PRIMARY                              | 5       | const,const |    1 |   100.00 | Using where              |
|  1 | SIMPLE      | node__field_tags             | NULL       | ref  | PRIMARY                                      | PRIMARY                              | 5       | const,const |    2 |   100.00 | Using where              |
|  1 | SIMPLE      | node__layout_builder__layout | NULL       | ref  | PRIMARY                                      | PRIMARY                              | 5       | const,const |    1 |   100.00 | Using where              |
+----+-------------+------------------------------+------------+------+----------------------------------------------+--------------------------------------+---------+-------------+------+----------+--------------------------+
5 rows in set, 1 warning (0.00 sec)

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3564689

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

catch created an issue. See original summary.

catch’s picture

Status: Active » Needs review

Entity and field module kernel tests all pass, let's see how the rest looks.

catch’s picture

Got some errors on CI I couldn't reproduce locally even with supposedly the same mysql and php combination, but pretty sure it's ordering.

Added a sort on delta ASC for each field table, still uses an index but it requires a temporary table and filesort - probably just on the rows that are returned, but not ideal.

mysql> EXPLAIN SELECT node_field_data.nid AS nid, node_field_data.langcode AS langcode, node__field_ingredients.field_ingredients_value AS field_ingredients_value, node__field_ingredients.delta AS field_ingredients_delta, node__field_recipe_category.field_recipe_category_target_id AS field_recipe_category_target_id, node__field_recipe_category.delta AS field_recipe_category_delta, node__field_tags.field_tags_target_id AS field_tags_target_id, node__field_tags.delta AS field_tags_delta, node__layout_builder__layout.layout_builder__l
field_ingredients node__field_ingredients ON node__field_ingredients.entity_id = node_field_data.nid AND node__field_ingredients.langcode = node_field_data.langcode AND node__field_ingredients.deleted = 0 LEFT OUTER JOIN node__field_recipe_category node__field_recipe_category ON node__field_recipe_category.entity_id = node_field_data.nid AND node__field_recipe_category.langcode = node_field_data.langcode AND node__field_recipe_category.deleted = 0 LEFT OUTER JOIN node__field_tags node__field_tags ON node__field_tags.entity_id =
node_field_data.nid AND node__field_tags.langcode = node_field_data.langcode AND node__field_tags.deleted = 0 LEFT OUTER JOIN node__layout_builder__layout node__layout_builder__l
ayout ON node__layout_builder__layout.entity_id = node_field_data.nid AND node__layout_builder__layout.langcode = node_field_data.langcode AND node__layout_builder__layout.delete
d = 0 WHERE (node_field_data.nid IN (1)) AND (node_field_data.langcode IN ("en", "es", "und", "zxx")) ORDER BY field_ingredients_delta ASC, field_recipe_category_delta ASC, field_tags_delta ASC, layout_builder__layout_delta ASC;
+----+-------------+------------------------------+------------+------+----------------------------------------------+--------------------------------------+---------+-------------+------+----------+-----------------------------------------------------------+
| id | select_type | table                        | partitions | type | possible_keys                                | key                                  | key_len | ref         | rows | filtered | Extra                                                     |
+----+-------------+------------------------------+------------+------+----------------------------------------------+--------------------------------------+---------+-------------+------+----------+-----------------------------------------------------------+
|  1 | SIMPLE      | node_field_data              | NULL       | ref  | PRIMARY,node__id__default_langcode__langcode | node__id__default_langcode__langcode | 4       | const       |    2 |    40.00 | Using where; Using index; Using temporary; Using filesort |
|  1 | SIMPLE      | node__field_ingredients      | NULL       | ref  | PRIMARY                                      | PRIMARY                              | 5       | const,const |   26 |   100.00 | Using where                                               |
|  1 | SIMPLE      | node__field_recipe_category  | NULL       | ref  | PRIMARY                                      | PRIMARY                              | 5       | const,const |    1 |   100.00 | Using where                                               |
|  1 | SIMPLE      | node__field_tags             | NULL       | ref  | PRIMARY                                      | PRIMARY                              | 5       | const,const |    2 |   100.00 | Using where                                               |
|  1 | SIMPLE      | node__layout_builder__layout | NULL       | ref  | PRIMARY                                      | PRIMARY                              | 5       | const,const |    1 |   100.00 | Using where                                               |
+----+-------------+------------------------------+------------+------+----------------------------------------------+--------------------------------------+---------+-------------+------+----------+-----------------------------------------------------------+
5 rows in set, 1 warning (0.00 sec)

However, that does fix all the test failures (except for the obvious performance test failures because those aren't updated yet) on CI.

catch’s picture

Issue summary: View changes

We don't need to do #4 because we can ksort() the results. MR should be green now.

Since the logic here is very similar, it should actually be possible to combine the single and multiple cardinality queries into one as well. However once we do that, we end up with all columns for all fields in all rows of the result, which would slightly increase the risk of hitting max_allowed_packet as discussed in the previous issue. I think that all that's necessary for this is removing the split and using the multiple cardinality logic only, might give it a go in a separate MR.

edit: tried this, but there is indeed too much duplication in the results, let's stick with the original idea here.

catch changed the visibility of the branch 3564689-combine-all to hidden.

catch’s picture

Issue summary: View changes

MR is green and ready for review.

After this, we can do #3561960: Combine the database queries in ::loadFromSharedTables() and ::loadFromDedicatedTables() which should remove one more query-per-entity-load (e.g. probably 3-5 queries per page in core tests), which would mean all entities are loaded with a maximum of three queries - the initial load query, the single cardinality fields, and the multiple cardinality fields.

catch’s picture

Issue tags: +gander
catch’s picture

Issue tags: +Needs change record

This can go on the same change record as https://www.drupal.org/node/3562172 - tagging to update that assuming this lands for 11.4.x

catch’s picture

catch’s picture

Priority: Normal » Major

Nearly 10% reduction in queries on the 'front page cool cache' performance test justifies major I think.

godotislate’s picture

Looks like build failed on composer authentication failure to github, probably just needs a re-run.

One question on the MR about the performance test for now. Quick review of the rest seems to make sense, but still needs a more in depth look later.

catch’s picture

Kicked off a fresh pipeline and tried to answer the question on the MR.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.

godotislate’s picture

A couple more questions on the MR, but I think this is close.

godotislate’s picture

Status: Needs review » Reviewed & tested by the community

lgtm!

alexpott’s picture

Version: main » 11.x-dev
Status: Reviewed & tested by the community » Fixed

Committed and pushed b68de1a5f60 to main and c840c458bc3 to 11.x. Thanks!

Time to update the CR.

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

  • alexpott committed c840c458 on 11.x
    perf: #3564689 Combine multiple cardinality field loading into a single...

  • alexpott committed b68de1a5 on main
    perf: #3564689 Combine multiple cardinality field loading into a single...
catch’s picture

catch’s picture

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.