Android synchronized onSensorChanged?

Question

This is a followup to my question here: Android thread runnable performance

I'm having some difficulty wrapping my head around synchronized methods for my app

I'm polling the sensors and storing the sensor values into arrays whenever they change

float[] accelerometerMatrix = new float[3];
float[] accelerometerWorldMatrix = new float[3];
float[] gyroscopeMatrix = new float[3];
float[] gravityMatrix = new float[3];
float[] magneticMatrix = new float[3];
float[] rotationMatrix = new float[9];

class InsertHandler implements Runnable {
        public void run() {
            //get values from arrays and insert into db
        }
    }

public void onSensorChanged(SensorEvent event) {
        sensor = event.sensor;

        int i = sensor.getType();
        if (i == MainActivity.TYPE_ACCELEROMETER) {
            accelerometerMatrix = event.values;
        } else if (i == MainActivity.TYPE_GYROSCOPE) {
            gyroscopeMatrix = event.values;
        } else if (i == MainActivity.TYPE_GRAVITY) {
            gravityMatrix = event.values;
        } else if (i == MainActivity.TYPE_MAGNETIC) {
            magneticMatrix = event.values;
        }

        long curTime = System.currentTimeMillis();
        long diffTime = (curTime - lastUpdate);

        // only allow one update every POLL_FREQUENCY.
        if(diffTime > POLL_FREQUENCY) {
            lastUpdate = curTime;

            //insert into database in background thread
            executor.execute(insertHandler);
        }
    }

Every 10ms, my app will take the current sensor values (from the arrays) and insert them into a database using a single thread executor. So the onSensorChanged method is both writing to the arrays, and reading from the arrays to write to database

My question is, should the onSensorChanged method be synchronized?

The most important thing is that I don't miss any data. Every 10ms I need to store the current sensor values - none can be missed.

So to my understanding, a synchronized method means that the UI thread will hold a lock, and it will write the sensor values to the arrays. During this time, the executor thread cannot read from those arrays due to the lock. Then the lock is lifted and the executor thread then locks, reads from arrays, and write to database, releases lock

I might be misunderstanding the use of synchronized methods here, especially given onSensorChanged is event driven and I'm not sure how that plays into it

But it seems that in a situation like this, I might not be inserting the most recent values every 10ms. When the UI thread establishes a lock, the executor thread can't write those values to the database. By the time the executor thread can write, the values would now be a few ms old and inaccurate

On the other hand, synchronization would mean that I dont have situations where the UI thread is changing the array values, while at the same time the executor thread is insert half changed values into the database

So for this type of situation where I need to insert the most recent/accurate sensor data every 10ms, should I be using a synchronized method?

Answer 1

Your current code is not thread safe as the Runnable is using the same arrays that the UI thread is writing to. Once you call executor.execute(insertHandler); there is no guarantee the UI thread won't get another sensor event and change one of the array values before the Runnable writes them to the database. It appears you understand this part.

To fix this I would not recommend using a synchronized block at all as it seems you just want to write out whatever values are stored in the arrays when diffTime > POLL_FREQUENCY. The onSensorChanged(...) method itself will only ever be called on the UI thread in your code so you don't have to worry about another thread changing the value of the arrays while in this method.

With all this said what you can do is store the current values of the arrays in a new instance of your Runnable class. I know it was suggested in your previous post to use the same instance but that will not make a noticeable difference. You can even verify by opening Android Monitor and check your memory usage as your application runs. By storing the current values it now won't matter if onSensorChanged() is called again before you write out the data because you already have a copy of the data you need that won't change.

Here is what I am suggesting in code:

class InsertHandler implements Runnable {
    final float[] accelerometerMatrix;
    final float[] accelerometerWorldMatrix;
    final float[] gyroscopeMatrix;
    final float[] gravityMatrix;
    final float[] magneticMatrix;
    final float[] rotationMatrix;

    public InsertHandler(float[] accelerometerMatrix, float[] accelerometerWorldMatrix,
            float[] gyroscopeMatrix, float[] gravityMatrix,
            float[] magneticMatrix, float[] rotationMatrix) {
        this.accelerometerMatrix = accelerometerMatrix;
        this.accelerometerWorldMatrix = accelerometerWorldMatrix;
        this.gyroscopeMatrix = gyroscopeMatrix;
        this.gravityMatrix = gravityMatrix;
        this.magneticMatrix = magneticMatrix;
        this.rotationMatrix = rotationMatrix;
    }

    public void run() {
        // use class field arrays values and insert into db
    }
}

And then when you add the Runnable to the executor use:

Runnable insertHandler = new InsertHandler(accelerometerMatrix, accelerometerWorldMatrix,
        gyroscopeMatrix, gravityMatrix, magneticMatrix, rotationMatrix);
executor.execute(insertHandler);