Optimize width/height calculation for ImageDerivative

[Berdir]

Extracting width/height from the physical image is can be extremely expensive, for example when using a distributed file system.

The code supports ->width/->height properties, but these don't actually exist, it seems they are just used for the test?

What does exist is ->width/->height on a certain image field, so if an image derivative is fetched through that, it could possibly set those or something else when passing the entity along for this. I'm not quite sure how you'd define that though, this is the definition that we have for that:

    $registry->addFieldResolver('Image', 'derivative',
      $builder->compose(
        $builder->produce('property_path')
          ->map('type', $builder->fromValue('field_item:image'))
          ->map('value', $builder->fromParent())
          ->map('path', $builder->fromValue('entity')),
        $builder->produce('image_derivative')
          ->map('entity', $builder->fromParent())
          ->map('style', $builder->compose(
            $builder->fromArgument('style'),
            $builder->callback([get_class($this), 'mapImageStyleEnum'])
          ))
      )
    );

I assume you would need to intercept the entity somehow and set those properties?

Additionally, what's possible is to optionally support the file_entity module for sites that still use it, which is the case for us, that has metadata on the file entity level and and an API to fetch that. That won't be helpful for many sites out there though. That really should be in core, but that's a different topic.

[Berdir]

Note that this isn't about improving things by a few %. based on early tests so far on our integration system on a request with a ton of images and image styles, we're talking about an improvement of 30s => 2s after a cache clear.

[pmelab]

We also encountered that and solved it as you suggested by monkey-patching the properties onto the file entity. Not sure if that could be generalized into a dependency-free solution.

Dimensions are stored on the field instance. Not sure whats the reasoning behind that. But it means that we can't even use that approach in cases where the file is fetched without going through a relation. In these cases we could FileEntity optionally?

[pmelab]

Just curious: Did this ever work? We used image styles in GraphQL heavily, but this problem popped up now.

[Berdir]

No, I don't think this ever worked. I vaguely remember a very old core issue about adding this but I can't find it anymore and only file_entity ever stored that information on the file entity.

It was changed in b0055eb to work but slowly, which is kinda funny, because those two names mean it was added for the exact project that I'm improving atm ;) and it was originally added in 5435b55, which you worked on :)

So that ->width and ->height is just wishful thinking and you made it true in the test.

Care to share how you do that monkey-patching in your project? Might be useful for others.

One way to fetch the information without file_entity would be a reverse-lookup of its usages with file_get_file_references(), but note that that function has bugs with usages in the non-default revision. No idea if graphql supports querying revisions. It would be less efficient I guess but probably still quite a bit faster than parsing it from the file again.

PS: "field instance" is a D7 concept/name, in D8+ it's just field, and the shared bit is "field storage" (which used to be called field in D7)