OOM with the new sheet approach #275

rubengees · 2018-03-29T11:11:21Z

Version of the library: Current SNAPSHOT
Affected devices: Low memory devices
Affected versions: Potentially all

I ran into an OOM with one of my old devices when opening an Activity with an EmojiTextView. This is due to the emoji Bitmap being really large when decoded. I can't really use it in production like that sadly.

I tried:

Only decoding the part of the sheet needed with BitmapRegionDecoder: Way too slow.
Using a different decoder for the same purpose: Way too slow.

I can't come up with a solution at this point. Maybe someone else has an idea? Otherwise: Should we revert?

The text was updated successfully, but these errors were encountered:

shi2134 · 2018-03-29T11:26:25Z

Before the png image is 72px, change it to 64px, also can reduce the library size.

vanniktech · 2018-03-29T11:33:42Z

There must be an option to work with Bitmaps. Everyone is doing that. Whatsapp, Slack etc. Maybe there's a magic flag somewhere?

daniele-athome · 2018-06-21T18:05:03Z

Some of my users are having OOM too.

Signal for Android is using a few methods which might help:

they used one sheet per category
they somewhat scaled the bitmap to save memory:

Bitmap scaledBitmap = Bitmap.createScaledBitmap(originalBitmap, (int)(originalBitmap.getWidth() * decodeScale), (int)(originalBitmap.getHeight() * decodeScale), false);

Details here: https://github.com/signalapp/Signal-Android/blob/master/src/org/thoughtcrime/securesms/components/emoji/parsing/EmojiPageBitmap.java#L82

I'll do some tests and see if I can detect a memory saving using profiling tools in Android Studio, but I don't have a low-end device to try them.

rocboronat · 2018-07-01T07:46:59Z

Hi guys! Just noticed this issue.

Some years ago, I had same issue dealing with fullscreen backgrounds. They were too much for first price devices.

The way we solved it was by using different .png's for different densities. You know: xxhdpi, xhdpi... Devices with low memory tend to be devices with low density, so using it saves so much memory. They started using low density backgrounds, and they had half size than originals, so it saved 75% of memory.

Right now, the icons are only inside the nodpi folder. Do you think placing it in different density folders might help to fix OOM error? If so, I could do some Photoshop boring and repetition based tasks 😇

vanniktech · 2018-07-01T08:01:21Z

It could help but then the library size would go up again which was the entire point of having a single sheet. Also if we scale the images down they might become blurry. Maybe one sheet per category is the way to go.

rocboronat · 2018-07-01T08:33:11Z

Oh, the idea was to scale down images but without compromising final quality, just the needed to fit well on such density.

Right now, I guess that emojis have the size to be shown great on our developer high end devices. So, it's a waste of memory when a common user has common low-end device.

I also guess (I'm new in the project, I'm always guessing) that one reason to only have the nodpi folder is because a developer can choose to show emojis in a very big size, and that's what you are afraid of when talking about loosing resolution.

Well, to fix it, if one app dev chooses to show the icons in a very big size, the library is also able to get emojis from a bigger density folder by using mipmap folders.

More thoughts...?

rocboronat · 2018-07-01T08:36:41Z

Oh, I forgot the apk size question!

About the size of the library, if you really want to make it SMALL we should put all emojis in a server and download them as soon as possible, using Glide or something like that.

Another good think about it is that Glide will resize emojis according to the user screen before showing them, and also using 16 bits instead of 32 for further memory saving.

vanniktech · 2018-07-01T08:42:07Z

Then we're just rebuilding the emoji support from Google. Which we have a module for to leverage that. Glide and downloading could be an option for another module but the main approach should be having the emojis as images.

rocboronat · 2018-07-01T08:53:30Z

Sorry, I don't get why using mipmap folders will be just rebuilding Google's emoji support... You're putting there great icons and also showing the emojis keyboard, so...

I think you have only read one of my two comments 😅

vanniktech · 2018-07-01T09:49:05Z

About the size of the library, if you really want to make it SMALL we should put all emojis in a server and download them as soon as possible, using Glide or something like that.

