From 3e61a1368af0cbcd760c20598870472c5117297f Mon Sep 17 00:00:00 2001 From: Tom Cherry Date: Wed, 27 May 2020 10:46:37 -0700 Subject: [PATCH 1/2] logd: rename FlushToResult to FilterResult This was a typo; the enum corresponds to the result of the 'Filter' function, not the 'FlushTo' function. Test: build Change-Id: Ib46f0646570b6dbaac17ae9fc95c990128cdbe72 --- logd/LogBuffer.h | 4 ++-- logd/LogReader.cpp | 12 ++++++------ logd/LogReaderThread.cpp | 28 ++++++++++++++-------------- logd/LogReaderThread.h | 4 ++-- logd/SimpleLogBuffer.cpp | 8 ++++---- logd/SimpleLogBuffer.h | 2 +- 6 files changed, 29 insertions(+), 29 deletions(-) diff --git a/logd/LogBuffer.h b/logd/LogBuffer.h index 7f1e1284e..d38c01840 100644 --- a/logd/LogBuffer.h +++ b/logd/LogBuffer.h @@ -27,7 +27,7 @@ class LogWriter; -enum class FlushToResult { +enum class FilterResult { kSkip, kStop, kWrite, @@ -48,7 +48,7 @@ class LogBuffer { virtual uint64_t FlushTo( LogWriter* writer, uint64_t start, pid_t* last_tid, // nullable - const std::function& filter) = 0; + const std::function& filter) = 0; virtual bool Clear(log_id_t id, uid_t uid) = 0; virtual unsigned long GetSize(log_id_t id) = 0; diff --git a/logd/LogReader.cpp b/logd/LogReader.cpp index 234ddc75b..26f2ed28a 100644 --- a/logd/LogReader.cpp +++ b/logd/LogReader.cpp @@ -172,26 +172,26 @@ bool LogReader::onDataAvailable(SocketClient* cli) { bool start_time_set = false; uint64_t last = sequence; auto log_find_start = [pid, logMask, start, &sequence, &start_time_set, - &last](const LogBufferElement* element) -> FlushToResult { + &last](const LogBufferElement* element) -> FilterResult { if (pid && pid != element->getPid()) { - return FlushToResult::kSkip; + return FilterResult::kSkip; } if ((logMask & (1 << element->getLogId())) == 0) { - return FlushToResult::kSkip; + return FilterResult::kSkip; } if (start == element->getRealTime()) { sequence = element->getSequence(); start_time_set = true; - return FlushToResult::kStop; + return FilterResult::kStop; } else { if (start < element->getRealTime()) { sequence = last; start_time_set = true; - return FlushToResult::kStop; + return FilterResult::kStop; } last = element->getSequence(); } - return FlushToResult::kSkip; + return FilterResult::kSkip; }; log_buffer_->FlushTo(socket_log_writer.get(), sequence, nullptr, log_find_start); diff --git a/logd/LogReaderThread.cpp b/logd/LogReaderThread.cpp index b2001b56b..8de9622ad 100644 --- a/logd/LogReaderThread.cpp +++ b/logd/LogReaderThread.cpp @@ -123,12 +123,12 @@ void LogReaderThread::ThreadFunction() { } // A first pass to count the number of elements -FlushToResult LogReaderThread::FilterFirstPass(const LogBufferElement* element) { +FilterResult LogReaderThread::FilterFirstPass(const LogBufferElement* element) { auto lock = std::lock_guard{reader_list_->reader_threads_lock()}; if (leading_dropped_) { if (element->getDropped()) { - return FlushToResult::kSkip; + return FilterResult::kSkip; } leading_dropped_ = false; } @@ -142,46 +142,46 @@ FlushToResult LogReaderThread::FilterFirstPass(const LogBufferElement* element) ++count_; } - return FlushToResult::kSkip; + return FilterResult::kSkip; } // A second pass to send the selected elements -FlushToResult LogReaderThread::FilterSecondPass(const LogBufferElement* element) { +FilterResult LogReaderThread::FilterSecondPass(const LogBufferElement* element) { auto lock = std::lock_guard{reader_list_->reader_threads_lock()}; start_ = element->getSequence(); if (skip_ahead_[element->getLogId()]) { skip_ahead_[element->getLogId()]--; - return FlushToResult::kSkip; + return FilterResult::kSkip; } if (leading_dropped_) { if (element->getDropped()) { - return FlushToResult::kSkip; + return FilterResult::kSkip; } leading_dropped_ = false; } // Truncate to close race between first and second pass if (non_block_ && tail_ && index_ >= count_) { - return FlushToResult::kStop; + return FilterResult::kStop; } if (!IsWatching(element->getLogId())) { - return FlushToResult::kSkip; + return FilterResult::kSkip; } if (pid_ && pid_ != element->getPid()) { - return FlushToResult::kSkip; + return FilterResult::kSkip; } if (start_time_ != log_time::EPOCH && element->getRealTime() <= start_time_) { - return FlushToResult::kSkip; + return FilterResult::kSkip; } if (release_) { - return FlushToResult::kStop; + return FilterResult::kStop; } if (!tail_) { @@ -191,7 +191,7 @@ FlushToResult LogReaderThread::FilterSecondPass(const LogBufferElement* element) ++index_; if (count_ > tail_ && index_ <= (count_ - tail_)) { - return FlushToResult::kSkip; + return FilterResult::kSkip; } if (!non_block_) { @@ -200,9 +200,9 @@ FlushToResult LogReaderThread::FilterSecondPass(const LogBufferElement* element) ok: if (!skip_ahead_[element->getLogId()]) { - return FlushToResult::kWrite; + return FilterResult::kWrite; } - return FlushToResult::kSkip; + return FilterResult::kSkip; } void LogReaderThread::cleanSkip_Locked(void) { diff --git a/logd/LogReaderThread.h b/logd/LogReaderThread.h index e48a3cab2..907bc83a7 100644 --- a/logd/LogReaderThread.h +++ b/logd/LogReaderThread.h @@ -63,8 +63,8 @@ class LogReaderThread { private: void ThreadFunction(); // flushTo filter callbacks - FlushToResult FilterFirstPass(const LogBufferElement* element); - FlushToResult FilterSecondPass(const LogBufferElement* element); + FilterResult FilterFirstPass(const LogBufferElement* element); + FilterResult FilterSecondPass(const LogBufferElement* element); std::condition_variable thread_triggered_condition_; LogBuffer* log_buffer_; diff --git a/logd/SimpleLogBuffer.cpp b/logd/SimpleLogBuffer.cpp index 1c834281d..d60805d8c 100644 --- a/logd/SimpleLogBuffer.cpp +++ b/logd/SimpleLogBuffer.cpp @@ -112,7 +112,7 @@ void SimpleLogBuffer::LogInternal(LogBufferElement&& elem) { uint64_t SimpleLogBuffer::FlushTo( LogWriter* writer, uint64_t start, pid_t* last_tid, - const std::function& filter) { + const std::function& filter) { auto shared_lock = SharedLock{lock_}; std::list::iterator it; @@ -146,11 +146,11 @@ uint64_t SimpleLogBuffer::FlushTo( } if (filter) { - FlushToResult ret = filter(&element); - if (ret == FlushToResult::kSkip) { + FilterResult ret = filter(&element); + if (ret == FilterResult::kSkip) { continue; } - if (ret == FlushToResult::kStop) { + if (ret == FilterResult::kStop) { break; } } diff --git a/logd/SimpleLogBuffer.h b/logd/SimpleLogBuffer.h index 9a2d01ab5..cd08acf69 100644 --- a/logd/SimpleLogBuffer.h +++ b/logd/SimpleLogBuffer.h @@ -37,7 +37,7 @@ class SimpleLogBuffer : public LogBuffer { uint16_t len) override; uint64_t FlushTo( LogWriter* writer, uint64_t start, pid_t* lastTid, - const std::function& filter) override; + const std::function& filter) override; bool Clear(log_id_t id, uid_t uid) override; unsigned long GetSize(log_id_t id) override; From 70fadea36f9d742e2e52e223abb6d75bcc5307f5 Mon Sep 17 00:00:00 2001 From: Tom Cherry Date: Wed, 27 May 2020 14:43:19 -0700 Subject: [PATCH 2/2] logd: remove LogBufferElement dependency of LogReaderThread In the future, not all log buffers will be implemented in terms of LogBufferElement. Test: build Change-Id: I5cf0d01414857b1bfa08c92a4f8035b43ef2aad7 --- logd/LogBuffer.h | 15 +++++++------- logd/LogReader.cpp | 18 +++++++++-------- logd/LogReaderThread.cpp | 42 +++++++++++++++++++++++++--------------- logd/LogReaderThread.h | 7 ++++--- logd/SimpleLogBuffer.cpp | 6 ++++-- logd/SimpleLogBuffer.h | 7 ++++--- 6 files changed, 55 insertions(+), 40 deletions(-) diff --git a/logd/LogBuffer.h b/logd/LogBuffer.h index d38c01840..859d7400c 100644 --- a/logd/LogBuffer.h +++ b/logd/LogBuffer.h @@ -21,11 +21,9 @@ #include #include -#include +#include -#include "LogBufferElement.h" - -class LogWriter; +#include "LogWriter.h" enum class FilterResult { kSkip, @@ -45,10 +43,11 @@ class LogBuffer { // valid message was from the same source so we can differentiate chatty // filter types (identical or expired) static const uint64_t FLUSH_ERROR = 0; - virtual uint64_t FlushTo( - LogWriter* writer, uint64_t start, - pid_t* last_tid, // nullable - const std::function& filter) = 0; + virtual uint64_t FlushTo(LogWriter* writer, uint64_t start, + pid_t* last_tid, // nullable + const std::function& filter) = 0; virtual bool Clear(log_id_t id, uid_t uid) = 0; virtual unsigned long GetSize(log_id_t id) = 0; diff --git a/logd/LogReader.cpp b/logd/LogReader.cpp index 26f2ed28a..35c46aac7 100644 --- a/logd/LogReader.cpp +++ b/logd/LogReader.cpp @@ -171,25 +171,27 @@ bool LogReader::onDataAvailable(SocketClient* cli) { if (start != log_time::EPOCH) { bool start_time_set = false; uint64_t last = sequence; - auto log_find_start = [pid, logMask, start, &sequence, &start_time_set, - &last](const LogBufferElement* element) -> FilterResult { - if (pid && pid != element->getPid()) { + auto log_find_start = [pid, logMask, start, &sequence, &start_time_set, &last]( + log_id_t element_log_id, pid_t element_pid, + uint64_t element_sequence, log_time element_realtime, + uint16_t) -> FilterResult { + if (pid && pid != element_pid) { return FilterResult::kSkip; } - if ((logMask & (1 << element->getLogId())) == 0) { + if ((logMask & (1 << element_log_id)) == 0) { return FilterResult::kSkip; } - if (start == element->getRealTime()) { - sequence = element->getSequence(); + if (start == element_realtime) { + sequence = element_sequence; start_time_set = true; return FilterResult::kStop; } else { - if (start < element->getRealTime()) { + if (start < element_realtime) { sequence = last; start_time_set = true; return FilterResult::kStop; } - last = element->getSequence(); + last = element_sequence; } return FilterResult::kSkip; }; diff --git a/logd/LogReaderThread.cpp b/logd/LogReaderThread.cpp index 8de9622ad..3a83f3f5f 100644 --- a/logd/LogReaderThread.cpp +++ b/logd/LogReaderThread.cpp @@ -75,13 +75,21 @@ void LogReaderThread::ThreadFunction() { if (tail_) { log_buffer_->FlushTo(writer_.get(), start, nullptr, - std::bind(&LogReaderThread::FilterFirstPass, this, _1)); + [this](log_id_t log_id, pid_t pid, uint64_t sequence, + log_time realtime, uint16_t dropped_count) { + return FilterFirstPass(log_id, pid, sequence, realtime, + dropped_count); + }); leading_dropped_ = true; // TODO: Likely a bug, if leading_dropped_ was not true before calling // flushTo(), then it should not be reset to true after. } start = log_buffer_->FlushTo(writer_.get(), start, last_tid_, - std::bind(&LogReaderThread::FilterSecondPass, this, _1)); + [this](log_id_t log_id, pid_t pid, uint64_t sequence, + log_time realtime, uint16_t dropped_count) { + return FilterSecondPass(log_id, pid, sequence, realtime, + dropped_count); + }); // We only ignore entries before the original start time for the first flushTo(), if we // get entries after this first flush before the original start time, then the client @@ -123,22 +131,23 @@ void LogReaderThread::ThreadFunction() { } // A first pass to count the number of elements -FilterResult LogReaderThread::FilterFirstPass(const LogBufferElement* element) { +FilterResult LogReaderThread::FilterFirstPass(log_id_t log_id, pid_t pid, uint64_t sequence, + log_time realtime, uint16_t dropped_count) { auto lock = std::lock_guard{reader_list_->reader_threads_lock()}; if (leading_dropped_) { - if (element->getDropped()) { + if (dropped_count) { return FilterResult::kSkip; } leading_dropped_ = false; } if (count_ == 0) { - start_ = element->getSequence(); + start_ = sequence; } - if ((!pid_ || pid_ == element->getPid()) && IsWatching(element->getLogId()) && - (start_time_ == log_time::EPOCH || start_time_ <= element->getRealTime())) { + if ((!pid_ || pid_ == pid) && IsWatching(log_id) && + (start_time_ == log_time::EPOCH || start_time_ <= realtime)) { ++count_; } @@ -146,18 +155,19 @@ FilterResult LogReaderThread::FilterFirstPass(const LogBufferElement* element) { } // A second pass to send the selected elements -FilterResult LogReaderThread::FilterSecondPass(const LogBufferElement* element) { +FilterResult LogReaderThread::FilterSecondPass(log_id_t log_id, pid_t pid, uint64_t sequence, + log_time realtime, uint16_t dropped_count) { auto lock = std::lock_guard{reader_list_->reader_threads_lock()}; - start_ = element->getSequence(); + start_ = sequence; - if (skip_ahead_[element->getLogId()]) { - skip_ahead_[element->getLogId()]--; + if (skip_ahead_[log_id]) { + skip_ahead_[log_id]--; return FilterResult::kSkip; } if (leading_dropped_) { - if (element->getDropped()) { + if (dropped_count) { return FilterResult::kSkip; } leading_dropped_ = false; @@ -168,15 +178,15 @@ FilterResult LogReaderThread::FilterSecondPass(const LogBufferElement* element) return FilterResult::kStop; } - if (!IsWatching(element->getLogId())) { + if (!IsWatching(log_id)) { return FilterResult::kSkip; } - if (pid_ && pid_ != element->getPid()) { + if (pid_ && pid_ != pid) { return FilterResult::kSkip; } - if (start_time_ != log_time::EPOCH && element->getRealTime() <= start_time_) { + if (start_time_ != log_time::EPOCH && realtime <= start_time_) { return FilterResult::kSkip; } @@ -199,7 +209,7 @@ FilterResult LogReaderThread::FilterSecondPass(const LogBufferElement* element) } ok: - if (!skip_ahead_[element->getLogId()]) { + if (!skip_ahead_[log_id]) { return FilterResult::kWrite; } return FilterResult::kSkip; diff --git a/logd/LogReaderThread.h b/logd/LogReaderThread.h index 907bc83a7..ba810634b 100644 --- a/logd/LogReaderThread.h +++ b/logd/LogReaderThread.h @@ -30,7 +30,6 @@ #include #include "LogBuffer.h" -#include "LogBufferElement.h" #include "LogWriter.h" class LogReaderList; @@ -63,8 +62,10 @@ class LogReaderThread { private: void ThreadFunction(); // flushTo filter callbacks - FilterResult FilterFirstPass(const LogBufferElement* element); - FilterResult FilterSecondPass(const LogBufferElement* element); + FilterResult FilterFirstPass(log_id_t log_id, pid_t pid, uint64_t sequence, log_time realtime, + uint16_t dropped_count); + FilterResult FilterSecondPass(log_id_t log_id, pid_t pid, uint64_t sequence, log_time realtime, + uint16_t dropped_count); std::condition_variable thread_triggered_condition_; LogBuffer* log_buffer_; diff --git a/logd/SimpleLogBuffer.cpp b/logd/SimpleLogBuffer.cpp index d60805d8c..8a11b929b 100644 --- a/logd/SimpleLogBuffer.cpp +++ b/logd/SimpleLogBuffer.cpp @@ -112,7 +112,8 @@ void SimpleLogBuffer::LogInternal(LogBufferElement&& elem) { uint64_t SimpleLogBuffer::FlushTo( LogWriter* writer, uint64_t start, pid_t* last_tid, - const std::function& filter) { + const std::function& filter) { auto shared_lock = SharedLock{lock_}; std::list::iterator it; @@ -146,7 +147,8 @@ uint64_t SimpleLogBuffer::FlushTo( } if (filter) { - FilterResult ret = filter(&element); + FilterResult ret = filter(element.getLogId(), element.getPid(), element.getSequence(), + element.getRealTime(), element.getDropped()); if (ret == FilterResult::kSkip) { continue; } diff --git a/logd/SimpleLogBuffer.h b/logd/SimpleLogBuffer.h index cd08acf69..72d26b060 100644 --- a/logd/SimpleLogBuffer.h +++ b/logd/SimpleLogBuffer.h @@ -35,9 +35,10 @@ class SimpleLogBuffer : public LogBuffer { int Log(log_id_t log_id, log_time realtime, uid_t uid, pid_t pid, pid_t tid, const char* msg, uint16_t len) override; - uint64_t FlushTo( - LogWriter* writer, uint64_t start, pid_t* lastTid, - const std::function& filter) override; + uint64_t FlushTo(LogWriter* writer, uint64_t start, pid_t* lastTid, + const std::function& + filter) override; bool Clear(log_id_t id, uid_t uid) override; unsigned long GetSize(log_id_t id) override;