openkylin
/
platform_system_core
mirror of https://gitee.com/openkylin/platform_system_core.git
9
0
Fork 0
Code Issues Projects Releases Wiki Activity

logd: remove faulty optimization 

TagNameKey contains a pointer to a std::string and a std::string_view,
such it can both own a string or reference a different string.  This
is meant to be a memory optimization.

This, however, is actually a net pessimization.  Due to these three
below cases and typical usage pattern.

Cases:
1) In the case where TagNameKey owns the string, 3 words are wasted,
one for the pointer and two for the std::string_view.

2) In the case where TagNameKey references a short string, the same 3
words are wasted.  This is because std::string has a feature called
"Short String Optimization" that means std::string does not allocate
for strings of sizes <= 10 on 32bit devices and <= 22 on 64bit
devices.

3) In the case where TagNameKey references a longer string than the
"Short String Optimization" limits, then this saves the string's
length in bytes.

Usage pattern:
After boot on 32 bit cuttlefish, there were 679 entries for the first
case, and only 69 in the third case.  The 679 entries have an overhead
of 679 * 3 * sizeof(void*) = 679 * 12 = 8148 bytes.  The 69 strings in
the third case have a total length and therefore savings of 1352
bytes.  This is a net pessimization of 6796 bytes.

I expect this same ratio to be similar throughout the device's uptime.

This situation is worse on 64 bit devices.  If cuttlefish were 64 bit,
then there would have been only 18 items in the third case due to the
larger "Short String Optimization" capacity, and the cost for the
first case would have doubled.

Given the above and the cost of maintaining extra code, this
optimization is removed and a std::string is used as the hash table
key instead.

Test: logging unit tests
Change-Id: I957c519b19edca4f7fc531d96b7144cf68bf4e16
This commit is contained in:
Tom Cherry 2020-06-03 15:38:32 -07:00
parent 4596b78d12
commit b0263af5a8
2 changed files with 39 additions and 94 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

27
logd/LogStatistics.cpp
View File

