Uploaded image for project: 'eZ Publish / Platform'
  1. eZ Publish / Platform
  2. EZP-29661

Performance of ez_image_alias twig function

    Details

    • Sprint:
      Candidates for next sprint

      Description

      Since 6.13.4 and this pull request (https://github.com/ezsystems/ezpublish-kernel/pull/2325) I determined some problems with ez_image_alias:

      In case that only the uri of an image alias is used in a template but not the height or width or any other variation information the twig function ez_image_alias can be very unperformant as it loads the corresponding binary file (ImagineAwareAliasGenerator.php(85)).
      Imagine there is a slider or gallery which uses srcset/sizes attributes: This leads to many calls of ez_image_alias and can result in an error (PHP Fatal error: Uncaught ImagickException: cache resources exhausted).

      Apart from implementing a custom twig function which only returns the uri of a variation the much consistent solution - in my opinion - would be to call ez_image_alias with a parameter. This parameter could take one or a list of particular variation properties like "uri", "fileName", "imageId" or "mimeType". In class ImagineAwareAliasGenerator it could be checked if it is necessary to load the variation file for returning the wanted properties or if it isn't necessary.

        Activity

        Hide
        Sylvain Guittard added a comment -
        Show
        Sylvain Guittard added a comment - ping Andrzej Longosz
        Hide
        Andrzej Longosz added a comment - - edited

        Hello. I'm very sorry for the long delay on this one.

        PHP Fatal error: Uncaught ImagickException: cache resources exhausted

        I need to clarify here one thing - the performance issue occurs in ImageMagick extension implementation itself, not in eZ Platform codebase.

        Apart from implementing a custom twig function which only returns the uri of a variation the much consistent solution - in my opinion - would be to call ez_image_alias with a parameter. This parameter could take one or a list of particular variation properties like "uri", "fileName", "imageId" or "mimeType". In class ImagineAwareAliasGenerator it could be checked if it is necessary to load the variation file for returning the wanted properties or if it isn't necessary.

        What you're proposing does not follow DDD practices. ez_image_alias calls underlying Image Variation Service which returns Domain Driven ValueObject, which is well-defined structure. While technically this might be possible, because of VO's internal implementation, it can lead to many unexpected errors.

        If you don't need to load an image width and height at all, the best bet is to decorate ezpublish.image_alias.imagine.variation.imagine_alias_generator Service and override getVariation method with the implementation that doesn't do that. To be on the safe side, some unit tests coverage would be required, as there is no explicit BC guarantee on that service.

        On our end possible solutions:
        1. Add an extra parameter e.g. `preloadImage` which will prevent image processing. This might inherit problems mentioned above but feels a little bit cleaner than a list of properties. width and height would be null in this case.
        2. Make width/height lazy. This might require redesigning chain of VariationHandler services though, so much more complicated.
        3. Store image variant dimensions in the database.

        We'll try to discuss this internally and decide how to proceed.

        Show
        Andrzej Longosz added a comment - - edited Hello. I'm very sorry for the long delay on this one. PHP Fatal error: Uncaught ImagickException: cache resources exhausted I need to clarify here one thing - the performance issue occurs in ImageMagick extension implementation itself, not in eZ Platform codebase. Apart from implementing a custom twig function which only returns the uri of a variation the much consistent solution - in my opinion - would be to call ez_image_alias with a parameter. This parameter could take one or a list of particular variation properties like "uri", "fileName", "imageId" or "mimeType". In class ImagineAwareAliasGenerator it could be checked if it is necessary to load the variation file for returning the wanted properties or if it isn't necessary. What you're proposing does not follow DDD practices. ez_image_alias calls underlying Image Variation Service which returns Domain Driven ValueObject, which is well-defined structure. While technically this might be possible, because of VO's internal implementation, it can lead to many unexpected errors. If you don't need to load an image width and height at all, the best bet is to decorate ezpublish.image_alias.imagine.variation.imagine_alias_generator Service and override getVariation method with the implementation that doesn't do that. To be on the safe side, some unit tests coverage would be required, as there is no explicit BC guarantee on that service. On our end possible solutions: 1. Add an extra parameter e.g. `preloadImage` which will prevent image processing. This might inherit problems mentioned above but feels a little bit cleaner than a list of properties. width and height would be null in this case. 2. Make width/height lazy. This might require redesigning chain of VariationHandler services though, so much more complicated. 3. Store image variant dimensions in the database. We'll try to discuss this internally and decide how to proceed.

          People

          • Assignee:
            Unassigned
            Reporter:
            Holger Marx
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated: