Memory leak by trying to load ALL entities at once for a simple count

Thanks for raising this issue.
I looked into the source and, interesting:

The DefaultSelection plugin uses a memory efficient count query, while the TermSelection plugin falls back to filtering entities in-memory, but only when no match operator is given. Strange and i can not see how this makes sense.

Please file a core issue for this and crosslink it. The count is needed on IEF's side.

\Drupal\Core\Entity\Plugin\EntityReferenceSelection\DefaultSelection::countReferenceableEntities

  public function countReferenceableEntities($match = NULL, $match_operator = 'CONTAINS') {
    $query = $this->buildEntityQuery($match, $match_operator);
    return $query
      ->count()
      ->execute();
  }

\Drupal\taxonomy\Plugin\EntityReferenceSelection\TermSelection::countReferenceableEntities

  public function countReferenceableEntities($match = NULL, $match_operator = 'CONTAINS') {
    if ($match) {
      return parent::countReferenceableEntities($match, $match_operator);
    }

    $total = 0;
    $referenceable_entities = $this->getReferenceableEntities($match, $match_operator, 0);
    foreach ($referenceable_entities as $entities) {
      $total += count($entities);
    }
    return $total;
  }

Hi Team, Checking in if there was any core issue created.
The same issue is happening with us as well causing pages to crash where heavy use of IEF.

Ditto here. I don't even believe my case constitutes "heavy use", but it's causing server errors when attempting to edit a certain node type, and I can confirm that the line of code referenced here is responsible.

It's a pretty nasty bug, so if there's no core solution then I think IEF should try to work around this issue another way? I have no immediate insight into how, although I'm very curious to know whether there would be any serious problems with the following change:

- $have_multiple_existing_entities = $handler->countReferenceableEntities() > 1;
+ $have_multiple_existing_entities = TRUE;

(and when I say "serious problems", bear in mind that our baseline is already "100% reliable server error" with the current code.)

This patch implements the workaround suggested in #4. It is not a general fix, and may not be appropriate for all users; but if the current code is killing your server, it's going to be an improvement.

In my case I have no entity reference fields which require a value, and so the entire purpose of the code which triggers the server errors is moot.

For reference, the relevant logic is as follows, and it's all in service of that final $may_remove value.

        /** @var \Drupal\Core\Entity\EntityReferenceSelection\SelectionInterface $handler */
        $handler = $this->selectionManager->getInstance($options);
        $have_multiple_existing_entities = $handler->countReferenceableEntities() > 1;
[...]
        // Determine if a reference may be removed.
        // Unless the user has permission to delete the entity, then they should
        // not be able to remove it if that will lead to its deletion.
        $may_remove_existing = $settings['removed_reference'] !== self::REMOVED_DELETE || $entity->access('delete');

        // Don't allow a user to remove the only entity if an entity is required
        // and the user cannot replace the entity if they remove it, because
        // this would put the form in an unrecoverable state.
        $can_replace_last_reference = $allow_new || ($allow_existing && $have_multiple_existing_entities);
        $reference_is_not_required = !$element['#required'] || $entities_count > 1 || $can_replace_last_reference;

        // Unsaved entities may always be removed.
        $may_remove = empty($entity_id) || ($may_remove_existing && $reference_is_not_required);

In principle that could all be refactored so that the expensive thing only happens if necessary, after establishing all of the cheap tests. This should be a worthwhile improvement (at a slight cost of readability) -- it would mean some users would simply never encounter this issue at all (when they will do with the current code), which represents a performance improvement even in cases where that count query wasn't taking down the server.

In principle that could all be refactored so that the expensive thing only happens if necessary, after establishing all of the cheap tests. This should be a worthwhile improvement (at a slight cost of readability) -- it would mean some users would simply never encounter this issue at all (when they will do with the current code), which represents a performance improvement even in cases where that count query wasn't taking down the server.

And here's a patch (untested) which does that.

The idea of this one is not to change the intended behaviour of the module, but defer that count query until absolutely necessary, meaning that it doesn't happen if it doesn't need to happen.

If I haven't messed up, I think this could be committed to the current code base as a general performance improvement which would also bypass the major problem in some cases.