That's rebuilding the emoji font provider from Google.

Previously we didn't get the emojis in a bigger size so we just used nodpi and it worked for everyone. Already now on some high resolution devices they are blurry.

rocboronat · 2018-07-05T11:00:33Z

@vanniktech yesterday I did some experiment with having a half size .png file, and it saves like 50% of memoy... while I expected 75% moreless. But right now, I'm not sure if it will be useful, because there aren't too much devices that can handle that small file without showing blurry emojis. For sure you knew it, but I needed to try 😅

@rubengees one question related to this: do you know if the devices related to the issue have a low-density screen or a high one?

Some data to showcase my experience:

Original image
used memory when opening the emoji keyboard: 75.5 MB
used memory when performing a full scoll in first category emojis: 92.1 MB

Half sized image
used memory when opening the emoji keyboard: 41.8 MB
used memory when performing a full scoll in first category emojis: 56.0 MB

unverbraucht · 2018-07-05T14:48:52Z

Regarding increasing APK sizes: the new App Bundle format from Google will create a custom APK for each device downloading from play store containing only the resources for this resolution and ABI. It's not quite mainstream yet since we need Android Studio 3.2 to use it (which is in beta2 right now) but it might make this approach easier to consider.

Another approach would be to have per-resolution APK splits, but that is a PITA to set up.

vanniktech · 2018-07-05T18:19:32Z

App bundles will help for sure - yes. Still it would me nice if we wouldn't practically force everyone to use it by having a big library size.

rubengees · 2018-07-07T18:50:16Z

@rocboronat The device I originally reproduced this with had naturally a low-density screen due to it's age. I think most low-memory devices also have a low-density screen, but I don't know if we can rely on that.

I have been thinking about dynamically downsampling the image if not enough memory is available (as recommended in the official guides), but would need testing with various devices to see how bad the images would look on a really low-ram device.

vanniktech · 2018-07-07T21:07:24Z

I can help with testing on low memory devices.

grote · 2018-07-18T17:17:15Z

We are currently considering switching to this library and were concerned about memory consumption. So we did some measurements on a low memory device that you might be interested in: https://code.briarproject.org/briar/briar/merge_requests/857#note_28863

We also noticed that you might not be using a WeakReference to hold the Emoji sprite in memory. So this memory can never be released even when no Emoji need to be shown any more.

We are currently using the Emoji approach from the Signal app which has one sprite per Emoji category. This helps to reduce the memory footprint for cases where not all categories are needed. Combined with a WeakReference this seems to work well.

rocboronat · 2018-07-18T18:22:07Z

@rubengees sorry for the lack of clarification when I mentioned you... May you share exactly what was the density of the phone? "Low density" is too broad nowadays. It's 2018, so I consider "Low density" a Retina display, hihi... Thanks! :·)

rubengees · 2018-07-18T19:01:32Z

@rocboronat It was a Samsung Galaxy Trend Lite which has 480x800.

I tried to do the downsampling approach based on available memory which did work - no more crashes - but lead to really bad looking images, even on a phone with that low of a density. This is because you can only halve the resolution when downsampling. I will look into other options, but not sure what to do at this point...

vanniktech · 2018-07-18T19:08:56Z

Can we do something in the middle?

@rubengees was your testing done with one sheet or sheets per category?

rubengees · 2018-07-18T19:17:41Z

@vanniktech I can't think of any middle ground currently. Sure - we could fall back to those low density images on devices not having enough memory, but I can imagine there are not few active ones in that category and the images do look really bad.

It was with one total sheet. Sheets per category would require some work on the generator involving creating the sheets since our datasource does not provide sheets per category.

vanniktech · 2018-07-18T20:05:04Z

Maybe just the sheet per category does the trick. I'm okay with having a worse experience on low memory / old devices.

akwizgran · 2018-07-19T08:44:39Z

Thanks for creating a fantastic library! I just started working on migrating my app from Signal's emoji code to your library, and I'm really impressed with the well-designed API.

I did some memory profiling of my app with versions 0.5.1 and 0.6.0-SNAPSHOT of your library, and the master branch of the app which uses Signal's emoji code. Results are here. It looks like 0.5.1 uses less memory than Signal, but also releases less memory during GC. 0.6.0-SNAPSHOT uses more memory than Signal and doesn't release it on GC. This could be a problem for us when 0.6.0 is released, as we're still targetting Android 4.

As well as using a separate sheet per category, Signal uses ~~WeakReferences~~ SoftReferences for the sheets so they can be garbage collected when they're no longer needed. Maybe that would be worth considering?

I guess another possibility might be to mmap the file?

EsteveAguilera · 2018-07-19T09:03:47Z

Yesterday we released an update of our app integrating the emoji-ios:0.6.0-SNAPSHOT version. The features we need are not on the release version yet 😅

We started by deploying to 10% of users, and we get the one OOM crash report.

I'm sharing here all the info we have about this crash, hope it will help in some way:

Device: Galaxy Grand Neo Plus
Android Version: 4.4.4
Error log:

Fatal Exception: java.lang.OutOfMemoryError
       at android.graphics.BitmapFactory.nativeDecodeAsset(BitmapFactory.java)
       at android.graphics.BitmapFactory.decodeStream(BitmapFactory.java:721)
       at android.graphics.BitmapFactory.decodeResourceStream(BitmapFactory.java:546)
       at android.graphics.BitmapFactory.decodeResource(BitmapFactory.java:574)
       at android.graphics.BitmapFactory.decodeResource(BitmapFactory.java:604)
       at com.vanniktech.emoji.ios.IosEmoji.getDrawable(Unknown Source)
       at com.vanniktech.emoji.EmojiSpan.getDrawable(Unknown Source)
       at com.vanniktech.emoji.EmojiSpan.draw(Unknown Source)
       at android.text.TextLine.handleReplacement(TextLine.java:926)
       at android.text.TextLine.handleRun(TextLine.java:1009)
       at android.text.TextLine.drawRun(TextLine.java:463)
       at android.text.TextLine.draw(TextLine.java:261)
       at android.text.Layout.drawText(Layout.java:351)
       at android.text.Layout.draw(Layout.java:208)
       at android.widget.TextView.onDraw(TextView.java:5646)
       at android.view.View.draw(View.java:14681)
       at android.view.View.getDisplayList(View.java:13575)
       at android.view.View.getDisplayList(View.java:13617)
       at android.view.View.draw(View.java:14395)
       at android.view.ViewGroup.drawChild(ViewGroup.java:3237)
       at android.view.ViewGroup.dispatchDraw(ViewGroup.java:3074)
       at android.support.constraint.ConstraintLayout.dispatchDraw(Unknown Source)
       at android.view.View.draw(View.java:14684)
       at android.view.View.getDisplayList(View.java:13575)
       at android.view.View.getDisplayList(View.java:13617)
       at android.view.View.draw(View.java:14395)
       at android.view.ViewGroup.drawChild(ViewGroup.java:3237)
       at android.view.ViewGroup.dispatchDraw(ViewGroup.java:3074)
       at android.view.View.getDisplayList(View.java:13570)
       at android.view.View.getDisplayList(View.java:13617)
       at android.view.View.draw(View.java:14395)
       at android.view.ViewGroup.drawChild(ViewGroup.java:3237)
       at android.support.v7.widget.RecyclerView.drawChild(Unknown Source)
       at android.view.ViewGroup.dispatchDraw(ViewGroup.java:3074)
       at android.view.View.draw(View.java:14684)
       at android.support.v7.widget.RecyclerView.draw(Unknown Source)
       at android.view.View.getDisplayList(View.java:13575)
       at android.view.View.getDisplayList(View.java:13617)
       at android.view.ViewGroup.dispatchGetDisplayList(ViewGroup.java:3211)
       at android.view.View.getDisplayList(View.java:13512)
       at android.view.View.getDisplayList(View.java:13617)
       at android.view.ViewGroup.dispatchGetDisplayList(ViewGroup.java:3211)
       at android.view.View.getDisplayList(View.java:13512)
       at android.view.View.getDisplayList(View.java:13617)
       at android.view.ViewGroup.dispatchGetDisplayList(ViewGroup.java:3211)
       at android.view.View.getDisplayList(View.java:13512)
       at android.view.View.getDisplayList(View.java:13617)
       at android.view.ViewGroup.dispatchGetDisplayList(ViewGroup.java:3211)
       at android.view.View.getDisplayList(View.java:13512)
       at android.view.View.getDisplayList(View.java:13617)
       at android.view.ViewGroup.dispatchGetDisplayList(ViewGroup.java:3211)
       at android.view.View.getDisplayList(View.java:13512)
       at android.view.View.getDisplayList(View.java:13617)
       at android.view.HardwareRenderer$GlRenderer.buildDisplayList(HardwareRenderer.java:1577)
       at android.view.HardwareRenderer$GlRenderer.draw(HardwareRenderer.java:1451)
       at android.view.ViewRootImpl.draw(ViewRootImpl.java:2551)
       at android.view.ViewRootImpl.performDraw(ViewRootImpl.java:2417)
       at android.view.ViewRootImpl.performTraversals(ViewRootImpl.java:2006)
       at android.view.ViewRootImpl.doTraversal(ViewRootImpl.java:1063)
       at android.view.ViewRootImpl$TraversalRunnable.run(ViewRootImpl.java:5993)
       at android.view.Choreographer$CallbackRecord.run(Choreographer.java:761)
       at android.view.Choreographer.doCallbacks(Choreographer.java:574)
       at android.view.Choreographer.doFrame(Choreographer.java:544)
       at android.view.Choreographer$FrameDisplayEventReceiver.run(Choreographer.java:747)
       at android.os.Handler.handleCallback(Handler.java:733)
       at android.os.Handler.dispatchMessage(Handler.java:95)
       at android.os.Looper.loop(Looper.java:136)
       at android.app.ActivityThread.main(ActivityThread.java:5584)
       at java.lang.reflect.Method.invokeNative(Method.java)
       at java.lang.reflect.Method.invoke(Method.java:515)
       at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:1268)
       at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1084)
       at dalvik.system.NativeStart.main(NativeStart.java)

vanniktech · 2018-07-19T09:13:03Z

This could be a problem for us when 0.6.0 is released, as we're still targetting Android 4.

I'm aware of the problems with memory in the 0.6.0 development version and won't release 0.6.0 until this is fixed. On the sad side I don't have much time to work on this project though so I'm hoping for external contributions.

As well as using a separate sheet per category, Signal uses WeakReferences SoftReferences for the sheets so they can be garbage collected when they're no longer needed. Maybe that would be worth considering?

We can try that out. First the generator would need to be adapter. Maybe it's worthwhile to generate both approaches. One sheet containing every emoji and then sheets per category. That way we can easily swap them out and see which one performs better.

akwizgran · 2018-07-27T17:07:32Z

I've experimented with a few approaches to this and found a solution that seems to work well.

The generator cuts the sprite sheet into strips, one sprite wide. (This was easier to implement than creating a sheet for each category.) The strips are loaded on demand and held by SoftReferences, so they're released when the app's under memory pressure. To avoid constantly loading and releasing strips, the most recently used emoji are cached without keeping the whole strip.

Here's my branch. If you think this approach sounds reasonable I'll open a PR:
master...akwizgran:sprite-strips-soft-refs-lru-cache

On devices with low-density screens, it's also possible to scale the strips (or sheets) by a factor of 2 when loading them, which reduces the memory overhead without increasing the APK size. However, this increases the CPU cost of loading a strip. This branch includes scaling as a separate commit:
master...akwizgran:sprite-strips-soft-refs-lru-cache-scaling

rocboronat · 2018-07-29T08:39:35Z

Just to share how this issue impacts to a released app, here's an screenshot of our Fabric console. Our crash-free rate was 99,9% before 😅

rocboronat · 2018-08-09T11:47:52Z

@akwizgran we're going to launch a version to Google Play with your approach. We're using your sprite-strips-soft-refs-lru-cache branch. I'll publish the resulting crash-free rate when the app will be released to 5% of devices at least, to have a reliable picture.

