Performance bug: getSetting() calls in the onSensorChange callbacks are __extremely__ expensive.

[mcourteaux]

The collection of data is draining my phone's battery, so I ran a tracing profiler in Android Studio. The culprit because obvious quite quickly: for each of the sensors, a thread is running and calling the onSensorChange callback. These callbacks have a couple of calls to Aware.getSetting() (to fetch some debug value, and to fetch the device ID to add it to the log). For example, in Gyroscope.java:

• Fetch Device ID:

  aware-client/aware-core/src/main/java/com/aware/Gyroscope.java

  Line 138 in 7d1f276

  rowData.put(Gyroscope_Data.DEVICE_ID, Aware.getSetting(getApplicationContext(), Aware_Preferences.DEVICE_ID));

• Fetch some debug flag:

  aware-client/aware-core/src/main/java/com/aware/Gyroscope.java

  Line 103 in 7d1f276

  if (!Aware.getSetting(getApplicationContext(), Aware_Preferences.DEBUG_DB_SLOW).equals("true")) {

  
  and:
  

  aware-client/aware-core/src/main/java/com/aware/Gyroscope.java

  Line 158 in 7d1f276

  if (!Aware.getSetting(getApplicationContext(), Aware_Preferences.DEBUG_DB_SLOW).equals("true")) {

These 3 calls in Gyroscope::onSensorChange take up 97% of the CPU time according to the tracing profiler:

This is due to the fact that it's querying the SQLite database every time to fetch those requested settings. So that's three queries per sample. The entire optimization strategy of buffering the data before running a bulkInsert() is nullified by this bug.

I'm not actively working with this framework, nor am I an experienced Android developer. I can think of several fixes, but I'm opening this issue to also have the discussion of what the best fix would be. My first ideas are:

• Cache the two settings per sensor-thread. Is this safe? Do these values update during the runtime of a single app?
• Cache the settings object on the Aware-class level:

  aware-client/aware-core/src/main/java/com/aware/Aware.java

  Line 1041 in 7d1f276

  public static String getSetting(Context context, String key) {

  
  This is probably the safer (but slightly more work) approach, as one can now invalidate cache values easily on the Aware class level.
• Swap out the current SQLite approach for maintaining settings for something more reasonable. I don't have suggestions here. SQLite is fine anyway if the settings are desired to be persistent.

I'd go for one of the first two fixes. If we can safely assume that the settings DEVICE_ID and DEBUG_DB_SLOW will not change during runtime (which I think is super reasonable), I'd be more than happy to just fix this using a caching strategy in the several sensor classes and open a Pull Request. But before I start working on this, I'd like some feedback from the maintainers.

[denzilferreira]

Dear Martjin,

Thanks for the report and proposal to fix. Been a while (10 years) since we developed the initial code 🙈

The first approach is ideal (cache the two settings per sensor-thread). The DEVICE_ID is set once when installing and running the client/app for the first time or when joining a new study (which will restart the sensors and plugins - calling onStartCommand). The DEBUG flag can be changed in runtime, but that will also trigger a restart of the active sensors and plugins - calling onStartCommand - so it is also safe to cache it. Assigning these variables inside onStartCommand and reusing them in the callbacks should address this.

[mcourteaux]

Thanks for the response! Looking at Accelerometer.java, there is also a flag for STATUS_WEBSOCKET, what about that one? That one gives me less-cacheable vibes 😝 . Or will it also be notified through onStartCommand()?

[denzilferreira]

Using websockets to stream in real time sensor data was a very bad idea for data collection but could be interesting to create interactive applications. Your phone dies in a matter of a couple of hours, plus the overhead of network. It can be cached too.

[mcourteaux]

I made the PR at: #298