@ -36,6 +36,30 @@ static const uint64_t monthSec = 31 * 24 * hourSec;
std::atomic<size_t> LogStatistics::SizesTotal;
static std::string TagNameKey(const LogStatisticsElement& element) {
if (IsBinary(element.log_id)) {
uint32_t tag = element.tag;
if (tag) {
const char* cp = android::tagToName(tag);
if (cp) {
return std::string(cp);
}
}
return android::base::StringPrintf("[%" PRIu32 "]", tag);
}
const char* msg = element.msg;
if (!msg) {
return "chatty";
}
++msg;
uint16_t len = element.msg_len;
len = (len <= 1) ? 0 : strnlen(msg, len - 1);
if (!len) {
return "<NULL>";
}
return std::string(msg, len);
}
LogStatistics::LogStatistics(bool enable_statistics) : enable(enable_statistics) {
log_time now(CLOCK_REALTIME);
log_id_for_each(id) {
@ -597,7 +621,8 @@ std::string TagNameEntry::formatHeader(const std::string& name,
std::string("BYTES"), std::string(""));
}
std::string TagNameEntry::format(const LogStatistics&, log_id_t, const TagNameKey& key_name) const {
std::string TagNameEntry::format(const LogStatistics&, log_id_t,
const std::string& key_name) const {
std::string name;
std::string pidstr;
if (pid_ != (pid_t)-1) {

106
logd/LogStatistics.h
View File

@ -397,93 +397,6 @@ class TagEntry : public EntryBaseDropped {
uid_t uid_;
};
struct TagNameKey {
std::string* alloc;
std::string_view name; // Saves space if const char*
explicit TagNameKey(const LogStatisticsElement& element)
: alloc(nullptr), name("", strlen("")) {
if (IsBinary(element.log_id)) {
uint32_t tag = element.tag;
if (tag) {
const char* cp = android::tagToName(tag);
if (cp) {
name = std::string_view(cp, strlen(cp));
return;
}
}
alloc = new std::string(
android::base::StringPrintf("[%" PRIu32 "]", tag));
if (!alloc) return;
name = std::string_view(alloc->c_str(), alloc->size());
return;
}
const char* msg = element.msg;
if (!msg) {
name = std::string_view("chatty", strlen("chatty"));
return;
}
++msg;
uint16_t len = element.msg_len;
len = (len <= 1) ? 0 : strnlen(msg, len - 1);
if (!len) {
name = std::string_view("<NULL>", strlen("<NULL>"));
return;
}
alloc = new std::string(msg, len);
if (!alloc) return;
name = std::string_view(alloc->c_str(), alloc->size());
}
explicit TagNameKey(TagNameKey&& rval) noexcept
: alloc(rval.alloc), name(rval.name.data(), rval.name.length()) {
rval.alloc = nullptr;
}
explicit TagNameKey(const TagNameKey& rval)
: alloc(rval.alloc ? new std::string(*rval.alloc) : nullptr),
name(alloc ? alloc->data() : rval.name.data(), rval.name.length()) {
}
~TagNameKey() {
if (alloc) delete alloc;
}
operator const std::string_view() const {
return name;
}
const char* data() const {
return name.data();
}
size_t length() const {
return name.length();
}
bool operator==(const TagNameKey& rval) const {
if (length() != rval.length()) return false;
if (length() == 0) return true;
return fastcmp<strncmp>(data(), rval.data(), length()) == 0;
}
bool operator!=(const TagNameKey& rval) const {
return !(*this == rval);
}
size_t getAllocLength() const {
return alloc ? alloc->length() + 1 + sizeof(std::string) : 0;
}
};
// Hash for TagNameKey
template <>
struct std::hash<TagNameKey>
: public std::unary_function<const TagNameKey&, size_t> {
size_t operator()(const TagNameKey& __t) const noexcept {
if (!__t.length()) return 0;
return std::hash<std::string_view>()(std::string_view(__t));
}
};
class TagNameEntry : public EntryBase {
public:
explicit TagNameEntry(const LogStatisticsElement& element)
@ -507,7 +420,7 @@ class TagNameEntry : public EntryBase {
}
std::string formatHeader(const std::string& name, log_id_t id) const;
std::string format(const LogStatistics& stat, log_id_t id, const TagNameKey& key_name) const;
std::string format(const LogStatistics& stat, log_id_t id, const std::string& key_name) const;
private:
pid_t tid_;
@ -515,7 +428,6 @@ class TagNameEntry : public EntryBase {
uid_t uid_;
};
// Log Statistics
class LogStatistics {
friend UidEntry;
friend PidEntry;
@ -556,7 +468,7 @@ class LogStatistics {
tagTable_t securityTagTable GUARDED_BY(lock_);
// global tag list
typedef LogHashtable<TagNameKey, TagNameEntry> tagNameTable_t;
typedef LogHashtable<std::string, TagNameEntry> tagNameTable_t;
tagNameTable_t tagNameTable;
size_t sizeOf() const REQUIRES(lock_) {
@ -573,13 +485,21 @@ class LogStatistics {
const char* name = it.second.name();
if (name) size += strlen(name) + 1;
}
for (auto it : tagNameTable) size += it.first.getAllocLength();
for (auto it : tagNameTable) {
size += sizeof(std::string);
size_t len = it.first.size();
// Account for short string optimization: if the string's length is <= 22 bytes for 64
// bit or <= 10 bytes for 32 bit, then there is no additional allocation.
if ((sizeof(std::string) == 24 && len > 22) ||
(sizeof(std::string) != 24 && len > 10)) {
size += len;
}
}
log_id_for_each(id) {
size += uidTable[id].sizeOf();
size += uidTable[id].size() * sizeof(uidTable_t::iterator);
size += pidSystemTable[id].sizeOf();
size +=
pidSystemTable[id].size() * sizeof(pidSystemTable_t::iterator);
size += pidSystemTable[id].size() * sizeof(pidSystemTable_t::iterator);
}
return size;
}