Revert "powerstats: timeout mechanism for AoC data provider"
Revert submission 17800080-power-stats-aoc-timeout
Reason for revert: Causing crashes due to improper thread sync
Reverted Changes:
I71a9e6817:powerstats: timeout mechanism for AoC data provide...
I725c00745:powerstats: set time limit as timeout for AoC
I5b290a53d:powerstats: set time limit as timeout for AoC
Change-Id: I6844a141739f93154d10debc29c7565f92349d25
diff --git a/powerstats/AocStateResidencyDataProvider.cpp b/powerstats/AocStateResidencyDataProvider.cpp
index 574a23d..c64496d 100644
--- a/powerstats/AocStateResidencyDataProvider.cpp
+++ b/powerstats/AocStateResidencyDataProvider.cpp
@@ -17,8 +17,6 @@
#include "AocStateResidencyDataProvider.h"
#include <android-base/logging.h>
-#include <chrono>
-#include <pthread.h>
namespace aidl {
namespace android {
@@ -26,23 +24,12 @@
namespace power {
namespace stats {
-struct async_data_t {
- pthread_cond_t *cond;
- pthread_mutex_t *lock;
- uint64_t timeoutMillis;
- std::unordered_map<std::string, std::vector<StateResidency>> *residencies;
- std::unordered_map<std::string,
- std::vector<std::unique_ptr<GenericStateResidencyDataProvider>>> *providers;
-};
-
AocStateResidencyDataProvider::AocStateResidencyDataProvider(std::vector<std::pair<std::string,
- std::string>> ids, std::vector<std::pair<std::string, std::string>> states,
- const uint64_t timeoutMillis) {
+ std::string>> ids, std::vector<std::pair<std::string, std::string>> states) {
// AoC stats are reported in ticks of 244.140625ns. The transform
// function converts ticks to milliseconds.
// 1000000 / 244.140625 = 4096.
static const uint64_t AOC_CLK = 4096;
- static const uint64_t TIMEOUT_MILLIS = 120;
std::function<uint64_t(uint64_t)> aocTickToMs = [](uint64_t a) { return a / AOC_CLK; };
GenericStateResidencyDataProvider::StateResidencyConfig config = {
.entryCountSupported = true,
@@ -67,17 +54,13 @@
mProviders[id.first].push_back(std::move(sdp));
}
}
- mStateCount = states.size();
- mTimeoutMillis = timeoutMillis == 0 ? TIMEOUT_MILLIS : timeoutMillis;
}
-void *getStateResidenciesAsync(void *arg) {
- struct timespec start, end;
- struct async_data_t *async = (struct async_data_t*)arg;
- uint64_t timeoutUs = async->timeoutMillis * 1000;
-
+bool AocStateResidencyDataProvider::getStateResidencies(
+ std::unordered_map<std::string, std::vector<StateResidency>> *residencies) {
// States from the same power entity are merged.
- for (const auto &providerList : *async->providers) {
+ bool ret = true;
+ for (const auto &providerList : mProviders) {
int32_t stateId = 0;
std::string curEntity = providerList.first;
std::vector<StateResidency> stateResidencies;
@@ -85,18 +68,7 @@
// Iterate over each provider in the providerList, appending each of the states
for (const auto &provider : providerList.second) {
std::unordered_map<std::string, std::vector<StateResidency>> residency;
-
- clock_gettime(CLOCK_REALTIME, &start);
- provider->getStateResidencies(&residency);
- clock_gettime(CLOCK_REALTIME, &end);
- uint64_t elapsedUs =
- ((end.tv_sec - start.tv_sec) * 1000000) + ((end.tv_nsec - start.tv_nsec) / 1000);
-
- if (elapsedUs >= timeoutUs) {
- LOG(WARNING) << "getStateResidencies latency for " << curEntity
- << " exceeds time allowed: " << elapsedUs << " us";
- return 0;
- }
+ ret &= provider->getStateResidencies(&residency);
// Each provider should only return data for curEntity but checking anyway
if (residency.find(curEntity) != residency.end()) {
@@ -112,64 +84,8 @@
}
}
- async->residencies->emplace(curEntity, stateResidencies);
+ residencies->emplace(curEntity, stateResidencies);
}
-
- pthread_mutex_lock(async->lock);
- pthread_cond_signal(async->cond);
- pthread_mutex_unlock(async->lock);
-
- return 0;
-}
-
-bool AocStateResidencyDataProvider::getStateResidencies(
- std::unordered_map<std::string, std::vector<StateResidency>> *residencies) {
- bool ret = true;
- int condResult = 0;
- pthread_t tid;
- pthread_cond_t cond = PTHREAD_COND_INITIALIZER;
- pthread_mutex_t lock = PTHREAD_MUTEX_INITIALIZER;
- std::unordered_map<std::string, std::vector<StateResidency>> stateResidencies;
- struct timespec start, timeout;
- struct async_data_t async = {
- .cond = &cond,
- .lock = &lock,
- .timeoutMillis = mTimeoutMillis,
- .residencies = &stateResidencies,
- .providers = &mProviders
- };
-
- pthread_create(&tid, NULL, &getStateResidenciesAsync, (void*)&async);
-
- clock_gettime(CLOCK_REALTIME, &start);
-
- uint64_t expirationMillis = mTimeoutMillis * mStateCount;
- timeout.tv_sec = start.tv_sec + expirationMillis / 1000;
- uint64_t nsec = start.tv_nsec + (expirationMillis % 1000) * 1000000;
- if (nsec < 1000000000) {
- timeout.tv_nsec = nsec;
- } else {
- timeout.tv_sec += 1;
- timeout.tv_nsec = nsec - 1000000000;
- }
-
- pthread_mutex_lock(&lock);
- condResult = pthread_cond_timedwait(&cond, &lock, &timeout);
- pthread_mutex_unlock(&lock);
-
- if (condResult != 0) {
- if (condResult == ETIMEDOUT) {
- LOG(WARNING) << __func__ << " latency for AoC timeout: " << expirationMillis << " ms";
- } else {
- LOG(ERROR) << "Failed to wait for the condition variable: " << condResult;
- }
- ret = false;
- } else {
- for (const auto &residency : stateResidencies) {
- residencies->emplace(residency.first, residency.second);
- }
- }
-
return ret;
}