Commit Graph

5 Commits

Author SHA1 Message Date
Tom Cherry f74503dd46 logd: optionally track the full size of log buffers
ChattyLogBuffer ignores the metadata (timestamp, pid, std::list<>
iterators, etc) of log entries when calculating the size used by a
given log buffer. For example, if 1MB is the specified size of the
'main' log buffer, logd will use between ~1.3MB and ~2MB of overall
memory for 'main' log buffer.  LogStatistics does track the overall
memory used and labels it 'Overhead', however this 'Overhead' is only
informative and is not used for Pruning or Chatty calculations.

This is problematic, since it makes logd's memory usage inconsistent:
depending on the pattern of logging, there can be substantially more
memory used than the specified log buffer size.  This is further
complicated by the fact that chatty messages are entirely metadata and
therefore not counted as contributing to the log buffer size.

This change would switch logd to always track the full size of log
buffers, but there are two problems with this approach:
1) Unless users double their buffer sizes, then they'd have
   substantially fewer logs after the change
2) Chatty logic would change and it's difficult to evaluate.

Therefore this change only provides the framework to track the full
size of log buffers.  This allows an apples to apples comparison of
ChattyLogBuffer and SerializedLogBuffer.  With this option enabled,
logd reports the following values:

ChattyLogBuffer:
Total log size (logcat -g), 'Total' / 'Now' (logcat -S), and
'Overhead' (logcat -S) all report the full size of log entries
including metadata.

SerializedLogBuffer:
Total log size (logcat -g) and 'Overhead' (logcat -S) report the
compressed size of the log entries including metadata.
'Total' / 'Now' (logcat -S) reports the uncompressed size of the log
entries that are available including metadata.

Test: logging statistics are correct
Change-Id: If17682af8bb605f31387d7b210b69a301dd48f07
2020-07-01 14:35:33 -07:00
Tom Cherry b6cb992cf3 logd: replace std::vector<uint8_t> in SerializedLogChunk
Turns out std::vector::resize() and std::vector::clear() don't
actually deallocate any memory.  std::vector::shrink_to_fit() can be
used for this but isn't a 'guarantee'.  Instead of trying to get
std::vector to play nice, this change replaces std::vector<uint8_t>
with std::unique_ptr<uint8_t[]>, which is more accurate to how I'm
using this memory anyway.

Test: logging unit tests
Change-Id: I9638e90bbf50bcf316c5aa172c8278ea945d27e7
2020-06-24 16:19:28 -07:00
Tom Cherry b07e339b53 logd: fix use after resize of contents_ vector
SerializedFlushToState::PopNextUnreadLog() was calling
AddMinHeapEntry() to replenish the element that was just popped off of
the heap, however AddMinHeapEntry() also manages reference counts for
the buffers, and this resulting in the following scenario:

PopNextUnreadLog() returns a pointer referencing log buffer #1
AddMinHeapEntry() sees that all logs from buffer #1 has been read, so
it decrements the reference count
The caller of PopNextUnreadLog() uses the result which references
invalid memory.

This calls CheckForNewLogs() within HasUnreadLogs() instead of
requiring a separate call, which fixes an additional issue where
continuing from the loop in SerializedLogBuffer::FlushTo() may not
pick up subsequent logs in a given log buffer, since CheckForNewLogs()
wouldn't be called.  This was exacerbated by the above change.

This adds a test to check the reference counts for this case and fixes
an argument mismatch in SerializedFlushToStateTest.

This adds the corpus that surfaced the issue.

Bug: 159753229
Bug: 159783005
Test: these unit tests, run fuzzer without error
Change-Id: Ib2636dfc14293b7e2cd00876b9def6e9dbbff4ce
2020-06-24 15:31:46 -07:00
Tom Cherry 9b4246dc2d logd: fix various clang-tidy issues
In order of severity:
1) Add a CHECK() that a pointer is not nullptr, where the analyzer
   believes this is possible.
2) Add `final` appropriately to functions called from constructors.
3) Add missing cloexec flags.
4) Add missing `noexcept` and other subtle performance warnings

Test: build with clang-tidy
Change-Id: Ifd9a1299a51027a47382926b2224748b5750d6cf
2020-06-17 11:40:55 -07:00
Tom Cherry 1a796bca57 logd: add a SerializedLogBuffer suitable for compression
Initial commit for a SerializedLogBuffer.  The intention here is for
the serialized data to be compressed (currently using zlib) to allow
for substantially longer logs in the same memory footprint.

Test: unit tests
Change-Id: I2528e4e1ff1cf3bc91130173a107f371f04d911a
2020-06-12 14:35:30 -07:00