codidact / qpixel

Q&A-based community knowledge-sharing software
https://codidact.com
GNU Affero General Public License v3.0
385 stars 69 forks source link

Codidact loads images in full size #1133

Open Taeir opened 1 year ago

Taeir commented 1 year ago

Describe the bug Even when displaying a user icon in a tiny fashion (e.g. as 48x48 in an image card on a post), the user is sent to load the full whatever-uploaded-size (max 2MB) image.

How to reproduce Look at image load times on the users page, and see that the browser is struggling to shrink all the images quickly enough (and if its your first time at that community, it's probably also needing to fetch quite a lot of data).

Expected behavior Qpixel could serve a much smaller version of the image.

Additional context This issue would be solved by #876 . However, in the current setup for the codidact communities, QPixel generates a 301 permanent redirect for image files directly to the S3 bucket. This means that the browser will attempt to fetch from codidact servers once, gets redirected and then always directly from s3.

For regular users, most of the time the browser will pull the images out of its cache, but especially first time viewing a users profile picture can pull in the large image file.

The pull request changes that behavior to server side generate a smaller sized image that fits the display dimensions. So if it is displayed as 48x48 pixels, no need to send more than that to the client. This reduces request sizes a lot. The smaller variant will be stored as well (in Codidacts case, in S3).

Now it does incur some additional storage costs (though the smaller images are likely significantly smaller than the source images). Every profile picture will eventually be stored in all variants displayed on the site). Additionally, it changes the redirection behavior to temporary redirects which may make the client to send more requests to codidact servers for these images which will then get redirected to S3. The question is if this is sufficient or will be a problem for expenses.

Another thing to think about, perhaps we should start reducing the size of the uploaded file directly after upload to a lower quality (if it is high res)? We never need more than 256x256, so we could downscale even the original to 512x512 for example.

cellio commented 1 year ago

Unless we were generating hundreds of copies of the same image at different sizes (which I very much doubt), I'm not concerned about the additional storage costs to be able to serve smaller images and thus improve performance.

I don't know how to answer the possible concern about extra requests. What order of magnitude do we expect the change to be? (I know it depends on user behavior so this would only be a guess.)

Taeir commented 1 year ago

If the browser does maximum caching, it would go to 1 extra request for each image (though handled relatively quickly with just a redirection). If the browser does not respect the 301 and just rechecks anyway, then I guess it wouldn't make a difference actually.

manassehkatz commented 1 year ago

100% agreed. Really this should be the case for all images, but profiles are a good place to start. To recap, at least the way I think it should work:

Along with this, images when initially uploaded should be compressed to a desired maximum size (systemwide configuration, though for post images (as opposed to profiles) possibly community-specific - e.g., photography may want to allow higher quality images to be saved despite the storage and bandwidth effects) and at the preferred (again system or community configuration, but much smaller for Profile images than for post images) maximum height/width. Done right, that can allow people to upload much larger images (e.g., 10 Meg. ) while limiting storage to a reasonable size (say 2 Meg. for a post image, 100k for a Profile image). You'd be amazed how many non-geeks can't figure out how to upload an image except at the default super-high-resolution of their latest smart phone. This causes problems Someplace Else on a regular basis.