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

improve memory consumption while image processing #2681

Open
cyBerta opened this issue Sep 13, 2021 · 15 comments · Fixed by deltachat/deltachat-ios#2020
Open

improve memory consumption while image processing #2681

cyBerta opened this issue Sep 13, 2021 · 15 comments · Fixed by deltachat/deltachat-ios#2020
Labels
bug Something is not working

Comments

@cyBerta
Copy link
Contributor

cyBerta commented Sep 13, 2021

Using the current iOS app 1.22.1 with core 1.60.0 an image of 9.5 mb sent by deltachat consumes more than 250 MB RAM.
This leads to crashes of the share extension app as described here deltachat/deltachat-ios#1330.

I wonder if the image processing can be memory optimized.
Moreover it would help if there was an API to define a fixed memory limit an image processing task can allocate and if it exceeds to raise an error, the UI can catch. This way we could avoid the situation that iOS throws an EXC_RESOURCE RESOURCE_TYPE_MEMORY exception, which can't be handled by try/catch.

@cyBerta cyBerta added the bug Something is not working label Sep 13, 2021
@r10s
Copy link
Member

r10s commented Sep 14, 2021

background figured out at deltachat/deltachat-ios#1330 (comment): the image in question has about 120 MB raw data - so the mentioned 250 MB needed for recoding seems reasonable.

however, as recoding fails we do not really know how if the 250 MB would be sufficient - would be interested to know the memory consumption for an image with say, 16mb raw data (2000px * 2000px) - if that is 32mb then, i'd say, core cannot do much upon that.

@cyBerta
Copy link
Contributor Author

cyBerta commented Apr 12, 2022

There are ways to load only tiles of an image, scale them and write them back to the memory before loading the next chunk and probably other even more efficient ways I'm not just aware of. Fixing this issue would give a UX boost for sharing larger images on iOS and generally it would help to not being killed under memory pressure.

@link2xt
Copy link
Collaborator

link2xt commented Mar 24, 2023

I have profiled memory consumption of the test suite. When you run cargo test, it outputs something like

Running unittests src/lib.rs (target/debug/deps/deltachat-c710e567f9c3c143)

in the first few lines. This way I find the binary name (target/debug/deps/deltachat-c710e567f9c3c143) and run

$ heaptrack target/debug/deps/deltachat-c710e567f9c3c143

At the end of testing, heaptrack outputs:

heaptrack stats:
        allocations:            20903794
        leaked allocations:     2735
        temporary allocations:  521322
Heaptrack finished! Now run the following to investigate the data:

  heaptrack --analyze ".../deltachat-core-rust/heaptrack.deltachat-c710e567f9c3c143.420312.zst"

heaptrack_gui detected, automatically opening the file...

JPEG-related functions in image recoding are near the top of the memory peak flamegraph:
1

@r10s
Copy link
Member

r10s commented Dec 15, 2023

thanks for the profiling - and sorry for coming back so late.

i am currently on trying to find a solution for deltachat/deltachat-ios#1330 - the core issue seems to be that iOS extensions must not use more than 120 mb in no means

JPEG-related functions in image recoding are near the top of the memory peak flamegraph:

tbh, i have no good idea what that should tell me :) which image size does the benchmark try to decode? a typical panorama may be sth as 16382 x 3690 pixels (tested on iphone7, iphone13) - so 180mb raw data assuming 3 bytes/pixel - i assume that is fully loaded to memory before trying to recode to sth. as 1280 x 288 would be? can you confirm the 3 bytes/pixel?

unless the recoding in core is achieved smarter, my idea is to not pass such large images to core from the share extension but show a message to the user, saying, you can do this in the app (which allows using up to 2000mb peak). alternatively, we could try to pre-scale the image using iOS, that seems to work better (eg. the thumbnails of the huge panoramas is displayed)

there is probably not much that can be improved in core about that and we could close this issue then (which is also no bug then)

@link2xt
Copy link
Collaborator

link2xt commented Nov 16, 2024

Recent report from @lk108

A friend of mine is running 1.48.3-gplay on Android. Sending large images does not work, the message composer aborts (?, probably need to ask for details again) and the log says
11-15 22:08:16.982 19649 19692 🔴 DeltaChat: [accId=1] Failed to send message: image decode failure: Memory limit exceeded
11-15 22:08:16.982 19649 19692 🔴 DeltaChat: Failed to send message: image decode failure: Memory limit exceeded

