Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Codidact loads images in full size #1133

Open
Taeir opened this issue Jul 21, 2023 · 3 comments · May be fixed by #876
Open

Codidact loads images in full size #1133

Taeir opened this issue Jul 21, 2023 · 3 comments · May be fixed by #876
Labels
area: backend Changes to server-side code priority: high type: change request New feature or request

Comments

@Taeir
Copy link
Contributor

Taeir commented Jul 21, 2023

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.

@Taeir Taeir linked a pull request Jul 21, 2023 that will close this issue
@cellio cellio added area: backend Changes to server-side code type: change request New feature or request priority: high labels Jul 21, 2023
@cellio
Copy link
Member

cellio commented Jul 21, 2023

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
Copy link
Contributor Author

Taeir commented Jul 21, 2023

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
Copy link

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:

  • Requests (i.e., URLs that are embedded in the generated web pages) should be some sort of standardized URL in codidact.com (or whatever the instance's domain is) and not S3
  • Requests should have some way to indicate the desired size (e.g., height and width in pixels)
  • Software servicing the request should either (a) check the database for security/authorization (if needed, I don't think that is an issue with the primary Codidact instance as it is all open for reading anyway), (b) check to see if the file is already on S3 (either by checking for the file directly or by checking a table of images in the database - I don't know if there is such a table right now) in the requested size
  • If the request is valid (no security problems, size is a valid size) then if the file exists, use a redirect to return it; if the file does not exist, create it and save it in S3 and return it directly (since it will already be in memory).

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: backend Changes to server-side code priority: high type: change request New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants