Everyone knows about it….

How Singleton Save Me From Out of Memory Error

Hi there, long time I don’t talk about something more “techies” (not that Techies from Dota 2). I’ve been working on an Android Project and I want to talk about one library I’ve used extensively: Google GSON. The particular usage of GSON is to simplify the painful method of processing JSON object in Java.

Wow. Simply wow. Look how convoluted is that to serialize or deserialize a JSON object in Java. Imagine what would you do if you want to process many user data with more fields with more “depth”. GSON is here to help simplify the matter with more familiar approach: treat JSON object as instance of class. What does it mean? It means that we can get any associated field from the variable using direct approach.

See, GSON makes handling JSON object simpler and easier to understand. Simpler doesn’t necessarily means shorter. As long as you have predefined the required classes, you’re good to go with GSON. However, please note that Java class is not like JavaScript or Python which can access a property from the hash key (in Javascript/Python: user.name == user["name"]), therefore for the sake of consistency, it is ill-advised to use -(dash) as property name (although, GSON can handle -(dash) with simple annotation).

The Problem

I use GSON extensively to convert JSON from socket.io to Notification class. My app contains list of ABUNDANT number of notification JSON, where each notification is a quite complex object. To enable ubiquity of notification, I made the notification in a singleton, a static variable of a class which can be accessed anywhere. Here’s the algorithm for notification:

Fetch all notification until app start time with GET notification API. If connection unavailable, read notification from local storage (SharedPreferences).

Store obtained list of notifications from API to local storage (SharedPreferences) to ensure data integrity

Append any incoming notification from socket.io to the list. New notification may update existing notification, therefore append only happen new unique notification. Update notification should move the respective notification to the top of the list.

Any problem? The code makes sense, so I left it as is. I run the code above in the background process, so that the app can handle any incoming socket message anytime.

We use Crashlytics — a library from Twitter’s Fabric SDK. Whenever crash occured, Crashlytics will record the crash and device stacktrace, then send the data to our dashboard. Therefore, we know when error occured, at what lines, and what type of error. We, however, do not track/record the processed data that cause the crash, because it is the same with privacy breach. Out of blue, Crashlytics in production shown that a lot of our customer experience the crash with a single, unknown exception: Java OutOfMemoryError Exception. In development mode, we rarely encountered that error; if occured, it occured on low-end devices.

…..

Damn.

The stacktraces were useless, I didn’t know why. Other errors that Crashlytics recorded were useful, at least the record which Android Activity the app crashed, at what line. Not this one. I had no clue why this happened. Sometimes the error is quite weird: Dalvik Scheduler stop? How am I supposed to care about something as low as OS runtime scheduler from my app.

I always think that maybe the RAM is unable to handle our app. Maybe the image is too big. Maybe other resource is too much. Or maybe the request is too big and too complex to handle. If it is the latter, then which API is the problem?

……

Then, one day, it occured to my device. It crash in NotificationUtility.getList() with OutOfMemoryError. Hmm, weird.

The Realization

I am a stupid, naive programmer. I couldn’t see a HUGE problem lies in the NotificationUtility code. But then it made me realize that the problem is two-fold actually.

I naively read from storage anytime I need the notification, then convert it to the list of notifications using GSON. I can reduce the overhead of write by using the list in RAM, and whenever notification is modified, just write. No more than once read.

The string from local storage is freed, of course, because the string is already unused beyond GSON conversion. The GSON object, however, is not freed, because I keep the reference in background process. Whenever I need to append new notifications, I use NotificationUtility.getList() which generate new GSON object to handle the list of notification’s data structure. Apparently, new GSON object keep generated whenever getList() invoked, without being able to be freed the data structure (perhaps because the method is static, therefore there’s no guarantee that the GC (garbage collector) will freed the allocated heap.

Solution: Make the List of notifications and GSON object as singleton.

List of notifications is obvious solution, but what’s wrong with GSON? With a single GSON across whole app, the sole GSON data structure will be repeatedly changed (because I use GSON to process any API response). Thus, it is guaranteed that only one GSON ever generated in this app. There will be no more “zombie” GSON object anymore. The logic should be

Fetch all notification until app start time with GET notification API. If connection unavailable, read notification from local storage (SharedPreferences) once.

Store obtained list of notifications from API stored to both List of Notification in RAM and local storage (SharedPreferences) to ensure data integrity.

Append any incoming notification from socket.io to the list. New notification may update existing notification, therefore append only happen new unique notification. Update notification should move the respective notification to the top of the list. Check to RAM only. Write to if changed. No more read from local storage.

Hmm.. Yep. One OutOfMemoryError done. Still some more causes to go. Perhaps a little bit of refactoring or changing image downloader library might be a viable choice. Any discussion to improve the code performance is welcomed.

8 thoughts on “How Singleton Save Me From Out of Memory Error”

This is not a singleton. And this code is probably one of many example why some people discourage the uses of static in Java. It’s not evil, but easy to abuse, and once you did it wrong you’ll end up with a lot of undesired behavior that is so hard to trace.

Is there any good reason not to use a normal instance object?
NotificationUtility.getList() is probably faster than (new NotificationUtility()).getList(), and it looks cleaner, but the difference in performance is negligible in most cases. The list of notification is the only static variable needed, that is somewhat justifiable because it is used as a simple memory cache.

On a completely different note. You are not expecting the notification to contains big amount of data, right? because if you are, you might find trouble later by storing it in shared preference, and by using strong reference as a cache.

Static object, why? Because the notification needed to be accessed from anywhere, like any activity or adapter. Rather than holding the reference to NotificationUtility instance in each activity/adapter like normally instatiated data ( NotificationUtility util; util.getList() ), it is much cleaner to treat the NotificationUtility as an object ( NotificationUtility.getList() ). Yes, I need the Notification Utility to be as ubiquitous as possible, that’s why I don’t make it normal object.

Oh for the last paragraph: our v1 notification API is quite stupid, as the response is not paged, and some notifications are duplicated (like: User A like your status X. User B like your status x), thus, the app need to trim-down the duplicated object in phone. So, the answer is yes, the number of object stored in the list is ABUNDANT.

Our team actually now working to optimize the notification code to make it much cleaner. For the time being, our app must hide the pain of unoptimized API response from user. After the v2 release, the app will handle less response, only the needed one to be stored in Shared Preference.

Because you lost a lot of OOP benefits if you put everything into static. No polymorphism, no inheritance, no other OOP goodness, you don’t even have any constructor to pass anything to. And you can’t use unit test on all static class like this. In other words, unless you are really really completely sure that each method will always be stateless now and later in the future, and you don’t need abstraction, then don’t use static.

Let’s say, for example. You had a requirement change, and for this you somewhat decided that you need to replace Gson with another library which handle serializing, caching, and every bit of nice feature you need.

Okay, easy, just replace gson variable with the new library, done.
Well, if things are that easy then you’re lucky. Let’s add a bit more problem here: you need to pass Context for the new library every time you use it.

What could you do? Create a setContext method? no, that won’t do, that makes it stateful and you’ll find it annoying later that the context is suddenly set as null, or changed to a completely unrelated context and that break things. After all setContext method is accessible from anywhere.

Okay, setContext won’t do, then the other options is to add Context argument to each of the method. It’s nice, easy to call from anywhere, and pose no problem. The “only” problem is that the existing method is called everywhere in the project and you need to update each one of them. Your code become coupled and your code might break somewhere you don’t know.

If this happens on a normal instance object. All you need to do is update the constructor and let the compiler solve the problem by itself (assuming you use dependency injection or something similiar, and even if you don’t there is still a lot of options to do).

This is just one of many problem that could happen on a class that have no abstraction. You might want to keep this class simple by naming it ‘Utility.’ Everything stateless, all happy. I would agree on that if software requirement is frozen, unchanging no matter what happens outside.

Frankly speaking, I think that this class is likely to change as the requirement changes, and making everything static just limit things.
And if you insist on static, then it’s better to use proper singleton. Not this.