In this case, the image was 24.33 MB, with a resolution of 16320x12240 (200MP). Is image size a known limitation?

Image was JPEG

So the problem also exists on Android, iOS workaround deltachat/deltachat-ios#2020 only helped on iOS.

@link2xt link2xt reopened this Nov 16, 2024
@link2xt
Copy link
Collaborator

link2xt commented Nov 16, 2024

Maybe related image crate issue: image-rs/image#2184

@link2xt
Copy link
Collaborator

link2xt commented Dec 12, 2024

The solution for this is to check the dimensions of image before resizing by reading its header. If dimensions are unknown or too large, we should not try to load the image into memory as bitmap and resize it, but send it as is.

@iequidoo
Copy link
Collaborator

The solution for this is to check the dimensions of image before resizing by reading its header. If dimensions are unknown or too large, we should not try to load the image into memory as bitmap and resize it, but send it as is.

Probably a UI setting is needed then. It's not clear if the user wants to send big images generating much traffic, maybe they would prefer to get an error by default and retry to send an image as file if needed.

@lk108
Copy link
Contributor

lk108 commented Dec 12, 2024

The friend I mentioned expects DC to automatically resize big images. They know it from other messengers, and it's mentioned in the DC FAQ that "For performance, images are optimized and sent at a smaller size by default". So just sending a 25 MB image as is without warning would be unexpected. Getting an error would be better, but it would still be perceived as a feature that is missing despite being advertised, at least by this one person. Besides, other apps are able to resize the image on their phone, so they wonder why other apps can do it, but DC can't.

Maybe it can't be helped at the moment, but if it depends on the mentioned image crate issue, it's more like a temporary workaround than a final solution?

@link2xt
Copy link
Collaborator

link2xt commented Dec 13, 2024

I don't propose to disable resizing completely, only not try to resize if it has unreasonable resolution like some astronomic images. Anything with resolution produced by phone cameras should be resized, but if someone sends 100000x100000 entirely black PNG with some image in the corner, we should send it as is even if it is slightly larger than normal size because image will fail to process it.

@iequidoo
Copy link
Collaborator

I don't propose to disable resizing completely, only not try to resize if it has unreasonable resolution like some astronomic images. Anything with resolution produced by phone cameras should be resized, but if someone sends 100000x100000 entirely black PNG with some image in the corner, we should send it as is even if it is slightly larger than normal size because image will fail to process it.

I agree that we should look at the image dimensions before trying to resize it, but allowing "slightly larger" (in byte size) images seems not helpful. Rather such a "strange" image would either fit into the byte size limit or exceed it overly, so allowing some extra room here would only add uncertainty (from the user's point of view) about which images can be sent by Delta Chat and which can't. So i'd say we should just return an error for images with big dimensions. If the user actually wants to send it, they can send it as file.

@link2xt
Copy link
Collaborator

link2xt commented Dec 15, 2024

Well, maybe return an error if bitmap size (width x height x bit depth) is higher than 120 mb, referencing https://forums.developer.apple.com/forums/thread/115259 as the source of the limit in the comment.

@lk108
Copy link
Contributor

lk108 commented Dec 15, 2024

So for the user that reported the issue, the image will not be resized, but an error will be shown? They weren't trying to attach something from an external camera, but an image taken by their phone (granted, a high-end model with resolution set to max).

@iequidoo
Copy link
Collaborator

So for the user that reported the issue, the image will not be resized, but an error will be shown?

I think that as a temporary solution it's the best choice. Then we maybe can try to fix it in the image crate, idk, didn't yet dive into it.

@link2xt
Copy link
Collaborator

link2xt commented Dec 15, 2024

So for the user that reported the issue, the image will not be resized, but an error will be shown? They weren't trying to attach something from an external camera, but an image taken by their phone (granted, a high-end model with resolution set to max).

This probably requires not only max resolution, but also making a panorama rather than a normal photo. They can still send it by attaching "as a file" or somehow resize it using their phone tools. We will see if users complain about this error once the limit is there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is not working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants