Android caching events results in crashing if locking

Question

I'm storing Events in a Queue when the Android app is not in foreground. Events can be a lot, so I'd like to remove the oldest, when I reach a certain limit, to avoid memory issues. Events can be of different type and I'd like to to avoid removing certain (if not really needed, eg. onTrimMemory()).

public void enqueue(T event) {
    synchronized (events) {
        if (events.size() > 2000) {
            for (int i = 0; i < events.size(); i++) {
                if (canRemove(events.get(i))) {
                    events.remove(i);
                    break;
                }
            }
        }

        events.add(event);
    }
}

canRemove(event) check if the Event is instanceof of something that can be removed and returns true/false.

After a bit Logcat is giving me: "long monitor contention event with owner method" and after a while Logcat reports

Starting a blocking GC Alloc

Waiting for a blocking GC Alloc

Clamp target GC heap from 65MB to 64MB and then, after a lot of different messages... the app crash.

From what I've understand reading a similar question (What might be the cause of "long monitor contention event with owner method"?) the problem is that I'm receveing a lot of Events and locking on events for "too much" time.

So the question is... what can I do? I can optimize it a bit by saving the last position I removed and the next time start the for loop from it, but I don't think it's enough. Or I can do different Queue for different Events so that I always remove the first one, but I always need to lock it. Any better idea?

I forgot to say I'm declaring: private LinkedList events = new LinkedList<>();

Mohammad Imran · Accepted Answer · 2015-09-17T12:54:10.760

0

Use this method

   public void enqueue(T event) {
    List<T> removable = new ArrayList<>();
    synchronized (events) {
        final int size = events.size();
        if (size > 2000) {
            for (int i = 0; i < size; i++) {
                final T t = events.get(i);
                if (canRemove(t)) {
                    removable.add(t);
                    break;
                }
            }
        }
        if (removable.size() > 0) {
            events.removeAll(removable);
        }
       events.add(event);
    }
}

Hopefully, it will solve your issue.

edited Sep 17 '15 at 12:54

answered Sep 16 '15 at 10:51

Mohammad Imran

3,264
4
23
37

Please note that I'm removing one event at a time when adding, so that I lost the minimum number of Events. – MoreOver Sep 16 '15 at 11:03
Thank you. Just a question: having events.add() outside the lock couldn't cause a ConcurrentException, considering events is a LinkedList? – MoreOver Sep 17 '15 at 08:05
Note that this implementation is not synchronized. If multiple threads access a linked list concurrently, and at least one of the threads modifies the list structurally, it mustbe synchronized externally. So why the .add() call is outside? – MoreOver Sep 17 '15 at 10:55
You can synchronized a whole method, rather than a block – Mohammad Imran Sep 17 '15 at 12:54

score 0 · Answer 2 · edited Jul 22 '16 at 15:45

I think you can use events.subList method to instead of remove object, like this:

public void enqueue(T event) {
    List<T> removable = new ArrayList<>();
    synchronized (events) {
        final int size = events.size();
        if (size > 2000) {
            int removeSize = events.size() - 2000;
            events = events.subList(removeSize, events.size());
        }
       events.add(event);
    }
}

Android caching events results in crashing if locking

2 Answers2