SF: Use RAII for TransactionTracing
Ensure the thread joins on destruction, which had been suppressed by a
leak fixed by ag/16613874.
Bug: 214486764
Test: libsurfaceflinger_arc_test
Test: libsurfaceflinger_unittest
Change-Id: Ib766735dc51e89d63080dd6b9afe8464da7c1c6a
diff --git a/services/surfaceflinger/Tracing/TransactionTracing.cpp b/services/surfaceflinger/Tracing/TransactionTracing.cpp
index b5966d5..7d75718 100644
--- a/services/surfaceflinger/Tracing/TransactionTracing.cpp
+++ b/services/surfaceflinger/Tracing/TransactionTracing.cpp
@@ -23,35 +23,22 @@
#include <utils/SystemClock.h>
#include <utils/Trace.h>
-#include "RingBuffer.h"
#include "TransactionTracing.h"
namespace android {
TransactionTracing::TransactionTracing() {
- mBuffer = std::make_unique<
- RingBuffer<proto::TransactionTraceFile, proto::TransactionTraceEntry>>();
-}
-
-TransactionTracing::~TransactionTracing() = default;
-
-bool TransactionTracing::enable() {
std::scoped_lock lock(mTraceLock);
- if (mEnabled) {
- return false;
- }
- mBuffer->setSize(mBufferSizeInBytes);
+
+ mBuffer.setSize(mBufferSizeInBytes);
mStartingTimestamp = systemTime();
- mEnabled = true;
{
std::scoped_lock lock(mMainThreadLock);
- mDone = false;
mThread = std::thread(&TransactionTracing::loop, this);
}
- return true;
}
-bool TransactionTracing::disable() {
+TransactionTracing::~TransactionTracing() {
std::thread thread;
{
std::scoped_lock lock(mMainThreadLock);
@@ -63,43 +50,20 @@
thread.join();
}
- std::scoped_lock lock(mTraceLock);
- if (!mEnabled) {
- return false;
- }
- mEnabled = false;
-
- writeToFileLocked();
- mBuffer->reset();
- mQueuedTransactions.clear();
- mStartingStates.clear();
- mLayerHandles.clear();
- return true;
-}
-
-bool TransactionTracing::isEnabled() const {
- std::scoped_lock lock(mTraceLock);
- return mEnabled;
+ writeToFile();
}
status_t TransactionTracing::writeToFile() {
std::scoped_lock lock(mTraceLock);
- if (!mEnabled) {
- return STATUS_OK;
- }
- return writeToFileLocked();
-}
-
-status_t TransactionTracing::writeToFileLocked() {
proto::TransactionTraceFile fileProto = createTraceFileProto();
addStartingStateToProtoLocked(fileProto);
- return mBuffer->writeToFile(fileProto, FILE_NAME);
+ return mBuffer.writeToFile(fileProto, FILE_NAME);
}
void TransactionTracing::setBufferSize(size_t bufferSizeInBytes) {
std::scoped_lock lock(mTraceLock);
mBufferSizeInBytes = bufferSizeInBytes;
- mBuffer->setSize(mBufferSizeInBytes);
+ mBuffer.setSize(mBufferSizeInBytes);
}
proto::TransactionTraceFile TransactionTracing::createTraceFileProto() const {
@@ -111,21 +75,16 @@
void TransactionTracing::dump(std::string& result) const {
std::scoped_lock lock(mTraceLock);
- base::StringAppendF(&result, "Transaction tracing state: %s\n",
- mEnabled ? "enabled" : "disabled");
base::StringAppendF(&result,
" queued transactions=%zu created layers=%zu handles=%zu states=%zu\n",
mQueuedTransactions.size(), mCreatedLayers.size(), mLayerHandles.size(),
mStartingStates.size());
- mBuffer->dump(result);
+ mBuffer.dump(result);
}
void TransactionTracing::addQueuedTransaction(const TransactionState& transaction) {
std::scoped_lock lock(mTraceLock);
ATRACE_CALL();
- if (!mEnabled) {
- return;
- }
mQueuedTransactions[transaction.id] =
TransactionProtoParser::toProto(transaction,
std::bind(&TransactionTracing::getLayerIdLocked, this,
@@ -206,7 +165,7 @@
std::string serializedProto;
entryProto.SerializeToString(&serializedProto);
entryProto.Clear();
- std::vector<std::string> entries = mBuffer->emplace(std::move(serializedProto));
+ std::vector<std::string> entries = mBuffer.emplace(std::move(serializedProto));
removedEntries.reserve(removedEntries.size() + entries.size());
removedEntries.insert(removedEntries.end(), std::make_move_iterator(entries.begin()),
std::make_move_iterator(entries.end()));
@@ -229,10 +188,10 @@
base::ScopedLockAssertion assumeLocked(mTraceLock);
mTransactionsAddedToBufferCv.wait(lock, [&]() REQUIRES(mTraceLock) {
proto::TransactionTraceEntry entry;
- if (mBuffer->used() > 0) {
- entry.ParseFromString(mBuffer->back());
+ if (mBuffer.used() > 0) {
+ entry.ParseFromString(mBuffer.back());
}
- return mBuffer->used() > 0 && entry.vsync_id() >= vsyncId;
+ return mBuffer.used() > 0 && entry.vsync_id() >= vsyncId;
});
}
@@ -352,7 +311,7 @@
std::scoped_lock<std::mutex> lock(mTraceLock);
proto::TransactionTraceFile proto = createTraceFileProto();
addStartingStateToProtoLocked(proto);
- mBuffer->writeToProto(proto);
+ mBuffer.writeToProto(proto);
return proto;
}
diff --git a/services/surfaceflinger/Tracing/TransactionTracing.h b/services/surfaceflinger/Tracing/TransactionTracing.h
index 26a3758..bc57e01 100644
--- a/services/surfaceflinger/Tracing/TransactionTracing.h
+++ b/services/surfaceflinger/Tracing/TransactionTracing.h
@@ -25,17 +25,16 @@
#include <mutex>
#include <thread>
+#include "RingBuffer.h"
#include "TransactionProtoParser.h"
using namespace android::surfaceflinger;
namespace android {
-template <typename FileProto, typename EntryProto>
-class RingBuffer;
-
class SurfaceFlinger;
class TransactionTracingTest;
+
/*
* Records all committed transactions into a ring bufffer.
*
@@ -54,10 +53,6 @@
TransactionTracing();
~TransactionTracing();
- bool enable();
- bool disable();
- bool isEnabled() const;
-
void addQueuedTransaction(const TransactionState&);
void addCommittedTransactions(std::vector<TransactionState>& transactions, int64_t vsyncId);
status_t writeToFile();
@@ -78,8 +73,7 @@
static constexpr auto FILE_NAME = "/data/misc/wmtrace/transactions_trace.winscope";
mutable std::mutex mTraceLock;
- bool mEnabled GUARDED_BY(mTraceLock) = false;
- std::unique_ptr<RingBuffer<proto::TransactionTraceFile, proto::TransactionTraceEntry>> mBuffer
+ RingBuffer<proto::TransactionTraceFile, proto::TransactionTraceEntry> mBuffer
GUARDED_BY(mTraceLock);
size_t mBufferSizeInBytes GUARDED_BY(mTraceLock) = CONTINUOUS_TRACING_BUFFER_SIZE;
std::unordered_map<uint64_t, proto::TransactionState> mQueuedTransactions
@@ -116,7 +110,6 @@
void tryPushToTracingThread() EXCLUDES(mMainThreadLock);
void addStartingStateToProtoLocked(proto::TransactionTraceFile& proto) REQUIRES(mTraceLock);
void updateStartingStateLocked(const proto::TransactionTraceEntry& entry) REQUIRES(mTraceLock);
- status_t writeToFileLocked() REQUIRES(mTraceLock);
// TEST
// Wait until all the committed transactions for the specified vsync id are added to the buffer.