storaged: remove protos from storaged class
protobuf is only needed when serializing/deserializing data. Instead of
maintaining a permanent buffer in storaged object, move the container to
stack so that the buffer is released when we don't need it. In addition,
we don't need to clear the buffer before updating it.
Also added a function to clear user io history when the user is removed.
Bug: 63740245
Change-Id: Ia5d19b9a0c3f92a93b061a56be89bb0b958a2a29
diff --git a/storaged/include/storaged.h b/storaged/include/storaged.h
index edaa32f..e5dd70d 100644
--- a/storaged/include/storaged.h
+++ b/storaged/include/storaged.h
@@ -84,12 +84,11 @@
sp<android::hardware::health::V2_0::IHealth> health;
unique_ptr<storage_info_t> storage_info;
static const uint32_t crc_init;
- unordered_map<int, storaged_proto::StoragedProto> protos;
- Mutex proto_mutex;
- void load_proto_locked(userid_t user_id);
- void prepare_proto(StoragedProto* proto, userid_t user_id);
- void flush_proto_locked(userid_t user_id);
- void flush_proto_user_system_locked(StoragedProto* proto);
+ unordered_map<userid_t, bool> proto_loaded;
+ void load_proto(userid_t user_id);
+ void prepare_proto(userid_t user_id, StoragedProto* proto);
+ void flush_proto(userid_t user_id, StoragedProto* proto);
+ void flush_proto_user_system(StoragedProto* proto);
string proto_path(userid_t user_id) {
return string("/data/misc_ce/") + to_string(user_id) +
"/storaged/storaged.proto";
@@ -116,7 +115,7 @@
map<uint64_t, struct uid_records> get_uid_records(
double hours, uint64_t threshold, bool force_report) {
- return mUidm.dump(hours, threshold, force_report, &protos);
+ return mUidm.dump(hours, threshold, force_report);
}
void update_uid_io_interval(int interval) {
@@ -135,7 +134,7 @@
void report_storage_info();
- void flush_protos();
+ void flush_protos(unordered_map<int, StoragedProto>* protos);
};
// Eventlog tag
diff --git a/storaged/include/storaged_uid_monitor.h b/storaged/include/storaged_uid_monitor.h
index 6310ae4..3a718fa 100644
--- a/storaged/include/storaged_uid_monitor.h
+++ b/storaged/include/storaged_uid_monitor.h
@@ -111,8 +111,7 @@
unordered_map<uint32_t, uid_info> get_uid_io_stats();
// called by dumpsys
map<uint64_t, struct uid_records> dump(
- double hours, uint64_t threshold, bool force_report,
- unordered_map<int, StoragedProto>* protos);
+ double hours, uint64_t threshold, bool force_report);
// called by battery properties listener
void set_charger_state(charger_stat_t stat);
// called by storaged periodic_chore or dump with force_report
@@ -120,6 +119,7 @@
void report(unordered_map<int, StoragedProto>* protos);
// restores io_history from protobuf
void load_uid_io_proto(const UidIOUsage& proto);
+ void clear_user_history(userid_t user_id);
};
#endif /* _STORAGED_UID_MONITOR_H_ */
diff --git a/storaged/storaged.cpp b/storaged/storaged.cpp
index e5130c3..915c095 100644
--- a/storaged/storaged.cpp
+++ b/storaged/storaged.cpp
@@ -165,19 +165,17 @@
}
void storaged_t::add_user_ce(userid_t user_id) {
- Mutex::Autolock _l(proto_mutex);
- protos.insert({user_id, {}});
- load_proto_locked(user_id);
- protos[user_id].set_loaded(1);
+ load_proto(user_id);
+ proto_loaded[user_id] = true;
}
void storaged_t::remove_user_ce(userid_t user_id) {
- Mutex::Autolock _l(proto_mutex);
- protos.erase(user_id);
+ proto_loaded[user_id] = false;
+ mUidm.clear_user_history(user_id);
RemoveFileIfExists(proto_path(user_id), nullptr);
}
-void storaged_t::load_proto_locked(userid_t user_id) {
+void storaged_t::load_proto(userid_t user_id) {
string proto_file = proto_path(user_id);
ifstream in(proto_file, ofstream::in | ofstream::binary);
@@ -185,32 +183,30 @@
stringstream ss;
ss << in.rdbuf();
- StoragedProto* proto = &protos[user_id];
- proto->Clear();
- proto->ParseFromString(ss.str());
+ StoragedProto proto;
+ proto.ParseFromString(ss.str());
- uint32_t crc = proto->crc();
- proto->set_crc(crc_init);
- string proto_str = proto->SerializeAsString();
+ uint32_t crc = proto.crc();
+ proto.set_crc(crc_init);
+ string proto_str = proto.SerializeAsString();
uint32_t computed_crc = crc32(crc_init,
reinterpret_cast<const Bytef*>(proto_str.c_str()),
proto_str.size());
if (crc != computed_crc) {
LOG_TO(SYSTEM, WARNING) << "CRC mismatch in " << proto_file;
- proto->Clear();
return;
}
- mUidm.load_uid_io_proto(proto->uid_io_usage());
+ mUidm.load_uid_io_proto(proto.uid_io_usage());
if (user_id == USER_SYSTEM) {
- storage_info->load_perf_history_proto(proto->perf_history());
+ storage_info->load_perf_history_proto(proto.perf_history());
}
}
-void storaged_t:: prepare_proto(StoragedProto* proto, userid_t user_id) {
- proto->set_version(2);
+void storaged_t:: prepare_proto(userid_t user_id, StoragedProto* proto) {
+ proto->set_version(3);
proto->set_crc(crc_init);
if (user_id == USER_SYSTEM) {
@@ -225,7 +221,7 @@
proto_str.size()));
}
-void storaged_t::flush_proto_user_system_locked(StoragedProto* proto) {
+void storaged_t::flush_proto_user_system(StoragedProto* proto) {
string proto_str = proto->SerializeAsString();
const char* data = proto_str.data();
uint32_t size = proto_str.size();
@@ -274,11 +270,11 @@
rename(tmp_file.c_str(), proto_file.c_str());
}
-void storaged_t::flush_proto_locked(userid_t user_id) {
- StoragedProto* proto = &protos[user_id];
- prepare_proto(proto, user_id);
+void storaged_t::flush_proto(userid_t user_id, StoragedProto* proto) {
+ prepare_proto(user_id, proto);
+
if (user_id == USER_SYSTEM) {
- flush_proto_user_system_locked(proto);
+ flush_proto_user_system(proto);
return;
}
@@ -293,21 +289,20 @@
rename(tmp_file.c_str(), proto_file.c_str());
}
-void storaged_t::flush_protos() {
- Mutex::Autolock _l(proto_mutex);
- for (const auto& it : protos) {
+void storaged_t::flush_protos(unordered_map<int, StoragedProto>* protos) {
+ for (auto& it : *protos) {
/*
- * Don't flush proto if we haven't loaded it from file and combined
- * with data in memory.
+ * Don't flush proto if we haven't attempted to load it from file.
*/
- if (it.second.loaded() != 1) {
- continue;
+ if (proto_loaded[it.first]) {
+ flush_proto(it.first, &it.second);
}
- flush_proto_locked(it.first);
}
}
void storaged_t::event(void) {
+ unordered_map<int, StoragedProto> protos;
+
if (mDsm.enabled()) {
mDsm.update();
if (!(mTimer % mConfig.periodic_chores_interval_disk_stats_publish)) {
@@ -316,17 +311,15 @@
}
if (!(mTimer % mConfig.periodic_chores_interval_uid_io)) {
- Mutex::Autolock _l(proto_mutex);
mUidm.report(&protos);
}
if (storage_info) {
- Mutex::Autolock _l(proto_mutex);
storage_info->refresh(protos[USER_SYSTEM].mutable_perf_history());
}
if (!(mTimer % mConfig.periodic_chores_interval_flush_proto)) {
- flush_protos();
+ flush_protos(&protos);
}
mTimer += mConfig.periodic_chores_interval_unit;
diff --git a/storaged/storaged.proto b/storaged/storaged.proto
index 18869fa..9dcd79e 100644
--- a/storaged/storaged.proto
+++ b/storaged/storaged.proto
@@ -54,8 +54,7 @@
message StoragedProto {
optional uint32 crc = 1;
optional uint32 version = 2;
- optional uint32 loaded = 3;
- optional UidIOUsage uid_io_usage = 4;
- optional IOPerfHistory perf_history = 5;
- repeated uint32 padding = 6;
+ optional UidIOUsage uid_io_usage = 3;
+ optional IOPerfHistory perf_history = 4;
+ repeated uint32 padding = 5;
}
diff --git a/storaged/storaged_info.cpp b/storaged/storaged_info.cpp
index 3b5edbb..036d7e1 100644
--- a/storaged/storaged_info.cpp
+++ b/storaged/storaged_info.cpp
@@ -68,6 +68,8 @@
void storage_info_t::load_perf_history_proto(const IOPerfHistory& perf_history)
{
+ Mutex::Autolock _l(si_mutex);
+
if (!perf_history.has_day_start_sec() ||
perf_history.daily_perf_size() > (int)daily_perf.size() ||
perf_history.weekly_perf_size() > (int)weekly_perf.size()) {
diff --git a/storaged/storaged_uid_monitor.cpp b/storaged/storaged_uid_monitor.cpp
index ab1c16e..5745782 100644
--- a/storaged/storaged_uid_monitor.cpp
+++ b/storaged/storaged_uid_monitor.cpp
@@ -257,11 +257,10 @@
}
std::map<uint64_t, struct uid_records> uid_monitor::dump(
- double hours, uint64_t threshold, bool force_report,
- unordered_map<int, StoragedProto>* protos)
+ double hours, uint64_t threshold, bool force_report)
{
if (force_report) {
- report(protos);
+ report(nullptr);
}
Mutex::Autolock _l(uidm_mutex);
@@ -374,7 +373,9 @@
update_curr_io_stats_locked();
add_records_locked(time(NULL));
- update_uid_io_proto(protos);
+ if (protos) {
+ update_uid_io_proto(protos);
+ }
}
namespace {
@@ -407,10 +408,6 @@
void uid_monitor::update_uid_io_proto(unordered_map<int, StoragedProto>* protos)
{
- for (auto& it : *protos) {
- it.second.mutable_uid_io_usage()->Clear();
- }
-
for (const auto& item : io_history) {
const uint64_t& end_ts = item.first;
const struct uid_records& recs = item.second;
@@ -449,10 +446,34 @@
}
}
+void uid_monitor::clear_user_history(userid_t user_id)
+{
+ Mutex::Autolock _l(uidm_mutex);
+
+ for (auto& item : io_history) {
+ vector<uid_record>* entries = &item.second.entries;
+ entries->erase(
+ remove_if(entries->begin(), entries->end(),
+ [user_id](const uid_record& rec) {
+ return rec.ios.user_id == user_id;}),
+ entries->end());
+ }
+
+ for (auto it = io_history.begin(); it != io_history.end(); ) {
+ if (it->second.entries.empty()) {
+ it = io_history.erase(it);
+ } else {
+ it++;
+ }
+ }
+}
+
void uid_monitor::load_uid_io_proto(const UidIOUsage& uid_io_proto)
{
if (!enabled()) return;
+ Mutex::Autolock _l(uidm_mutex);
+
for (const auto& item_proto : uid_io_proto.uid_io_items()) {
const UidIORecords& records_proto = item_proto.records();
struct uid_records* recs = &io_history[item_proto.end_ts()];
diff --git a/storaged/tests/storaged_test.cpp b/storaged/tests/storaged_test.cpp
index 9281193..6a5fc61 100644
--- a/storaged/tests/storaged_test.cpp
+++ b/storaged/tests/storaged_test.cpp
@@ -449,17 +449,7 @@
},
};
- StoragedProto proto_0;
- UidIOItem* item = proto_0.mutable_uid_io_usage()->add_uid_io_items();
- item->set_end_ts(200);
- item->mutable_records()->set_start_ts(100);
- UidRecord* rec = item->mutable_records()->add_entries();
- rec->set_uid_name("app1");
- rec->set_user_id(0);
- rec->mutable_uid_io()->set_wr_fg_chg_on(1000);
-
unordered_map<int, StoragedProto> protos;
- protos[0] = proto_0;
uidm.update_uid_io_proto(&protos);
@@ -589,4 +579,17 @@
EXPECT_EQ(merged_entries_2.size(), 1UL);
EXPECT_EQ(merged_entries_2.count("app1"), 1UL);
EXPECT_EQ(merged_entries_2["app1"].bytes[WRITE][FOREGROUND][CHARGER_ON], 1000UL);
+
+ uidm.clear_user_history(0);
+
+ EXPECT_EQ(uidm.io_history.size(), 2UL);
+ EXPECT_EQ(uidm.io_history.count(200), 1UL);
+ EXPECT_EQ(uidm.io_history.count(300), 1UL);
+
+ EXPECT_EQ(uidm.io_history[200].entries.size(), 1UL);
+ EXPECT_EQ(uidm.io_history[300].entries.size(), 1UL);
+
+ uidm.clear_user_history(1);
+
+ EXPECT_EQ(uidm.io_history.size(), 0UL);
}