cloudinary / cloudinary_php

PHP extension for Cloudinary
https://cloudinary.com/documentation/php_integration
MIT License
388 stars 150 forks source link

Noticeably reduced performance with version 2 #244

Open dwightwatson opened 3 years ago

dwightwatson commented 3 years ago

Bug report for Cloudinary PHP SDK

Before proceeding, please update to latest version and test if the issue persists

Describe the bug in a sentence or two.

After upgrading to the new PHP SDK (from 1.x to 2.x) I noticed our 50th percentile drop considerably. I isolated the change and reverted back to 1.x and saw my response times go back to where they were previously. Unfortunately New Relic wasn't running correctly so I didn't manage to capture any better metrics.

image

I've opened this issue to see if anyone else has noticed similar issues. I would not have expected simply upgrading Cloudinary to have an a noticeable impact on response time. Additionally - we only use the PHP SDK for URL generation, not for uploading assets or anything, so it should be pretty negligible.

Issue Type (Can be multiple)

Operating System

Environment and Frameworks (fill in the version numbers)

const-cloudinary commented 3 years ago

@dwightwatson thank you for reporting the issue!

We did run some performance checks on the new version, for example we took this transformation: https://cloudinary.com/documentation/php1_image_manipulation#adding_text_and_image_overlays

And ran it 10000 times in a loop.

On my computer it took: v1 : 2.422s v2: 2.657s

You can see some performance decrease of around 10% for just generating the transformations.

Is that the difference you are experiencing?

If you could provide more details, maybe send a few typical transformations that you use, we could investigate it further.

dwightwatson commented 3 years ago

I'm not entirely sure - I'm waiting on New Relic to get back up and running so I can crunch the numbers there, otherwise I'll see if I can replicate locally. I thought it was worth opening an issue just in case it was a more widespread problem. My use-case is literally just generating signed URLs so I'm surprised there would be any performance change.

grubersjoe commented 2 years ago

We also face this issue. In some payloads we generate lots of image URLs (like several hundreds) and v2 seems to worsen performance considerably. We're also a bit confused why supposedly simple string transformation takes so much time...

atdcloud commented 2 years ago

@grubersjoe Just checking if you can share a code snippet so that we can test it on our end.

dwightwatson commented 2 years ago

I ended up biting the bullet and moving to v2 regardless because of the deprecation warnings from v1. Also realised that using signed vs non-signed URLs was a performance drain (makes sense in hindsight but disappointing nonetheless).