Make file usage faster [#2092693]

Followup from: #2073033: Optimize file usage updates in file/image fields (comments #14 and #15).

We want to figure out if files usage mechanism can be optimized. There are two proposals that needs investigation first:

Add addMultiple() and deleteMultiple() to \Drupal\file\FileUsage\FileUsageInterface. Those new methods should accept a list of files as first argument and same other arguments. Then, in DatabaseFileUsageBackend, those methods should use optimized SQL statements like multi-record INSERTs. Basically we want to run a single multi-row SQL query instead of running a single-row query multiple times when updating the usage for more than one file once.
Investigate the possibility to replace the first argument of ::add() and ::delete() (+multiple) methods with FIDs instead of full file objects. This is applicable also for [add|delete]Multiple() new methods.

Comments

Comment #1

claudiu.cristea

Romanian

Arad 🇷🇴

CreditAttribution: claudiu.cristea commented 19 September 2013 at 05:32

Issue tags:

+Performance

Tagging.

Comment #2

Berdir

German

Switzerland

CreditAttribution: Berdir commented 19 September 2013 at 06:14

The full objects are needed because file usage also controls the file status.

Comment #3

yched CreditAttribution: yched commented 19 September 2013 at 15:42

@Berdir: yes, it just seems weird that acting on the status is the job of FileUsage ?
But yes, that might be a bit much to refactor here.

Comment #4

Berdir

German

Switzerland

CreditAttribution: Berdir commented 19 September 2013 at 15:56

That's how it used to be, but that makes using this service so much more complicated to use, see https://api.drupal.org/api/drupal/modules%21file%21file.field.inc/functi....

And it doesn't change much. We still have to do this temporary/permanent thing, it would just be moved to the code that calls the API.

Comment #5

yched CreditAttribution: yched commented 19 September 2013 at 16:01

Agreed, nevermind. Moving the methods to multiple would be good enough here.

Comment #6

claudiu.cristea

Romanian

Arad 🇷🇴

CreditAttribution: claudiu.cristea commented 28 September 2013 at 06:58

Assigned:

Unassigned

» claudiu.cristea

Taking this.

Comment #7

claudiu.cristea

Romanian

Arad 🇷🇴

CreditAttribution: claudiu.cristea commented 28 September 2013 at 07:15

Status:

Active

» Postponed

And postponing till #2073033: Optimize file usage updates in file/image fields as it has to act on FileField rather than FileItem.

Comment #8

Berdir

German

Switzerland

CreditAttribution: Berdir commented 28 September 2013 at 07:23

Are we sure that multiple is actually going to be faster? I guess the only improvement would be a single select to get the current usages, but then we need separate updates/deletes/inserts/file saves anyway?

Not sure if it's really worth an API addition...

Comment #9

claudiu.cristea

Romanian

Arad 🇷🇴

CreditAttribution: claudiu.cristea commented 28 September 2013 at 07:42

I guess the only improvement would be a single select to get the current usages,...

This is the first improvement.

...but then we need separate updates/deletes/inserts/file saves anyway?

On an addMultiple() there will be no more than 3 queries, at least 2:

The SELECT you had mention
An UPDATE -- if there are files with usage records.
An INSERT -- for files not having usage records

For an entity ref field with 3 items the existing merge query will perform 6 queries, while this patch will perform 2 or 3 queries.

Here's a first idea of addMultiple():

  public function addMultiple(array $files, $module, $type, $id, $count = 1) {
    if (!$files) {
      return;
    }
    $fids = array_map(function ($file) { return $file->id(); }, $files);

    // Build the condition object.
    $condition = new Condition('AND');
    $condition
      ->condition('fid', $fids, 'IN')
      ->condition('module', $module)
      ->condition('type', $type)
      ->condition('id', $id);

    $existing_fids = $this->connection->select($this->tableName)
      ->fields($this->tableName, array('fid'))
      ->condition($condition)
      ->execute()
      ->fetchCol();

    // Update existing usage records.
    if ($existing_fids) {
      $this->connection->update($this->tableName)
        ->expression('count', 'count + :count', array(':count' => $count))
        ->condition($condition)
        ->execute();
    }

    // Insert new usage records.
    $new_fids = array_diff($fids, $existing_fids);
    if ($new_fids) {
      $query = $this->connection->insert($this->tableName)
        ->fields(array('fid', 'module', 'type', 'id', 'count'));
      foreach ($new_fids as $fid) {
        $query->values(array(
          'fid' => $fid,
          'module' => $module,
          'type' => $type,
          'id' => $id,
          'count' => $count,
        ));
      }
      $query->execute();
    }

    parent::addMultiple($files, $module, $type, $id, $count);
  }

Also I didn't dropped totally the idea of passing FIDs instead file objects. That is really absurd what is going on now. Running an update query for each file (I mean $file->setPermanent()) when we can do that in a single cheap query. I'm thinking that FileStorageController can implement bulk status set for files while it's really a storage controller job (not the model should implement that).

Comment #10

claudiu.cristea

Romanian

Arad 🇷🇴

CreditAttribution: claudiu.cristea commented 28 September 2013 at 08:44

Issue tags:

+needs profiling

Needs profiling

Comment #11

mgifford

he/him

English

CreditAttribution: mgifford commented 1 October 2014 at 17:40

Issue summary:	View changes
Status:	Postponed	» Needs work
Related issues:		+#2073033: Optimize file usage updates in file/image fields

Comment #12

1 October 2014 at 17:40

Version:

8.0.x-dev

» 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Comment #13

1 October 2014 at 17:40

Version:

8.1.x-dev

» 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Comment #14

1 October 2014 at 17:40

Version:

8.2.x-dev

» 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Comment #15

1 October 2014 at 17:40

Version:

8.3.x-dev

» 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Comment #16

1 October 2014 at 17:40

Version:

8.4.x-dev

» 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Comment #17

1 October 2014 at 17:40

Version:

8.5.x-dev

» 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.