3

Sorry about the confusing title. Not sure quite how to phrase it, which may be the problem!

I'm looking for a good abstraction to use for a situation involving concurrent threads.

I've been able to get close, but not quite nail it.

Simplified slightly, I've got two kinds of sensor input being collected on an Android phone - orientation-type stuff and WiFi scans.

When enough of both have been collected, I want to trigger an event to occur which uses the data. However, I don't want to stop there - this process should continue N times.

At first I just used while loops on the condition, e.g.

startCollecting();
while (samplesCollected < X){
    // wait
    while (directionCount < Y || scanCount < Z){}; 
    // then
    doSomeStuff();
    samplesCollected++; 
}
stopCollecting();

However, I've been told by SO that this is a poor performer, and indeed I have been experiencing some lock up of the UI (even though it's on a different thread), so I decided to utilise java.util.concurrent.

The problem is that I can't quite work out which abstraction to use, perhaps due to my inexperience.

  • Condition(s) on ReentrantLock:

    idea of condition seems great - but it's not the case that I want to control a Shared Resource - I want the data collection to continue in the background whilst the first batch is processed - so where do I call lock? If I don't lock, then it throws an IllegalMonitorStateException.

  • CountdownLatch:

    seems ideal - when the collecting threads have data available, they can call countDown(), and when countDown has been called enough times then the action can continue. But countDown is supposed to be a one-off execution, and I need this to repeat several times.

  • CyclicBarrier:

    docs for CountdownLatch suggest a CyclicBarrier should be used instead if you want the behaviour to be repeatable - but the metaphor of the CyclicBarrier seems totally wrong for this situation, and I don't understand how to use it for this.

I've linked a few related questions below - any guidance would be much appreciated.

Efficiency - use of Thread.yield in loop that waits for variable change

How to make a Java thread wait for another thread's output?

is there a 'block until condition becomes true' function in java?

A simple scenario using wait() and notify() in java

Community
  • 1
  • 1
Alex
  • 18,332
  • 10
  • 49
  • 53
  • Condition, CountdownLatch etc are low level primitives, and often are interchangeable, so that using one instead of the other may be less efficient but still correct. You should first formulate your task on a higher level: describe how reading from sensors is done (is it blocking read?) and how you want to run the reaction when enough data are collected (on a separate thread? overlapping with sensor reading?). – Alexei Kaigorodov May 02 '13 at 03:15
  • Alexei, the Android API only allows for non-blocking reads - basically the threading on the sensor reading is all handled on background threads in the API, is my understanding. We get an onReceive callback function to override. I'd like the reaction when data are collected to run overlapping with sensor reading. – Alex May 02 '13 at 03:34
  • onReceive? http://developer.android.com/guide/topics/sensors/sensors_overview.html talks only about SensorEventListener.( onAccuracyChanged, onSensorChanged). Anyway, this not non-blocking read - this is asynchronous read. The callback methods should collect data and, when collected enough, should notify a working thread which would process the data in parallel. The working thread can do blocking wait on input queue which holds data, or it can be an ExecutorService which processes Runnable tasks. – Alexei Kaigorodov May 02 '13 at 09:20

3 Answers3

2

You might wish to use a concurrent queue (of the BlockingQueue variety).

You fill the queue from the threads reading the sensors (instead of putting them into whatever structures you are putting them in now).

In your loop, take a piece of data, examine what it is (orientation or wifi), increment the correct counter, and put the data somewhere (probably a local list of some sort). Once you have enough, pass the data you collected to your processing function.

This works because your thread sleeps when it tries to take something from the queue and nothing is available, so it doesn't sit around polling the counter.

yiding
  • 3,482
  • 18
  • 17
  • Ah, interesting - I think that would work. So basically the processing task just calls take() as if the data were already available, and the waiting will be handled by the BlockingQueue? Clever. – Alex May 02 '13 at 03:36
2

We have a similar implementaion in our code. We have created a inner class which implements runnable and can process the data. We keep on reading the data in a single thread and once the size of data reaches a particular limit then we pass on that data to an instance of inner class and submit that inner class instance as task to ThreadPoolExecutor service.

This works very well for us.

Lokesh
  • 7,810
  • 6
  • 48
  • 78
  • +1 - there doesn't seem to be any advantage to having the sample processing thread sitting around idle when you can just create a new one as needed – Zim-Zam O'Pootertoot May 02 '13 at 03:31
  • Right, that makes a lot of sense as well, and side-steps the problem of concurrency control to some extent. To clarify, you mean put the check on data size into the onReceive() method, and trigger the data collection from there on a separate thread? – Alex May 02 '13 at 03:40
  • That's right, just be sure to use a ThreadPoolExecutor to trigger the thread - repeatedly calling `new Thread()` will cause a performance hit – Zim-Zam O'Pootertoot May 02 '13 at 03:48
  • I've marked this as the correct answer, since it uses fewer threads, and seems to perform really well – Alex May 20 '13 at 01:41
1

You're app locked up because that piece of code is busy waiting which is generally a bad thing. For a quick fix you can add a Thread.sleep(25) (or equivalent) in the inner while loop and that should fix the lock up.

Couple other things ... First off the variables samplesCollected, directionCount, and scanCount should be marked volatile or be AtomicLong (or AtomicInteger). Otherwise you are not guaranteed to see changes made in another thread to them. Read about memory barriers and more specifically about the Java memory model to understand why.

If you ensure the variables are thread safe and add the Thread.sleep(...) you should be fine (it won't lock up your app and should function correctly) though it's not an ideal solution.

This isn't an ideal solution though. You can get rid of this master thread by having the worker threads themselves check after each increment if it's above the threshold. If so then they can kick off a thread (or in the same thread) execute your post-aggregation code. Similarly if it reaches a certain max threshold you can signal all the threads to stop collecting. You'll need to use AtomicLong.incrementAndGet() for this to work properly though to ensure that the threads process the counts properly (volatile will not work).

sehrope
  • 1,777
  • 13
  • 16
  • +1 for the simple fix of adding Sleep. Without it that loop would kill the app - CPU through the roof! – xagyg May 02 '13 at 03:48
  • 'simple fix of adding Sleep' - I would call it the 'simple fix of adding avoidable latency and poor performance'. Sleep() has its valid uses, but a substitute for efficient inter-thread comms is not one of them. – Martin James May 02 '13 at 07:41