Improve Performance

I've validated patch #8 and the whole approach: everything said above makes sense and works. Thank you @slashrsm for working on this.

I'm attaching a re-rolled #8 with prefixed cache id name and code formatted according to Drupal coding standards.

Unfortunately, this module in the current state will most likely have a significant performance impact on a site with more than 1000 nodes, especially for content editors.

I think this patch breaks \Drupal\permissions_by_entity\Service\AccessChecker since the constructor changed. Anyway, it blew up on me, but I made a lot of changes, so need to do some additional testing.

FYI, there is nothing to test against. Please review.

Even with these changes applied, looking at New Relic, \Drupal\permissions_by_entity\Service\AccessChecker::isAccessControlled is still taking too much time.

Looking at the code:

 public function isAccessControlled(FieldableEntityInterface $entity, bool $clearCache = TRUE): bool {
    if ($clearCache) {
      $this->checkedEntityCache->clear();
    }

    if ($entity->getEntityTypeId() == 'node') {
      // Make sure to leave nodes to the permissions_by_term module.
      return FALSE;
    }

Maybe rearranging these two so that cache is not cleared before the node check might help. Also, I am wondering if we always need to clear all the cache for every entity every time access is checked?

Also it looks like:

// Iterate over the fields the entity contains.
    foreach ($entity->getFields() as $field) {
      // We only need to check for entity reference fields
      // which references to a taxonomy term.
      if (
        $field->getFieldDefinition()->getType() == 'entity_reference' &&
        $field->getFieldDefinition()->getSetting('target_type') == 'taxonomy_term'
      ) {
        // Iterate over each referenced taxonomy term.
        /** @var \Drupal\Core\Field\FieldItemInterface $item */
        foreach ($field->getValue() as $item) {
          if(!empty($item['target_id']) && $this->isAnyPermissionSetForTerm($item['target_id'], $entity->language()->getId())) {
            return TRUE;
          }
        }
      }
...

We never cache this result whether true or not (and by extension, we never check cache).

Maybe changing to something like:

  /**
   * {@inheritdoc}
   */
  public function isAccessControlled(FieldableEntityInterface $entity, bool $clearCache = TRUE): bool {
    if ($entity->getEntityTypeId() == 'node') {
      // Make sure to leave nodes to the permissions_by_term module.
      return FALSE;
    }

    if ($clearCache) {
      $this->checkedEntityCache->clear();
    }
    elseif ($this->checkedEntityCache->isChecked($entity)) {
      return TRUE;
    }

    // Iterate over the fields the entity contains.
    foreach ($entity->getFields() as $field) {
      // We only need to check for entity reference fields
      // which references to a taxonomy term.
      if (
        $field->getFieldDefinition()->getType() == 'entity_reference' &&
        $field->getFieldDefinition()->getSetting('target_type') == 'taxonomy_term'
      ) {
        // Iterate over each referenced taxonomy term.
        /** @var \Drupal\Core\Field\FieldItemInterface $item */
        foreach ($field->getValue() as $item) {
          if(!empty($item['target_id']) && $this->isAnyPermissionSetForTerm($item['target_id'], $entity->language()->getId())) {
            // Add entity to cache.
            $this->checkedEntityCache->add($entity);
            return TRUE;
          }
        }
      }
...

This way we can short circuit the entire recursive access check.

Added langcode to cache tags to support multilingual.

fix to commit #13 patch

Reroll the patch to ensure compatibility with the most recent version.

Tried the patch from #16, minimal performance gains on our site with thousands of nodes. Switched to Taxonomy Access Control Lite →
instead with minimal configuration changes required. Works the same, just faster!