From 700fb87648047d23c947d9020fd35cd47621dc7c Mon Sep 17 00:00:00 2001 From: Mark Salyzyn Date: Wed, 21 Sep 2016 09:24:13 -0700 Subject: [PATCH] liblog: event_tag_map use unordered_map >3 times faster access for lookup functions. Code simplification and C++ coding style conversion. android_lookupEventTagNum has been expanded to utilize the Format match, and then drops down to just Tag match. Performance testing on Hikey, all services stopped, shell only access, CPU not locked had good repeatability. $ adb reboot ; adb wait-for-device ; adb shell su root stop Before this adjustment using binary search: $ /data/nativetest64/liblog-benchmarks/liblog-benchmarks BM_lookupEvent* iterations ns/op Precharge: start Precharge: stop 231 BM_lookupEventTag 10000000 153 BM_lookupEventTag_NOT 20000000 139 BM_lookupEventFormat 10000000 153 After this adjustment strictly using std::unordered_map lookup: $ /data/nativetest64/liblog-benchmarks/liblog-benchmarks BM_lookupEvent* iterations ns/op Precharge: start Precharge: stop 231 BM_lookupEventTag 50000000 56 BM_lookupEventTag_NOT 100000000 21 BM_lookupEventFormat 50000000 56 Test: gTest logd-unit-tests and liblog-unit-tests, liblog-benchmarks. Bug: 31456426 Change-Id: I1cfc3d641c3ea9ab08d455efe0c844d3a6381440 --- liblog/event_tag_map.cpp | 608 +++++++++++++++------------------------ 1 file changed, 234 insertions(+), 374 deletions(-) diff --git a/liblog/event_tag_map.cpp b/liblog/event_tag_map.cpp index e8e0335e8..115542233 100644 --- a/liblog/event_tag_map.cpp +++ b/liblog/event_tag_map.cpp @@ -1,5 +1,5 @@ /* - * Copyright (C) 2007 The Android Open Source Project + * Copyright (C) 2007-2016 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. @@ -24,362 +24,106 @@ #include #include -#include +#include +#include +#include + #include #include "log_portability.h" #define OUT_TAG "EventTagMap" -/* - * Single entry. - */ -typedef struct EventTag { - uint32_t tagIndex; - char* tagStr; - size_t tagLen; - char* fmtStr; - size_t fmtLen; -} EventTag; +typedef std::experimental::string_view MapString; -/* - * Map. - */ -struct EventTagMap { - /* memory-mapped source file; we get strings from here */ - void* mapAddr; - size_t mapLen; +typedef std::pair TagFmt; - /* array of event tags, sorted numerically by tag index */ - EventTag* tagArray; - int numTags; +template <> struct _LIBCPP_TYPE_VIS_ONLY std::hash + : public std::unary_function { + _LIBCPP_INLINE_VISIBILITY + size_t operator()(const TagFmt& __t) const _NOEXCEPT { + // Tag is typically unique. Will cost us an extra 100ns for the + // unordered_map lookup if we instead did a hash that combined + // both of tag and fmt members, e.g.: + // + // return std::hash()(__t.first) ^ + // std::hash()(__t.second); + return std::hash()(__t.first); + } }; -/* fwd */ -static int processFile(EventTagMap* map); -static int countMapLines(const EventTagMap* map); -static int parseMapLines(EventTagMap* map); -static int scanTagLine(char** pData, EventTag* tag, int lineNum); -static int sortTags(EventTagMap* map); +// Map +struct EventTagMap { + // memory-mapped source file; we get strings from here + void* mapAddr; + size_t mapLen; -/* - * Open the map file and allocate a structure to manage it. - * - * We create a private mapping because we want to terminate the log tag - * strings with '\0'. - */ -LIBLOG_ABI_PUBLIC EventTagMap* android_openEventTagMap(const char* fileName) -{ - EventTagMap* newTagMap; - off_t end; - int save_errno; - const char* tagfile = fileName ? fileName : EVENT_TAG_MAP_FILE; +private: + std::unordered_map Idx2TagFmt; - int fd = open(tagfile, O_RDONLY | O_CLOEXEC); - if (fd < 0) { - save_errno = errno; - fprintf(stderr, "%s: unable to open map '%s': %s\n", - OUT_TAG, tagfile, strerror(save_errno)); - goto fail_errno; - } +public: + EventTagMap() : mapAddr(NULL), mapLen(0) { } - end = lseek(fd, 0L, SEEK_END); - save_errno = errno; - (void) lseek(fd, 0L, SEEK_SET); - if (end < 0) { - fprintf(stderr, "%s: unable to seek map '%s' %s\n", - OUT_TAG, tagfile, strerror(save_errno)); - goto fail_close; - } - - newTagMap = (EventTagMap*)calloc(1, sizeof(EventTagMap)); - if (newTagMap == NULL) { - save_errno = errno; - goto fail_close; - } - - newTagMap->mapAddr = mmap(NULL, end, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0); - save_errno = errno; - close(fd); - fd = -1; - if ((newTagMap->mapAddr == MAP_FAILED) || (newTagMap->mapAddr == NULL)) { - fprintf(stderr, "%s: mmap(%s) failed: %s\n", - OUT_TAG, tagfile, strerror(save_errno)); - goto fail_free; - } - - newTagMap->mapLen = end; - - if (processFile(newTagMap) != 0) goto fail_unmap; - - return newTagMap; - -fail_unmap: - munmap(newTagMap->mapAddr, newTagMap->mapLen); - save_errno = EINVAL; -fail_free: - free(newTagMap); -fail_close: - close(fd); -fail_errno: - errno = save_errno; -fail: - return NULL; -} - -/* - * Close the map. - */ -LIBLOG_ABI_PUBLIC void android_closeEventTagMap(EventTagMap* map) -{ - if (map == NULL) return; - - munmap(map->mapAddr, map->mapLen); - free(map->tagArray); - free(map); -} - -/* - * Look up an entry in the map. - * - * The entries are sorted by tag number, so we can do a binary search. - */ -LIBLOG_ABI_PUBLIC const char* android_lookupEventTag_len(const EventTagMap* map, - size_t *len, - unsigned int tag) -{ - int lo = 0; - int hi = map->numTags - 1; - - while (lo <= hi) { - int mid = (lo + hi) / 2; - int cmp = map->tagArray[mid].tagIndex - tag; - - if (cmp < 0) { - /* tag is bigger */ - lo = mid + 1; - } else if (cmp > 0) { - /* tag is smaller */ - hi = mid - 1; - } else { - /* found */ - if (len) *len = map->tagArray[mid].tagLen; - /* - * b/31456426 to check if gTest can detect copy-on-write issue - * add the following line to break us: - * map->tagArray[mid].tagStr[map->tagArray[mid].tagLen] = '\0'; - * or explicitly use deprecated android_lookupEventTag(). - */ - return map->tagArray[mid].tagStr; + ~EventTagMap() { + Idx2TagFmt.clear(); + if (mapAddr) { + munmap(mapAddr, mapLen); + mapAddr = 0; } } - errno = ENOENT; - if (len) *len = 0; - return NULL; -} + bool emplaceUnique(uint32_t tag, const TagFmt& tagfmt, bool verbose = false); + const TagFmt* find(uint32_t tag) const; +}; -/* - * Look up an entry in the map. - * - * The entries are sorted by tag number, so we can do a binary search. - */ -LIBLOG_ABI_PUBLIC const char* android_lookupEventFormat_len( - const EventTagMap* map, size_t *len, unsigned int tag) -{ - int lo = 0; - int hi = map->numTags - 1; - - while (lo <= hi) { - int mid = (lo + hi) / 2; - int cmp = map->tagArray[mid].tagIndex - tag; - - if (cmp < 0) { - /* tag is bigger */ - lo = mid + 1; - } else if (cmp > 0) { - /* tag is smaller */ - hi = mid - 1; - } else { - /* found */ - if (len) *len = map->tagArray[mid].fmtLen; - return map->tagArray[mid].fmtStr; +bool EventTagMap::emplaceUnique(uint32_t tag, const TagFmt& tagfmt, bool verbose) { + std::unordered_map::const_iterator it; + it = Idx2TagFmt.find(tag); + if (it != Idx2TagFmt.end()) { + if (verbose) { + fprintf(stderr, + OUT_TAG ": duplicate tag entries %" PRIu32 + ":%.*s:%.*s and %" PRIu32 ":%.*s:%.*s)\n", + it->first, + (int)it->second.first.length(), it->second.first.data(), + (int)it->second.second.length(), it->second.second.data(), + tag, + (int)tagfmt.first.length(), tagfmt.first.data(), + (int)tagfmt.second.length(), tagfmt.second.data()); } + return false; } - errno = ENOENT; - if (len) *len = 0; - return NULL; + Idx2TagFmt.emplace(std::make_pair(tag, tagfmt)); + return true; } -LIBLOG_ABI_PUBLIC const char* android_lookupEventTag(const EventTagMap* map, - unsigned int tag) -{ - size_t len; - const char* tagStr = android_lookupEventTag_len(map, &len, tag); +const TagFmt* EventTagMap::find(uint32_t tag) const { + std::unordered_map::const_iterator it; + it = Idx2TagFmt.find(tag); + if (it == Idx2TagFmt.end()) return NULL; + return &(it->second); +} + +// Scan one tag line. +// +// "*pData" should be pointing to the first digit in the tag number. On +// successful return, it will be pointing to the last character in the +// tag line (i.e. the character before the start of the next line). +// +// Returns 0 on success, nonzero on failure. +static int scanTagLine(EventTagMap* map, char** pData, int lineNum) { char* cp; - - if (!tagStr) return tagStr; - cp = (char*)tagStr; - cp += len; - if (*cp) *cp = '\0'; /* Trigger copy on write :-( */ - return tagStr; -} - -/* - * Crunch through the file, parsing the contents and creating a tag index. - */ -static int processFile(EventTagMap* map) -{ - /* get a tag count */ - map->numTags = countMapLines(map); - if (map->numTags < 0) { - errno = ENOENT; - return -1; - } - - /* allocate storage for the tag index array */ - map->tagArray = (EventTag*)calloc(1, sizeof(EventTag) * map->numTags); - if (map->tagArray == NULL) return -1; - - /* parse the file, null-terminating tag strings */ - if (parseMapLines(map) != 0) return -1; - - /* sort the tags and check for duplicates */ - if (sortTags(map) != 0) return -1; - - return 0; -} - -/* - * Run through all lines in the file, determining whether they're blank, - * comments, or possibly have a tag entry. - * - * This is a very "loose" scan. We don't try to detect syntax errors here. - * The later pass is more careful, but the number of tags found there must - * match the number of tags found here. - * - * Returns the number of potential tag entries found. - */ -static int countMapLines(const EventTagMap* map) -{ - const char* cp = (const char*) map->mapAddr; - const char* endp = cp + map->mapLen; - int numTags = 0; - int unknown = 1; - - while (cp < endp) { - if (*cp == '\n') { - unknown = 1; - } else if (unknown) { - if (isdigit(*cp)) { - /* looks like a tag to me */ - numTags++; - unknown = 0; - } else if (isspace(*cp)) { - /* might be leading whitespace before tag num, keep going */ - } else { - /* assume comment; second pass can complain in detail */ - unknown = 0; - } - } else { - /* we've made up our mind; just scan to end of line */ - } - cp++; - } - - return numTags; -} - -/* - * Parse the tags out of the file. - */ -static int parseMapLines(EventTagMap* map) -{ - int tagNum, lineStart, lineNum; - char* cp = (char*) map->mapAddr; - char* endp = cp + map->mapLen; - - /* insist on EOL at EOF; simplifies parsing and null-termination */ - if (*(endp - 1) != '\n') { - fprintf(stderr, "%s: map file missing EOL on last line\n", OUT_TAG); - errno = EINVAL; - return -1; - } - - tagNum = 0; - lineStart = 1; - lineNum = 1; - while (cp < endp) { - if (*cp == '\n') { - lineStart = 1; - lineNum++; - } else if (lineStart) { - if (*cp == '#') { - /* comment; just scan to end */ - lineStart = 0; - } else if (isdigit(*cp)) { - /* looks like a tag; scan it out */ - if (tagNum >= map->numTags) { - fprintf(stderr, - "%s: more tags than expected (%d)\n", OUT_TAG, tagNum); - errno = EMFILE; - return -1; - } - if (scanTagLine(&cp, &map->tagArray[tagNum], lineNum) != 0) { - return -1; - } - tagNum++; - lineNum++; // we eat the '\n' - /* leave lineStart==1 */ - } else if (isspace(*cp)) { - /* looks like leading whitespace; keep scanning */ - } else { - fprintf(stderr, - "%s: unexpected chars (0x%02x) in tag number on line %d\n", - OUT_TAG, *cp, lineNum); - errno = EINVAL; - return -1; - } - } else { - /* this is a blank or comment line */ - } - cp++; - } - - if (tagNum != map->numTags) { - fprintf(stderr, "%s: parsed %d tags, expected %d\n", - OUT_TAG, tagNum, map->numTags); - errno = EINVAL; - return -1; - } - - return 0; -} - -/* - * Scan one tag line. - * - * "*pData" should be pointing to the first digit in the tag number. On - * successful return, it will be pointing to the last character in the - * tag line (i.e. the character before the start of the next line). - * - * Returns 0 on success, nonzero on failure. - */ -static int scanTagLine(char** pData, EventTag* tag, int lineNum) -{ - char* cp; - unsigned long val = strtoul(*pData, &cp, 10); if (cp == *pData) { - fprintf(stderr, "%s: malformed tag number on line %d\n", OUT_TAG, lineNum); + fprintf(stderr, OUT_TAG ": malformed tag number on line %d\n", lineNum); errno = EINVAL; return -1; } - tag->tagIndex = val; - if (tag->tagIndex != val) { - fprintf(stderr, "%s: tag number too large on line %d\n", OUT_TAG, lineNum); + uint32_t tagIndex = val; + if (tagIndex != val) { + fprintf(stderr, OUT_TAG ": tag number too large on line %d\n", lineNum); errno = ERANGE; return -1; } @@ -388,76 +132,192 @@ static int scanTagLine(char** pData, EventTag* tag, int lineNum) } if (*cp == '\n') { - fprintf(stderr, "%s: missing tag string on line %d\n", OUT_TAG, lineNum); + fprintf(stderr, OUT_TAG ": missing tag string on line %d\n", lineNum); errno = EINVAL; return -1; } - tag->tagStr = cp; - - /* Determine whether "c" is a valid tag char. */ - while (isalnum(*++cp) || (*cp == '_')) { - } - tag->tagLen = cp - tag->tagStr; + const char* tag = cp; + // Determine whether "c" is a valid tag char. + while (isalnum(*++cp) || (*cp == '_')) { } + size_t tagLen = cp - tag; if (!isspace(*cp)) { - fprintf(stderr, "%s: invalid tag chars on line %d\n", OUT_TAG, lineNum); + fprintf(stderr, OUT_TAG ": invalid tag chars on line %d\n", lineNum); errno = EINVAL; return -1; } while (isspace(*cp) && (*cp != '\n')) ++cp; + const char* fmt = NULL; + size_t fmtLen = 0; if (*cp != '#') { - tag->fmtStr = cp; + fmt = cp; while ((*cp != '\n') && (*cp != '#')) ++cp; - while ((cp > tag->fmtStr) && isspace(*(cp - 1))) --cp; - tag->fmtLen = cp - tag->fmtStr; + while ((cp > fmt) && isspace(*(cp - 1))) --cp; + fmtLen = cp - fmt; } while (*cp != '\n') ++cp; +#ifdef DEBUG + fprintf(stderr, "%d: %p: %.*s\n", lineNum, tag, (int)(cp - *pData), *pData); +#endif *pData = cp; - return 0; + if (map->emplaceUnique(tagIndex, TagFmt(std::make_pair( + MapString(tag, tagLen), MapString(fmt, fmtLen))), true)) { + return 0; + } + errno = EMLINK; + return -1; } -/* - * Compare two EventTags. - */ -static int compareEventTags(const void* v1, const void* v2) -{ - const EventTag* tag1 = (const EventTag*) v1; - const EventTag* tag2 = (const EventTag*) v2; +// Parse the tags out of the file. +static int parseMapLines(EventTagMap* map) { + char* cp = static_cast(map->mapAddr); + size_t len = map->mapLen; + char* endp = cp + len; - return tag1->tagIndex - tag2->tagIndex; -} + // insist on EOL at EOF; simplifies parsing and null-termination + if (!len || (*(endp - 1) != '\n')) { +#ifdef DEBUG + fprintf(stderr, OUT_TAG ": map file missing EOL on last line\n"); +#endif + errno = EINVAL; + return -1; + } -/* - * Sort the EventTag array so we can do fast lookups by tag index. After - * the sort we do a quick check for duplicate tag indices. - * - * Returns 0 on success. - */ -static int sortTags(EventTagMap* map) -{ - int i; - - qsort(map->tagArray, map->numTags, sizeof(EventTag), compareEventTags); - - for (i = 1; i < map->numTags; i++) { - if (map->tagArray[i].tagIndex == map->tagArray[i - 1].tagIndex) { - fprintf(stderr, - "%s: duplicate tag entries (%" PRIu32 ":%.*s:%.*s and %" PRIu32 ":%.*s:%.*s)\n", - OUT_TAG, - map->tagArray[i].tagIndex, - (int)map->tagArray[i].tagLen, map->tagArray[i].tagStr, - (int)map->tagArray[i].fmtLen, map->tagArray[i].fmtStr, - map->tagArray[i - 1].tagIndex, - (int)map->tagArray[i - 1].tagLen, map->tagArray[i - 1].fmtStr, - (int)map->tagArray[i - 1].fmtLen, map->tagArray[i - 1].fmtStr); - errno = EMLINK; - return -1; + bool lineStart = true; + int lineNum = 1; + while (cp < endp) { + if (*cp == '\n') { + lineStart = true; + lineNum++; + } else if (lineStart) { + if (*cp == '#') { + // comment; just scan to end + lineStart = false; + } else if (isdigit(*cp)) { + // looks like a tag; scan it out + if (scanTagLine(map, &cp, lineNum) != 0) { + return -1; + } + lineNum++; // we eat the '\n' + // leave lineStart==true + } else if (isspace(*cp)) { + // looks like leading whitespace; keep scanning + } else { + fprintf(stderr, + OUT_TAG ": unexpected chars (0x%02x) in tag number on line %d\n", + *cp, lineNum); + errno = EINVAL; + return -1; + } + } else { + // this is a blank or comment line } + cp++; } return 0; } + +// Open the map file and allocate a structure to manage it. +// +// We create a private mapping because we want to terminate the log tag +// strings with '\0'. +LIBLOG_ABI_PUBLIC EventTagMap* android_openEventTagMap(const char* fileName) { + int save_errno; + + const char* tagfile = fileName ? fileName : EVENT_TAG_MAP_FILE; + int fd = open(tagfile, O_RDONLY | O_CLOEXEC); + if (fd < 0) { + save_errno = errno; + fprintf(stderr, OUT_TAG ": unable to open map '%s': %s\n", + tagfile, strerror(save_errno)); + errno = save_errno; + return NULL; + } + off_t end = lseek(fd, 0L, SEEK_END); + save_errno = errno; + (void)lseek(fd, 0L, SEEK_SET); + if (end < 0) { + fprintf(stderr, OUT_TAG ": unable to seek map '%s' %s\n", + tagfile, strerror(save_errno)); + close(fd); + errno = save_errno; + return NULL; + } + + EventTagMap* newTagMap = new EventTagMap; + if (newTagMap == NULL) { + save_errno = errno; + close(fd); + errno = save_errno; + return NULL; + } + + newTagMap->mapAddr = mmap(NULL, end, PROT_READ | PROT_WRITE, + MAP_PRIVATE, fd, 0); + save_errno = errno; + close(fd); + fd = -1; + if ((newTagMap->mapAddr == MAP_FAILED) || (newTagMap->mapAddr == NULL)) { + fprintf(stderr, OUT_TAG ": mmap(%s) failed: %s\n", + tagfile, strerror(save_errno)); + delete newTagMap; + errno = save_errno; + return NULL; + } + + newTagMap->mapLen = end; + + if (parseMapLines(newTagMap) != 0) { + delete newTagMap; + return NULL; + } + + return newTagMap; +} + +// Close the map. +LIBLOG_ABI_PUBLIC void android_closeEventTagMap(EventTagMap* map) { + if (map) delete map; +} + +// Look up an entry in the map. +LIBLOG_ABI_PUBLIC const char* android_lookupEventTag_len(const EventTagMap* map, + size_t *len, + unsigned int tag) { + if (len) *len = 0; + const TagFmt* str = map->find(tag); + if (!str) return NULL; + if (len) *len = str->first.length(); + return str->first.data(); +} + +// Look up an entry in the map. +LIBLOG_ABI_PUBLIC const char* android_lookupEventFormat_len( + const EventTagMap* map, size_t *len, unsigned int tag) { + if (len) *len = 0; + const TagFmt* str = map->find(tag); + if (!str) return NULL; + if (len) *len = str->second.length(); + return str->second.data(); +} + +// This function is deprecated and replaced with android_lookupEventTag_len +// since it will cause the map to change from Shared and backed by a file, +// to Private Dirty and backed up by swap, albeit highly compressible. By +// deprecating this function everywhere, we save 100s of MB of memory space. +LIBLOG_ABI_PUBLIC const char* android_lookupEventTag(const EventTagMap* map, + unsigned int tag) { + size_t len; + const char* tagStr = android_lookupEventTag_len(map, &len, tag); + + if (!tagStr) return tagStr; + char* cp = const_cast(tagStr); + cp += len; + if (*cp) *cp = '\0'; // Trigger copy on write :-( and why deprecated. + return tagStr; +}