vanniktech · 2018-08-09T12:24:18Z

I was on vacation so sorry for the delay. Please do open a PR with your changes @akwizgran

grote · 2018-08-09T12:27:45Z

Hope you had a nice vacation @vanniktech! :)

Do you want the branch that uses scaling or the one that doesn't? Maybe there could be an option for the library user to decide about scaling?

vanniktech · 2018-08-09T12:31:05Z

Can we detect whether scaling is needed or not depending on the device? If so that would be favorable.

akwizgran · 2018-08-09T14:23:13Z

Scaling is enabled if the screen density is less than DENSITY_XHIGH. But there's a CPU cost for scaling, so I'm not sure if it's better for low-end devices to suffer the memory cost of not scaling, or the CPU cost of scaling.

Scaling is implemented as a separate commit on top of the other changes, so shall I open a PR for the other changes and then we can evaluate the scaling changes separately?

vanniktech · 2018-08-09T14:29:06Z

Scaling is implemented as a separate commit on top of the other changes, so shall I open a PR for the other changes and then we can evaluate the scaling changes separately?

Yes that sounds perfect!

rocboronat · 2018-08-09T14:40:56Z

I went with the version without scaling to see if the scaling was needed or not. Please, wait for some data before taking any decision 😄

rocboronat · 2018-08-10T12:09:08Z

Good news there: we are at 6,1% with 0 crashes. IMHO, move on with the PR of the branch sprite-strips-soft-refs-lru-cache. Congrats and thanks for that magnificent work, @akwizgran!

vanniktech · 2018-09-11T20:19:00Z

I'll close this since I've merged the PR which fixed this. #297
Please test out the latest snapshot and report any issues back.

vanniktech · 2018-09-11T20:19:11Z

Thanks everyone for your awesome help!

vanniktech added bug pr welcome labels May 16, 2018

daniele-athome mentioned this issue Jun 21, 2018

Out of memory with new emoji version kontalk/androidclient#1185

Closed

vanniktech mentioned this issue Jun 30, 2018

New release? #289

Closed

vanniktech closed this as completed Sep 11, 2018

OOM with the new sheet approach #275

OOM with the new sheet approach #275

Comments

rubengees commented Mar 29, 2018 • edited Loading

shi2134 commented Mar 29, 2018

vanniktech commented Mar 29, 2018

daniele-athome commented Jun 21, 2018

rocboronat commented Jul 1, 2018

vanniktech commented Jul 1, 2018

rocboronat commented Jul 1, 2018

rocboronat commented Jul 1, 2018

vanniktech commented Jul 1, 2018

rocboronat commented Jul 1, 2018

vanniktech commented Jul 1, 2018

rocboronat commented Jul 5, 2018

unverbraucht commented Jul 5, 2018

vanniktech commented Jul 5, 2018

rubengees commented Jul 7, 2018

vanniktech commented Jul 7, 2018

grote commented Jul 18, 2018

rocboronat commented Jul 18, 2018

rubengees commented Jul 18, 2018

vanniktech commented Jul 18, 2018

rubengees commented Jul 18, 2018

vanniktech commented Jul 18, 2018

akwizgran commented Jul 19, 2018 • edited Loading

EsteveAguilera commented Jul 19, 2018

vanniktech commented Jul 19, 2018 • edited Loading

akwizgran commented Jul 27, 2018

rocboronat commented Jul 29, 2018 • edited Loading

rocboronat commented Aug 9, 2018

vanniktech commented Aug 9, 2018

grote commented Aug 9, 2018

vanniktech commented Aug 9, 2018

akwizgran commented Aug 9, 2018

vanniktech commented Aug 9, 2018

rocboronat commented Aug 9, 2018

rocboronat commented Aug 10, 2018

vanniktech commented Sep 11, 2018

vanniktech commented Sep 11, 2018

rubengees commented Mar 29, 2018 •

edited

Loading

akwizgran commented Jul 19, 2018 •

edited

Loading

vanniktech commented Jul 19, 2018 •

edited

Loading

rocboronat commented Jul 29, 2018 •

edited

Loading