Adds guardrail for memory usage for statsd uid map.
Checks if current memory usage of uid map is above a configured limit
and if so, we start deleting snapshots. If there are no more
snapshots, we begin deleting two of the deltas. Also records stats
in the guardrail StatsdStats. Also fixes an edge case where a config
is added after the snapshots are added. We request a snapshot of all
installed uid's at that moment. Finally, adds the uid map memory size
when determining if we should send a broadcast to trigger collection.
Test: Added unit-tests and check they pass on marlin.
Change-Id: Id5d86378bd1efe12a06b409164c777c0c6f4e3ab
diff --git a/cmds/statsd/src/StatsLogProcessor.cpp b/cmds/statsd/src/StatsLogProcessor.cpp
index 57b4fc1..2fd7947 100644
--- a/cmds/statsd/src/StatsLogProcessor.cpp
+++ b/cmds/statsd/src/StatsLogProcessor.cpp
@@ -220,7 +220,7 @@
const unique_ptr<MetricsManager>& metricsManager) {
std::lock_guard<std::mutex> lock(mBroadcastTimesMutex);
- size_t totalBytes = metricsManager->byteSize();
+ size_t totalBytes = metricsManager->byteSize() + mUidMap->getBytesUsed();
if (totalBytes > .9 * kMaxSerializedBytes) { // Send broadcast so that receivers can pull data.
auto lastFlushNs = mLastBroadcastTimes.find(key);
if (lastFlushNs != mLastBroadcastTimes.end()) {
diff --git a/cmds/statsd/src/StatsService.cpp b/cmds/statsd/src/StatsService.cpp
index c019d5d..7eca5aa 100644
--- a/cmds/statsd/src/StatsService.cpp
+++ b/cmds/statsd/src/StatsService.cpp
@@ -480,7 +480,7 @@
fprintf(out, "Config %s uses %zu bytes\n", key.ToString().c_str(),
mProcessor->GetMetricsSize(key));
}
- fprintf(out, "Detailed statsd stats in logcat...");
+ fprintf(out, "Detailed statsd stats in logcat...\n");
StatsdStats& statsdStats = StatsdStats::getInstance();
bool reset = false;
if (args.size() > 1) {
diff --git a/cmds/statsd/src/guardrail/StatsdStats.cpp b/cmds/statsd/src/guardrail/StatsdStats.cpp
index 2bd3612..2957457 100644
--- a/cmds/statsd/src/guardrail/StatsdStats.cpp
+++ b/cmds/statsd/src/guardrail/StatsdStats.cpp
@@ -45,6 +45,7 @@
const int FIELD_ID_CONDITION_STATS = 5;
const int FIELD_ID_METRIC_STATS = 6;
const int FIELD_ID_ATOM_STATS = 7;
+const int FIELD_ID_UIDMAP_STATS = 8;
const int FIELD_ID_MATCHER_STATS_NAME = 1;
const int FIELD_ID_MATCHER_STATS_COUNT = 2;
@@ -173,6 +174,27 @@
it->second.add_dump_report_time_sec(timeSec);
}
+void StatsdStats::noteUidMapDropped(int snapshots, int deltas) {
+ lock_guard<std::mutex> lock(mLock);
+ mUidMapStats.set_dropped_snapshots(mUidMapStats.dropped_snapshots() + snapshots);
+ mUidMapStats.set_dropped_changes(mUidMapStats.dropped_changes() + deltas);
+}
+
+void StatsdStats::setUidMapSnapshots(int snapshots) {
+ lock_guard<std::mutex> lock(mLock);
+ mUidMapStats.set_snapshots(snapshots);
+}
+
+void StatsdStats::setUidMapChanges(int changes) {
+ lock_guard<std::mutex> lock(mLock);
+ mUidMapStats.set_changes(changes);
+}
+
+void StatsdStats::setCurrentUidMapMemory(int bytes) {
+ lock_guard<std::mutex> lock(mLock);
+ mUidMapStats.set_bytes_used(bytes);
+}
+
void StatsdStats::noteConditionDimensionSize(const ConfigKey& key, const string& name, int size) {
lock_guard<std::mutex> lock(mLock);
// if name doesn't exist before, it will create the key with count 0.
@@ -364,6 +386,15 @@
}
}
+ const int numBytes = mUidMapStats.ByteSize();
+ vector<char> buffer(numBytes);
+ mUidMapStats.SerializeToArray(&buffer[0], numBytes);
+ proto.write(FIELD_TYPE_MESSAGE | FIELD_ID_UIDMAP_STATS, &buffer[0], buffer.size());
+ VLOG("UID map stats: bytes=%d, snapshots=%d, changes=%d, snapshots lost=%d, changes "
+ "lost=%d",
+ mUidMapStats.bytes_used(), mUidMapStats.snapshots(), mUidMapStats.changes(),
+ mUidMapStats.dropped_snapshots(), mUidMapStats.dropped_changes());
+
output->clear();
size_t bufferSize = proto.size();
output->resize(bufferSize);
diff --git a/cmds/statsd/src/guardrail/StatsdStats.h b/cmds/statsd/src/guardrail/StatsdStats.h
index 451144f..d6f6566 100644
--- a/cmds/statsd/src/guardrail/StatsdStats.h
+++ b/cmds/statsd/src/guardrail/StatsdStats.h
@@ -45,6 +45,10 @@
const static int kMaxTimestampCount = 20;
+ // Cap the UID map's memory usage to this. This should be fairly high since the UID information
+ // is critical for understanding the metrics.
+ const static size_t kMaxBytesUsedUidMap = 50 * 1024;
+
/**
* Report a new config has been received and report the static stats about the config.
*
@@ -113,6 +117,18 @@
void noteAtomLogged(int atomId, int32_t timeSec);
/**
+ * Records the number of snapshot and delta entries that are being dropped from the uid map.
+ */
+ void noteUidMapDropped(int snapshots, int deltas);
+
+ /**
+ * Updates the number of snapshots currently stored in the uid map.
+ */
+ void setUidMapSnapshots(int snapshots);
+ void setUidMapChanges(int changes);
+ void setCurrentUidMapMemory(int bytes);
+
+ /**
* Reset the historical stats. Including all stats in icebox, and the tracked stats about
* metrics, matchers, and atoms. The active configs will be kept and StatsdStats will continue
* to collect stats after reset() has been called.
@@ -133,6 +149,9 @@
int32_t mStartTimeSec;
+ // Track the number of dropped entries used by the uid map.
+ StatsdStatsReport_UidMapStats mUidMapStats;
+
// The stats about the configs that are still in use.
std::map<const ConfigKey, StatsdStatsReport_ConfigStats> mConfigStats;
diff --git a/cmds/statsd/src/packages/UidMap.cpp b/cmds/statsd/src/packages/UidMap.cpp
index 6c32d3e..db592e2 100644
--- a/cmds/statsd/src/packages/UidMap.cpp
+++ b/cmds/statsd/src/packages/UidMap.cpp
@@ -13,11 +13,14 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-
+#define DEBUG true // STOPSHIP if true
#include "Log.h"
+#include "guardrail/StatsdStats.h"
#include "packages/UidMap.h"
+#include <android/os/IStatsCompanionService.h>
+#include <binder/IServiceManager.h>
#include <utils/Errors.h>
using namespace android;
@@ -26,6 +29,11 @@
namespace os {
namespace statsd {
+UidMap::UidMap() : mBytesUsed(0) {
+}
+UidMap::~UidMap() {
+}
+
bool UidMap::hasApp(int uid, const string& packageName) const {
lock_guard<mutex> lock(mMutex);
@@ -73,6 +81,10 @@
t->set_version(int(versionCode[j]));
t->set_uid(uid[j]);
}
+ mBytesUsed += snapshot->ByteSize();
+ StatsdStats::getInstance().setCurrentUidMapMemory(mBytesUsed);
+ StatsdStats::getInstance().setUidMapSnapshots(mOutput.snapshots_size());
+ ensureBytesUsedBelowLimit();
}
void UidMap::updateApp(const String16& app_16, const int32_t& uid, const int32_t& versionCode) {
@@ -96,6 +108,10 @@
log->set_app(app);
log->set_uid(uid);
log->set_version(versionCode);
+ mBytesUsed += log->ByteSize();
+ StatsdStats::getInstance().setCurrentUidMapMemory(mBytesUsed);
+ StatsdStats::getInstance().setUidMapChanges(mOutput.changes_size());
+ ensureBytesUsedBelowLimit();
auto range = mMap.equal_range(int(uid));
for (auto it = range.first; it != range.second; ++it) {
@@ -103,7 +119,7 @@
it->second.versionCode = int(versionCode);
return;
}
- ALOGD("updateApp failed to find the app %s with uid %i to update", app.c_str(), uid);
+ VLOG("updateApp failed to find the app %s with uid %i to update", app.c_str(), uid);
return;
}
@@ -111,6 +127,28 @@
mMap.insert(make_pair(uid, AppData(app, int(versionCode))));
}
+void UidMap::ensureBytesUsedBelowLimit() {
+ size_t limit;
+ if (maxBytesOverride <= 0) {
+ limit = StatsdStats::kMaxBytesUsedUidMap;
+ } else {
+ limit = maxBytesOverride;
+ }
+ while (mBytesUsed > limit) {
+ VLOG("Bytes used %zu is above limit %zu, need to delete something", mBytesUsed, limit);
+ if (mOutput.snapshots_size() > 0) {
+ auto snapshots = mOutput.mutable_snapshots();
+ snapshots->erase(snapshots->begin()); // Remove first snapshot.
+ StatsdStats::getInstance().noteUidMapDropped(1, 0);
+ } else if (mOutput.changes_size() > 0) {
+ auto changes = mOutput.mutable_changes();
+ changes->DeleteSubrange(0, 1);
+ StatsdStats::getInstance().noteUidMapDropped(0, 1);
+ }
+ mBytesUsed = mOutput.ByteSize();
+ }
+}
+
void UidMap::removeApp(const String16& app_16, const int32_t& uid) {
removeApp(time(nullptr) * NS_PER_SEC, app_16, uid);
}
@@ -128,6 +166,10 @@
log->set_timestamp_nanos(timestamp);
log->set_app(app);
log->set_uid(uid);
+ mBytesUsed += log->ByteSize();
+ StatsdStats::getInstance().setCurrentUidMapMemory(mBytesUsed);
+ StatsdStats::getInstance().setUidMapChanges(mOutput.changes_size());
+ ensureBytesUsedBelowLimit();
auto range = mMap.equal_range(int(uid));
for (auto it = range.first; it != range.second; ++it) {
@@ -136,7 +178,7 @@
return;
}
}
- ALOGD("removeApp failed to find the app %s with uid %i to remove", app.c_str(), uid);
+ VLOG("removeApp failed to find the app %s with uid %i to remove", app.c_str(), uid);
return;
}
@@ -177,7 +219,6 @@
void UidMap::clearOutput() {
mOutput.Clear();
-
// Re-initialize the initial state for the outputs. This results in extra data being uploaded
// but helps ensure we can re-construct the UID->app name, versionCode mapping in server.
auto snapshot = mOutput.add_snapshots();
@@ -187,6 +228,12 @@
t->set_version(it.second.versionCode);
t->set_uid(it.first);
}
+
+ // Also update the guardrail trackers.
+ StatsdStats::getInstance().setUidMapChanges(0);
+ StatsdStats::getInstance().setUidMapSnapshots(1);
+ mBytesUsed = snapshot->ByteSize();
+ StatsdStats::getInstance().setCurrentUidMapMemory(mBytesUsed);
}
int64_t UidMap::getMinimumTimestampNs() {
@@ -201,6 +248,10 @@
return m;
}
+size_t UidMap::getBytesUsed() {
+ return mBytesUsed;
+}
+
UidMapping UidMap::getOutput(const ConfigKey& key) {
return getOutput(time(nullptr) * NS_PER_SEC, key);
}
@@ -236,6 +287,10 @@
}
}
}
+ mBytesUsed = mOutput.ByteSize(); // Compute actual size after potential deletions.
+ StatsdStats::getInstance().setCurrentUidMapMemory(mBytesUsed);
+ StatsdStats::getInstance().setUidMapChanges(mOutput.changes_size());
+ StatsdStats::getInstance().setUidMapSnapshots(mOutput.snapshots_size());
return ret;
}
@@ -250,6 +305,23 @@
void UidMap::OnConfigUpdated(const ConfigKey& key) {
mLastUpdatePerConfigKey[key] = -1;
+
+ // Ensure there is at least one snapshot available since this configuration also needs to know
+ // what all the uid's represent.
+ if (mOutput.snapshots_size() == 0) {
+ sp<IStatsCompanionService> statsCompanion = nullptr;
+ // Get statscompanion service from service manager
+ const sp<IServiceManager> sm(defaultServiceManager());
+ if (sm != nullptr) {
+ const String16 name("statscompanion");
+ statsCompanion = interface_cast<IStatsCompanionService>(sm->checkService(name));
+ if (statsCompanion == nullptr) {
+ ALOGW("statscompanion service unavailable!");
+ return;
+ }
+ statsCompanion->triggerUidSnapshot();
+ }
+ }
}
void UidMap::OnConfigRemoved(const ConfigKey& key) {
diff --git a/cmds/statsd/src/packages/UidMap.h b/cmds/statsd/src/packages/UidMap.h
index 24eb966..d2971c9 100644
--- a/cmds/statsd/src/packages/UidMap.h
+++ b/cmds/statsd/src/packages/UidMap.h
@@ -50,15 +50,16 @@
// at any given moment. This map must be updated by StatsCompanionService.
class UidMap : public virtual android::RefBase {
public:
+ UidMap();
+ ~UidMap();
+
/*
* All three inputs must be the same size, and the jth element in each array refers to the same
* tuple, ie. uid[j] corresponds to packageName[j] with versionCode[j].
*/
- // TODO: Add safeguards to call clearOutput if there's too much data already stored.
void updateMap(const vector<int32_t>& uid, const vector<int32_t>& versionCode,
const vector<String16>& packageName);
- // TODO: Add safeguards to call clearOutput if there's too much data already stored.
void updateApp(const String16& packageName, const int32_t& uid, const int32_t& versionCode);
void removeApp(const String16& packageName, const int32_t& uid);
@@ -98,11 +99,13 @@
// in case we lose a previous upload.
void clearOutput();
+ // Get currently cached value of memory used by UID map.
+ size_t getBytesUsed();
+
private:
void updateMap(const int64_t& timestamp, const vector<int32_t>& uid,
const vector<int32_t>& versionCode, const vector<String16>& packageName);
- // TODO: Add safeguards to call clearOutput if there's too much data already stored.
void updateApp(const int64_t& timestamp, const String16& packageName, const int32_t& uid,
const int32_t& versionCode);
void removeApp(const int64_t& timestamp, const String16& packageName, const int32_t& uid);
@@ -135,8 +138,22 @@
// Returns the minimum value from mConfigKeys.
int64_t getMinimumTimestampNs();
+ // If our current used bytes is above the limit, then we clear out the earliest snapshot. If
+ // there are no more snapshots, then we clear out the earliest delta. We repeat the deletions
+ // until the memory consumed by mOutput is below the specified limit.
+ void ensureBytesUsedBelowLimit();
+
+ // Override used for testing the max memory allowed by uid map. -1 means we use the value
+ // specified in StatsdStats.h with the rest of the guardrails.
+ size_t maxBytesOverride = -1;
+
+ // Cache the size of mOutput;
+ size_t mBytesUsed;
+
// Allows unit-test to access private methods.
FRIEND_TEST(UidMapTest, TestClearingOutput);
+ FRIEND_TEST(UidMapTest, TestMemoryComputed);
+ FRIEND_TEST(UidMapTest, TestMemoryGuardrail);
};
} // namespace statsd
diff --git a/cmds/statsd/src/stats_log.proto b/cmds/statsd/src/stats_log.proto
index e7e1d43..cc8a26d 100644
--- a/cmds/statsd/src/stats_log.proto
+++ b/cmds/statsd/src/stats_log.proto
@@ -220,4 +220,13 @@
}
repeated AtomStats atom_stats = 7;
+
+ message UidMapStats {
+ optional int32 snapshots = 1;
+ optional int32 changes = 2;
+ optional int32 bytes_used = 3;
+ optional int32 dropped_snapshots = 4;
+ optional int32 dropped_changes = 5;
+ }
+ optional UidMapStats uidmap_stats = 8;
}
\ No newline at end of file