Cms: Performance improvement

[ostark]

I've noticed some expensive calls during a common request (on Happy Lager Demo / index). Beside the obvious DB related calls, this guy craft\helpers\StringHelper::toLowerCase was called about 1,500 times.

It happens in craft\services\Images::getSupportedImageFormats() when using ImageMagick.

The math: 213 formats x 7 Images (I guess).

---

Fix

By replacing [StringHelper::class, 'toLowerCase'] with strtolower here, I was able to save 24ms or 14% of the wall time.

See the comparison on blackfire.io.

---

Additional info

• PHP version: 7.1.5
• MySQL: 5.7.11-log
• Imagick 3.4.3, ImageMagick 6.7.7-10
• Craft Pro 3.0.0-beta.26

[angrybrad]

Seems perfectly safe since that method doesn't return any multi-byte characters. http://phpimagick.com/Imagick/queryFormats

Thanks @ostark!

[ostark]

@takobell
There is more room for improvements (I hope I'm not too annoying :-)),
https://gist.github.com/ostark/a7060b48e8d4ebfac9c4240a4bd248fd

I do understand if you keep it as it is. But for users with many image transforms on a site, this can have an impact > some milliseconds with 7 images.

Edit:
There is no need to use array_unique at all, since at the end of canManipulateAsImage is just a in_arraycheck and $format has no further use.

[angrybrad]

@ostark not annoying at all. Guessing that whole method could be replaced with this safely:

public static function canManipulateAsImage(string $extension): bool
{
    $formats = Craft::$app->getImages()->getSupportedImageFormats();
    $formats[] = 'svg';

    return in_array(strtolower($extension), $formats, true);
}

[angrybrad]

Really, no need for the strict in_array type check, either. It'll all be strings. Look safe @andris-sevcenko?

public static function canManipulateAsImage(string $extension): bool
{
    $formats = Craft::$app->getImages()->getSupportedImageFormats();
    $formats[] = 'svg';

    return in_array(strtolower($extension), $formats);
}

[andris-sevcenko]

@takobell Looks good. You can skip the third argument if you enjoy your IDE complaining about you not being paranoid enough, though.

[angrybrad]

@ostark https://github.com/craftcms/cms/commit/e392d171bcc84a0e5175998e157b20a42ce7d14e

[khalwat]

SVGs are supported as manipulatable images now?

[andris-sevcenko]

@khalwat they have been since Craft 2.5.

[khalwat]

@andris-sevcenko weird, were they temporarily not in some build of Craft 3? I could swear that the special-case adding of svg was newly added.

[andris-sevcenko]

@khalwat yeah. That's because yours truly messed up by not adding it the first time, since SVG is always supported and it shouldn't have mattered what GD or Imagick says.