In http://drupal.org/node/324313#comment-1117795, Damien raised the point that we're adding a query for individual nodes when loading taxonomy terms. I realised I've not written up the full consequences of this point anywhere, so opening a new issue for it.

In #306224: EOL Taxonomy sprint: add proper taxonomy term hooks, we added hook_taxonomy_term_load() - but many taxonomy functions (nodeapi, taxonomy_get_tree) simply do their own queries on the database and load a 'term object' from there - bypassing taxonomy_term_load() and hence the hook completely. Without a hook, it made sense to do the direct query for performance reasons, but now we have it, we're left with the prospect of inconsistent term objects, or loading every term in it's own query.

Take the front page with 30 nodes and 5 terms per node. With 30 node loads, each taxonomy_nodeapi_load() does a single query to grab the 5 terms, 30 queries in total.

If we convert taxonomy_nodeapi to use taxonomy_term_load() for loading terms (so that modules like taxonomy_image can have their data accessed reliably on node listings for example), then that's 150 queries using taxonomy_term_load() itself +150 for every hook implementation for each term. Not nice.

For an individual node with 5 terms, in HEAD we use one query, it'd be 5 (+5) queries using taxonomy_term_load() (essential for modules like taxonomy_image).

With #324313: Load multiple nodes and terms at once, we have 2 queries, + one extra query for each hook implementation, regardless of whether it's a node listing or individual node view.

Assuming multiple load gets in, to allow modules to take full advantage of the API, and avoid themers etc. loading full term objects again when they've been partially loaded from the database, we should convert all places where 'term objects' are loaded in taxonomy.module to use taxonomy_term_multiple_load(). This should be pretty straightforward in most cases, except taxonomy_get_tree - which needs a ground up rewrite, I have the beginnings of a plan for this which would allow it to load proper term objects and need to write up elsewhere.

Marking this as postponed on the other patch, but I'll want to get onto it asap once that's in.

CommentFileSizeAuthor
#1 multiple_load_conversion.patch6.84 KBcatch

Comments

catch’s picture

Status: Postponed » Needs review
StatusFileSize
new6.84 KB

Forgot about this issue, marked #343788: Taxonomy module doesn't use it's own APIs as duplicate.

Attached patch converts taxonomy_get_term_by_name() and taxonomy_select_nodes(). Comes with new tests for taxonomy_get_term_by_name().

catch’s picture

Status: Needs review » Closed (duplicate)

Actually makes sense to keep these relatively separate.