From 787ac1b388f144f5e6dd38f8b807866a5256dafc Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Tue, 18 Sep 2012 18:49:18 -0700 Subject: [PATCH] improve sensor battery usage tracking until now we were tracking when a sensors was physically enabled or disabled and we were reporting that to the BattaryService. this wasn incorrect because we could have several different apps enabling the same sensor, so the accounting by the battery service would be incorrect in that case (depending on the order in which these apps disabled said sensor). BatteryService tracks sensors per uid, however SensorService does this per binder connection, so we could have several binder connections for the same uid, to solve this we keep a list of sensor/uid -> count, which is the bulk of this change. Bug: 6661604 Change-Id: I561c198c42ba1736a8671bdacda4c76d72b9dd6f --- services/sensorservice/Android.mk | 1 + services/sensorservice/BatteryService.cpp | 126 ++++++++++++++++++++++ services/sensorservice/BatteryService.h | 71 ++++++++++++ services/sensorservice/SensorDevice.cpp | 75 +------------ services/sensorservice/SensorService.cpp | 7 +- 5 files changed, 207 insertions(+), 73 deletions(-) create mode 100644 services/sensorservice/BatteryService.cpp create mode 100644 services/sensorservice/BatteryService.h diff --git a/services/sensorservice/Android.mk b/services/sensorservice/Android.mk index 6a302c061..e0cfaa663 100644 --- a/services/sensorservice/Android.mk +++ b/services/sensorservice/Android.mk @@ -2,6 +2,7 @@ LOCAL_PATH:= $(call my-dir) include $(CLEAR_VARS) LOCAL_SRC_FILES:= \ + BatteryService.cpp \ CorrectedGyroSensor.cpp \ Fusion.cpp \ GravitySensor.cpp \ diff --git a/services/sensorservice/BatteryService.cpp b/services/sensorservice/BatteryService.cpp new file mode 100644 index 000000000..70b65ab69 --- /dev/null +++ b/services/sensorservice/BatteryService.cpp @@ -0,0 +1,126 @@ +/* + * Copyright (C) 2012 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include +#include +#include + +#include +#include +#include + +#include +#include + +#include "BatteryService.h" + +namespace android { +// --------------------------------------------------------------------------- + +BatteryService::BatteryService() { + const sp sm(defaultServiceManager()); + if (sm != NULL) { + const String16 name("batteryinfo"); + mBatteryStatService = sm->getService(name); + } +} + +status_t BatteryService::noteStartSensor(int uid, int handle) { + Parcel data, reply; + data.writeInterfaceToken(DESCRIPTOR); + data.writeInt32(uid); + data.writeInt32(handle); + status_t err = mBatteryStatService->transact( + TRANSACTION_noteStartSensor, data, &reply, 0); + err = reply.readExceptionCode(); + return err; +} + +status_t BatteryService::noteStopSensor(int uid, int handle) { + Parcel data, reply; + data.writeInterfaceToken(DESCRIPTOR); + data.writeInt32(uid); + data.writeInt32(handle); + status_t err = mBatteryStatService->transact( + TRANSACTION_noteStopSensor, data, &reply, 0); + err = reply.readExceptionCode(); + return err; +} + +bool BatteryService::addSensor(uid_t uid, int handle) { + Mutex::Autolock _l(mActivationsLock); + Info key(uid, handle); + ssize_t index = mActivations.indexOf(key); + if (index < 0) { + index = mActivations.add(key); + } + Info& info(mActivations.editItemAt(index)); + info.count++; + return info.count == 1; +} + +bool BatteryService::removeSensor(uid_t uid, int handle) { + Mutex::Autolock _l(mActivationsLock); + ssize_t index = mActivations.indexOf(Info(uid, handle)); + if (index < 0) return false; + Info& info(mActivations.editItemAt(index)); + info.count--; + return info.count == 0; +} + + +void BatteryService::enableSensorImpl(uid_t uid, int handle) { + if (mBatteryStatService != 0) { + if (addSensor(uid, handle)) { + int64_t identity = IPCThreadState::self()->clearCallingIdentity(); + noteStartSensor(uid, handle); + IPCThreadState::self()->restoreCallingIdentity(identity); + } + } +} +void BatteryService::disableSensorImpl(uid_t uid, int handle) { + if (mBatteryStatService != 0) { + if (removeSensor(uid, handle)) { + int64_t identity = IPCThreadState::self()->clearCallingIdentity(); + noteStopSensor(uid, handle); + IPCThreadState::self()->restoreCallingIdentity(identity); + } + } +} + +void BatteryService::cleanupImpl(uid_t uid) { + if (mBatteryStatService != 0) { + Mutex::Autolock _l(mActivationsLock); + int64_t identity = IPCThreadState::self()->clearCallingIdentity(); + for (ssize_t i=0 ; irestoreCallingIdentity(identity); + } +} + +const String16 BatteryService::DESCRIPTOR("com.android.internal.app.IBatteryStats"); + +ANDROID_SINGLETON_STATIC_INSTANCE(BatteryService) + +// --------------------------------------------------------------------------- +}; // namespace android + diff --git a/services/sensorservice/BatteryService.h b/services/sensorservice/BatteryService.h new file mode 100644 index 000000000..86cc88420 --- /dev/null +++ b/services/sensorservice/BatteryService.h @@ -0,0 +1,71 @@ +/* + * Copyright (C) 2012 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include +#include + +#include + +namespace android { +// --------------------------------------------------------------------------- + +class BatteryService : public Singleton { + static const int TRANSACTION_noteStartSensor = IBinder::FIRST_CALL_TRANSACTION + 3; + static const int TRANSACTION_noteStopSensor = IBinder::FIRST_CALL_TRANSACTION + 4; + static const String16 DESCRIPTOR; + + friend class Singleton; + sp mBatteryStatService; + + BatteryService(); + status_t noteStartSensor(int uid, int handle); + status_t noteStopSensor(int uid, int handle); + + void enableSensorImpl(uid_t uid, int handle); + void disableSensorImpl(uid_t uid, int handle); + void cleanupImpl(uid_t uid); + + struct Info { + uid_t uid; + int handle; + int32_t count; + Info() : uid(0), handle(0), count(0) { } + Info(uid_t uid, int handle) : uid(uid), handle(handle), count(0) { } + bool operator < (const Info& rhs) const { + return (uid == rhs.uid) ? (handle < rhs.handle) : (uid < rhs.uid); + } + }; + + Mutex mActivationsLock; + SortedVector mActivations; + bool addSensor(uid_t uid, int handle); + bool removeSensor(uid_t uid, int handle); + +public: + static void enableSensor(uid_t uid, int handle) { + BatteryService::getInstance().enableSensorImpl(uid, handle); + } + static void disableSensor(uid_t uid, int handle) { + BatteryService::getInstance().disableSensorImpl(uid, handle); + } + static void cleanup(uid_t uid) { + BatteryService::getInstance().cleanupImpl(uid); + } +}; + +// --------------------------------------------------------------------------- +}; // namespace android + diff --git a/services/sensorservice/SensorDevice.cpp b/services/sensorservice/SensorDevice.cpp index 2244a8688..a9e3ef4f1 100644 --- a/services/sensorservice/SensorDevice.cpp +++ b/services/sensorservice/SensorDevice.cpp @@ -32,68 +32,6 @@ #include "SensorService.h" namespace android { -// --------------------------------------------------------------------------- -class BatteryService : public Singleton { - static const int TRANSACTION_noteStartSensor = IBinder::FIRST_CALL_TRANSACTION + 3; - static const int TRANSACTION_noteStopSensor = IBinder::FIRST_CALL_TRANSACTION + 4; - static const String16 DESCRIPTOR; - - friend class Singleton; - sp mBatteryStatService; - - BatteryService() { - const sp sm(defaultServiceManager()); - if (sm != NULL) { - const String16 name("batteryinfo"); - mBatteryStatService = sm->getService(name); - } - } - - status_t noteStartSensor(int uid, int handle) { - Parcel data, reply; - data.writeInterfaceToken(DESCRIPTOR); - data.writeInt32(uid); - data.writeInt32(handle); - status_t err = mBatteryStatService->transact( - TRANSACTION_noteStartSensor, data, &reply, 0); - err = reply.readExceptionCode(); - return err; - } - - status_t noteStopSensor(int uid, int handle) { - Parcel data, reply; - data.writeInterfaceToken(DESCRIPTOR); - data.writeInt32(uid); - data.writeInt32(handle); - status_t err = mBatteryStatService->transact( - TRANSACTION_noteStopSensor, data, &reply, 0); - err = reply.readExceptionCode(); - return err; - } - -public: - void enableSensor(int handle) { - if (mBatteryStatService != 0) { - int uid = IPCThreadState::self()->getCallingUid(); - int64_t identity = IPCThreadState::self()->clearCallingIdentity(); - noteStartSensor(uid, handle); - IPCThreadState::self()->restoreCallingIdentity(identity); - } - } - void disableSensor(int handle) { - if (mBatteryStatService != 0) { - int uid = IPCThreadState::self()->getCallingUid(); - int64_t identity = IPCThreadState::self()->clearCallingIdentity(); - noteStopSensor(uid, handle); - IPCThreadState::self()->restoreCallingIdentity(identity); - } - } -}; - -const String16 BatteryService::DESCRIPTOR("com.android.internal.app.IBatteryStats"); - -ANDROID_SINGLETON_STATIC_INSTANCE(BatteryService) - // --------------------------------------------------------------------------- ANDROID_SINGLETON_STATIC_INSTANCE(SensorDevice) @@ -218,16 +156,9 @@ status_t SensorDevice::activate(void* ident, int handle, int enabled) ALOGD_IF(DEBUG_CONNECTIONS, "\t>>> actuating h/w"); err = mSensorDevice->activate(mSensorDevice, handle, enabled); - if (enabled) { - ALOGE_IF(err, "Error activating sensor %d (%s)", handle, strerror(-err)); - if (err == 0) { - BatteryService::getInstance().enableSensor(handle); - } - } else { - if (err == 0) { - BatteryService::getInstance().disableSensor(handle); - } - } + ALOGE_IF(err, "Error %s sensor %d (%s)", + enabled ? "activating" : "disabling", + handle, strerror(-err)); } { // scope for the lock diff --git a/services/sensorservice/SensorService.cpp b/services/sensorservice/SensorService.cpp index cc2f745ea..e3dcd0287 100644 --- a/services/sensorservice/SensorService.cpp +++ b/services/sensorservice/SensorService.cpp @@ -39,6 +39,7 @@ #include +#include "BatteryService.h" #include "CorrectedGyroSensor.h" #include "GravitySensor.h" #include "LinearAccelerationSensor.h" @@ -421,6 +422,7 @@ void SensorService::cleanupConnection(SensorEventConnection* c) } } mActiveConnections.remove(connection); + BatteryService::cleanup(c->getUid()); } status_t SensorService::enable(const sp& connection, @@ -458,6 +460,7 @@ status_t SensorService::enable(const sp& connection, if (err == NO_ERROR) { // connection now active if (connection->addSensor(handle)) { + BatteryService::enableSensor(connection->getUid(), handle); // the sensor was added (which means it wasn't already there) // so, see if this connection becomes active if (mActiveConnections.indexOf(connection) < 0) { @@ -483,7 +486,9 @@ status_t SensorService::disable(const sp& connection, SensorRecord* rec = mActiveSensors.valueFor(handle); if (rec) { // see if this connection becomes inactive - connection->removeSensor(handle); + if (connection->removeSensor(handle)) { + BatteryService::disableSensor(connection->getUid(), handle); + } if (connection->hasAnySensor() == false) { mActiveConnections.remove(connection); }