Problem/Motivation

Several problems with media calculations:
Each link to a file found tries to get the media field definition \Drupal\entity_mesh\Repository::getMediaFileByEntityFile

  public function getMediaFileByEntityFile(FileInterface $file): ?MediaInterface {
    $media_types_with_tile_fields = $this->getMediaFieldTypeWithFileFields();
    if (empty($media_types_with_tile_fields)) {
      return NULL;
    }
...
  protected function getMediaFieldTypeWithFileFields() {
    $medias_type_reference_to_file = [];
    $media_types = $this->entityTypeManager->getStorage('media_type')->loadMultiple();
    foreach ($media_types as $media_type) {
      $fields = $this->entityFieldManager->getFieldDefinitions('media', (string) $media_type->id());
      foreach ($fields as $field) {
        if (in_array($field->getType(), ['file', 'image'])) {
          $medias_type_reference_to_file[] = [
            'field_name' => $field->getName(),
            'field_type' => $media_type->id(),
          ];
        }
      }
    }
    return $medias_type_reference_to_file;
  }

The problem is that it loads all media types and definitions each time the method is called (once per link).

But the most problematic code is in the method itself \Drupal\entity_mesh\Repository::getMediaFileByEntityFile, as it introduces a condition on target_id, it can use an index

  public function getMediaFileByEntityFile(FileInterface $file): ?MediaInterface {
    $media_types_with_tile_fields = $this->getMediaFieldTypeWithFileFields();
    if (empty($media_types_with_tile_fields)) {
      return NULL;
    }
    $media_query = $this->entityTypeManager->getStorage('media')->getQuery();
    $media_query->accessCheck(FALSE);
    $or_condition = $media_query->orConditionGroup();
    foreach ($media_types_with_tile_fields as $media_type_file_field) {
      $or_condition->condition($media_type_file_field['field_name'] . '.target_id', $file->id());
    }
    $media_query->condition($or_condition);
    $results = $media_query->execute();
    if (empty($results)) {
      return NULL;
    }
    return $this->entityTypeManager->getStorage('media')->load(reset($results));
  }

Example of query generated:

  SELECT base_table.mid
  FROM media AS base_table
  LEFT JOIN media__field_media_image      ON ...
  LEFT JOIN media__field_media_document   ON ...
  LEFT JOIN media__field_media_audio_file ON ...
  LEFT JOIN media__field_media_video_file ON ...
  -- ...one JOIN for each file field
  WHERE media__field_media_image.field_media_image_target_id      = :fid
     OR media__field_media_document.field_media_document_target_id = :fid
     OR media__field_media_audio_file..._target_id                 = :fid
     OR ...

Steps to reproduce

Try to process nodes from the entity mesh processing batch with multiple media on the same content, it will be slow, and it will take several seconds for each node.

Proposed resolution

As the field map does not change, save it on a property to avoid recalculating it, it will be recalculated once for execution, not by link:
\Drupal\entity_mesh\Repository::getMediaFieldTypeWithFileFields

  protected function getMediaFieldTypeWithFileFields() {
    // The media field map does not change during a run; build it once.
    if ($this->mediaFileFieldsCache !== NULL) {
      return $this->mediaFileFieldsCache;
    }

Do not recalculate files that were already found

  public function getMediaFileByEntityFile(FileInterface $file): ?MediaInterface {
    $fid = $file->id();

    if (array_key_exists($fid, $this->fileToMediaCache)) {
      return $this->fileToMediaCache[$fid] ?: NULL;
    }

Do not recalculate as some bundles could share the same field name.

    $media_storage = $this->entityTypeManager->getStorage('media');
    $media = NULL;
    $queried_fields = [];
    foreach ($media_types_with_tile_fields as $media_type_file_field) {
      $field_name = $media_type_file_field['field_name'];
      // Several media bundles may share a field name; query each once.
      if (isset($queried_fields[$field_name])) {
        continue;
      }

Instead of using a big query with multiple JOINS in the WHERE condition, just make smaller queries, filtering by target_id as the field is indexed, and if it is found, jump out of the foreach.

      $results = $media_storage->getQuery()
        ->accessCheck(FALSE)
        ->condition($field_name . '.target_id', $fid)
        ->range(0, 1)
        ->execute();
      if (!empty($results)) {
        $media = $media_storage->load(reset($results));
        break;
      }

Saving it on the property to avoid recalculating it:

    $this->fileToMediaCache[$fid] = $media ?: FALSE;
    return $media ?: NULL;
  }
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

eduardo morales alberti’s picture

Issue summary: View changes
eduardo morales alberti’s picture

Issue summary: View changes

eduardo morales alberti’s picture

Status: Active » Needs review
eduardo morales alberti’s picture

eduardo morales alberti’s picture

Assigned: lpeidro » Unassigned
lpeidro’s picture

Assigned: Unassigned » eduardo morales alberti
Status: Needs review » Reviewed & tested by the community

Hello Edu:

Reviewed all the changes in the MR, they look correct and will definitely improve performance.

Just one suggestion. Here in the file-to-media cache we are storing the full Media entity:

$this->fileToMediaCache[$fid] = $media ?: FALSE;

I would store the mid instead of the entity. A hard reference here pins the entity even after $storage->resetCache(). Reloading the entity costs almost nothing: entity loads are memory-cached per request, so a load() on a cached mid won't rebuild the entity from scratch.

Thanks

eduardo morales alberti’s picture

@lpeidro Thanks for the review. I don't think storing the mid instead of the entity would be an improvement here, and I think it would actually be slightly worse.

Currently, on a cache hit:
src/Repository.php


if (array_key_exists($fid, $this->fileToMediaCache)) {
  return $this->fileToMediaCache[$fid] ?: NULL;   // one array lookup, done
}

With the mid, since the interface must return ?MediaInterface, we have to load the entity again on every hit:

      if (array_key_exists($fid, $this->mediaIdCache)) {
        $mid = $this->mediaIdCache[$fid];
        if ($mid === FALSE) {
          return NULL;
        }
        return $this->entityTypeManager->getStorage('media')->load($mid);
      }

That load() goes through EntityStorageBase::load() -> loadMultiple() -> key building -> memory cache lookup. Cheap, but it is strictly more work than an array lookup.

The two parts of the argument also cancel each other out:

- If nobody called resetCache(), the media storage memory cache still holds the same object, so storing the mid frees nothing and we paid the extra load().
- If resetCache() did run, we do free the object, but then load() is not "almost nothing": it hits the database again (base + data + field tables) and rehydrates the entity.

There is no resetCache() call anywhere in the module, and entity_mesh.repository is a normal per-request service, so we are in the first case.

On array size specifically: an array slot holding an object is a zval with a pointer, the same footprint as a slot holding an int, so the array itself does not shrink either way.

There is a real optimization nearby, but it is a wider change. The only consumerneeds three scalars (src/EntityRender.php):

      $target->setEntityType($media->getEntityTypeId());  // always 'media'
      $target->setEntityBundle($media->bundle());
      $target->setEntityId((string) $media->id());

We load a full Media entity just to read those and discard it, on every cache miss. Both values are already in the field's dedicated table (which has a bundle column), so the entity load could be skipped entirely:

Example:

      // illustrative; real code would resolve the table/column via TableMappingInterface
      $row = $this->database->select($table, 'f')
        ->fields('f', ['entity_id', 'bundle'])
        ->condition('f.' . $column, $fid)
        ->range(0, 1)
        ->execute()
        ->fetchAssoc();

      $this->fileToMediaCache[$fid] = $row
        ? ['id' => $row['entity_id'], 'bundle' => $row['bundle']]
        : FALSE;

I would not do it in this MR, though: getMediaFileByEntityFile() is public API (RepositoryInterface.php:188) and returns ?MediaInterface, so it needs a new method plus a deprecated wrapper and test changes, it only covers configurable fields with dedicated tables, and it bypasses the Entity API. The gain is also secondary: the main bottleneck was the orConditionGroup query without a range plus rebuilding the field map on every call, which this MR already fixes. Skipping the entity load is minor on normal runs and only measurable with thousands of distinct files, so I would profile the queue worker before committing to it.

Comment generated with the help of IA

eduardo morales alberti’s picture

Status: Reviewed & tested by the community » Fixed

Fixed!

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.

Status: Fixed » Closed (fixed)

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