Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't prime term cache when no posts are passed #2030

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

tyrann0us
Copy link

@tyrann0us tyrann0us commented Dec 18, 2023

Please check if the PR fulfills these requirements

  • The commit message follows our guidelines
  • Tests for the changes have been added (for bug fixes/features)
  • Docs have been added/updated (for bug fixes/features)

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
Bugfix

What is the current behavior? (You can also link to an open issue here)
When using a permanent object cache, any cache priming is unnecessary because the items are most likely already in the permanent cache.

To make matters worse, the Tribe\Utils\Taxonomy::prime_term_cache method has a major performance problem when called with an empty $posts array. This is the case, for example, when no future events are to be displayed in the "Events List" widget.

In this part of the method:

$args = [
'fields' => 'all_with_object_id',
'object_ids' => $ids,
'taxonomy' => $taxonomies,
];
$terms = get_terms( $args );

the following values are passed:

$args = [
  'fields' => 'all_with_object_id',
  'object_ids' => [], // this is the offending part
  'taxonomy'  => ['post_tag', \Tribe__Events__Main::TAXONOMY],
];
$terms = get_terms($args);

This is highly insufficient for systems with a large number of post tags because all post tags will be fetched and passed to _prime_term_caches(). On a project with ~12,000 post tags, we had ~100k requests to the cache.

What is the new behavior (if this is a feature change)?
If the $posts array is empty (i.e. no events are displayed in the widget), bail early and return an empty array. With ~12,000 post tags, we saw a reduction from ~100k requests to ~20k.

Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)
I don't expect this to be a breaking change.

Other information:
There's an incomplete sentence in the function's DocBlock: Important to note that.
Tagging some of the maintainers to get visibility: @lucatume, @bordoni, @Camwyn (sorry for the notification spam)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant