Merge "SF: Use RAII for TransactionTracing" (93970282) · Commits · e / os / android_frameworks_native

services/surfaceflinger/SurfaceFlinger.cpp

+39 −31

Original line number	Diff line number	Diff line
		@@ -505,10 +505,8 @@ SurfaceFlinger::SurfaceFlinger(Factory& factory) : SurfaceFlinger(factory, SkipI

		enableLatchUnsignaledConfig = getLatchUnsignaledConfig();

		mTransactionTracingEnabled =
		!mIsUserBuild && property_get_bool("debug.sf.enable_transaction_tracing", true);
		if (mTransactionTracingEnabled) {
		mTransactionTracing.enable();
		if (!mIsUserBuild && base::GetBoolProperty("debug.sf.enable_transaction_tracing"s, true)) {
		mTransactionTracing.emplace();
		}
		}

		@@ -3770,8 +3768,8 @@ bool SurfaceFlinger::applyTransactions(std::vector<TransactionState>& transactio
		}
		}

		if (mTransactionTracingEnabled) {
		mTransactionTracing.addCommittedTransactions(transactions, vsyncId);
		if (mTransactionTracing) {
		mTransactionTracing->addCommittedTransactions(transactions, vsyncId);
		}
		return needsTraversal;
		}
		@@ -4031,8 +4029,8 @@ status_t SurfaceFlinger::setTransactionState(
		mBufferCountTracker.increment(state.surface->localBinder());
		});

		if (mTransactionTracingEnabled) {
		mTransactionTracing.addQueuedTransaction(state);
		if (mTransactionTracing) {
		mTransactionTracing->addQueuedTransaction(state);
		}
		queueTransaction(state);

		@@ -4568,8 +4566,8 @@ status_t SurfaceFlinger::mirrorLayer(const LayerCreationArgs& args,
		}

		*outLayerId = mirrorLayer->sequence;
		if (mTransactionTracingEnabled) {
		mTransactionTracing.onMirrorLayerAdded((*outHandle)->localBinder(), mirrorLayer->sequence,
		if (mTransactionTracing) {
		mTransactionTracing->onMirrorLayerAdded((*outHandle)->localBinder(), mirrorLayer->sequence,
		args.name, mirrorFrom->sequence);
		}
		return addClientLayer(args.client, outHandle, mirrorLayer / layer /, nullptr / parent */,
		@@ -4630,8 +4628,8 @@ status_t SurfaceFlinger::createLayer(LayerCreationArgs& args, sp<IBinder>* outHa
		if (parentSp != nullptr) {
		parentId = parentSp->getSequence();
		}
		if (mTransactionTracingEnabled) {
		mTransactionTracing.onLayerAdded((*outHandle)->localBinder(), layer->sequence, args.name,
		if (mTransactionTracing) {
		mTransactionTracing->onLayerAdded((*outHandle)->localBinder(), layer->sequence, args.name,
		args.flags, parentId);
		}

		@@ -4722,8 +4720,8 @@ void SurfaceFlinger::onHandleDestroyed(BBinder* handle, sp<Layer>& layer) {
		markLayerPendingRemovalLocked(layer);
		mBufferCountTracker.remove(handle);
		layer.clear();
		if (mTransactionTracingEnabled) {
		mTransactionTracing.onHandleRemoved(handle);
		if (mTransactionTracing) {
		mTransactionTracing->onHandleRemoved(handle);
		}
		}

		@@ -4958,7 +4956,9 @@ status_t SurfaceFlinger::doDump(int fd, const DumpArgs& args, bool asProto) {
		status_t SurfaceFlinger::dumpCritical(int fd, const DumpArgs&, bool asProto) {
		if (asProto) {
		mLayerTracing.writeToFile();
		mTransactionTracing.writeToFile();
		if (mTransactionTracing) {
		mTransactionTracing->writeToFile();
		}
		}

		return doDump(fd, DumpArgs(), asProto);
		@@ -5355,9 +5355,15 @@ void SurfaceFlinger::dumpAllLocked(const DumpArgs& args, std::string& result) co
		* Tracing state
		*/
		mLayerTracing.dump(result);
		result.append("\n");
		mTransactionTracing.dump(result);
		result.append("\n");

		result.append("\nTransaction tracing: ");
		if (mTransactionTracing) {
		result.append("enabled\n");
		mTransactionTracing->dump(result);
		} else {
		result.append("disabled\n");
		}
		result.push_back('\n');

		/*
		* HWC layer minidump
		@@ -6037,15 +6043,17 @@ status_t SurfaceFlinger::onTransact(uint32_t code, const Parcel& data, Parcel* r
		return NO_ERROR;
		}
		case 1041: { // Transaction tracing
		if (mTransactionTracing) {
		if (data.readInt32()) {
		// Transaction tracing is always running but allow the user to temporarily
		// increase the buffer when actively debugging.
		mTransactionTracing.setBufferSize(
		mTransactionTracing->setBufferSize(
		TransactionTracing::ACTIVE_TRACING_BUFFER_SIZE);
		} else {
		mTransactionTracing.setBufferSize(
		mTransactionTracing->setBufferSize(
		TransactionTracing::CONTINUOUS_TRACING_BUFFER_SIZE);
		mTransactionTracing.writeToFile();
		mTransactionTracing->writeToFile();
		}
		}
		reply->writeInt32(NO_ERROR);
		return NO_ERROR;
		@@ -6887,8 +6895,8 @@ void SurfaceFlinger::onLayerDestroyed(Layer* layer) {
		if (!layer->isRemovedFromCurrentState()) {
		mScheduler->deregisterLayer(layer);
		}
		if (mTransactionTracingEnabled) {
		mTransactionTracing.onLayerRemoved(layer->getSequence());
		if (mTransactionTracing) {
		mTransactionTracing->onLayerRemoved(layer->getSequence());
		}
		}

services/surfaceflinger/SurfaceFlinger.h

+1 −2

Original line number	Diff line number	Diff line
		@@ -1197,8 +1197,7 @@ private:
		LayerTracing mLayerTracing{*this};
		bool mLayerTracingEnabled = false;

		TransactionTracing mTransactionTracing;
		bool mTransactionTracingEnabled = false;
		std::optional<TransactionTracing> mTransactionTracing;
		std::atomic<bool> mTracingEnabledChanged = false;

		const std::shared_ptr<TimeStats> mTimeStats;

services/surfaceflinger/Tracing/TransactionTracing.cpp

+12 −53

Original line number	Diff line number	Diff line
		@@ -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 @@ bool TransactionTracing::disable() {
		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 @@ proto::TransactionTraceFile TransactionTracing::createTraceFileProto() const {

		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 @@ void TransactionTracing::addEntry(const std::vector<CommittedTransactions>& comm
		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 @@ void TransactionTracing::flush(int64_t vsyncId) {
		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 @@ proto::TransactionTraceFile TransactionTracing::writeToProto() {
		std::scoped_lock<std::mutex> lock(mTraceLock);
		proto::TransactionTraceFile proto = createTraceFileProto();
		addStartingStateToProtoLocked(proto);
		mBuffer->writeToProto(proto);
		mBuffer.writeToProto(proto);
		return proto;
		}

services/surfaceflinger/Tracing/TransactionTracing.h

+3 −10

Original line number	Diff line number	Diff line
		@@ -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 @@ public:
		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 @@ private:
		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 @@ private:
		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.

services/surfaceflinger/tests/unittests/TransactionTracingTest.cpp

+30 −105

Original line number	Diff line number	Diff line
		@@ -29,79 +29,32 @@ namespace android {
		class TransactionTracingTest : public testing::Test {
		protected:
		static constexpr size_t SMALL_BUFFER_SIZE = 1024;
		std::unique_ptr<android::TransactionTracing> mTracing;
		void SetUp() override { mTracing = std::make_unique<android::TransactionTracing>(); }
		TransactionTracing mTracing;

		void TearDown() override {
		mTracing->disable();
		mTracing.reset();
		}

		auto getCommittedTransactions() {
		std::scoped_lock<std::mutex> lock(mTracing->mMainThreadLock);
		return mTracing->mCommittedTransactions;
		}

		auto getQueuedTransactions() {
		std::scoped_lock<std::mutex> lock(mTracing->mTraceLock);
		return mTracing->mQueuedTransactions;
		}

		auto getUsedBufferSize() {
		std::scoped_lock<std::mutex> lock(mTracing->mTraceLock);
		return mTracing->mBuffer->used();
		}

		auto flush(int64_t vsyncId) { return mTracing->flush(vsyncId); }
		void flush(int64_t vsyncId) { mTracing.flush(vsyncId); }
		proto::TransactionTraceFile writeToProto() { return mTracing.writeToProto(); }

		auto bufferFront() {
		std::scoped_lock<std::mutex> lock(mTracing->mTraceLock);
		proto::TransactionTraceEntry bufferFront() {
		std::scoped_lock<std::mutex> lock(mTracing.mTraceLock);
		proto::TransactionTraceEntry entry;
		entry.ParseFromString(mTracing->mBuffer->front());
		entry.ParseFromString(mTracing.mBuffer.front());
		return entry;
		}

		bool threadIsJoinable() {
		std::scoped_lock lock(mTracing->mMainThreadLock);
		return mTracing->mThread.joinable();
		}

		proto::TransactionTraceFile writeToProto() { return mTracing->writeToProto(); }

		auto getCreatedLayers() {
		std::scoped_lock<std::mutex> lock(mTracing->mTraceLock);
		return mTracing->mCreatedLayers;
		}

		auto getStartingStates() {
		std::scoped_lock<std::mutex> lock(mTracing->mTraceLock);
		return mTracing->mStartingStates;
		}

		void queueAndCommitTransaction(int64_t vsyncId) {
		TransactionState transaction;
		transaction.id = static_cast<uint64_t>(vsyncId * 3);
		transaction.originUid = 1;
		transaction.originPid = 2;
		mTracing->addQueuedTransaction(transaction);
		mTracing.addQueuedTransaction(transaction);
		std::vector<TransactionState> transactions;
		transactions.emplace_back(transaction);
		mTracing->addCommittedTransactions(transactions, vsyncId);
		mTracing.addCommittedTransactions(transactions, vsyncId);
		flush(vsyncId);
		}

		// Test that we clean up the tracing thread and free any memory allocated.
		void verifyDisabledTracingState() {
		EXPECT_FALSE(mTracing->isEnabled());
		EXPECT_FALSE(threadIsJoinable());
		EXPECT_EQ(getCommittedTransactions().size(), 0u);
		EXPECT_EQ(getQueuedTransactions().size(), 0u);
		EXPECT_EQ(getUsedBufferSize(), 0u);
		EXPECT_EQ(getStartingStates().size(), 0u);
		}

		void verifyEntry(const proto::TransactionTraceEntry& actualProto,
		const std::vector<TransactionState> expectedTransactions,
		const std::vector<TransactionState>& expectedTransactions,
		int64_t expectedVsyncId) {
		EXPECT_EQ(actualProto.vsync_id(), expectedVsyncId);
		EXPECT_EQ(actualProto.transactions().size(),
		@@ -113,16 +66,7 @@ protected:
		}
		};

		TEST_F(TransactionTracingTest, enable) {
		EXPECT_FALSE(mTracing->isEnabled());
		mTracing->enable();
		EXPECT_TRUE(mTracing->isEnabled());
		mTracing->disable();
		verifyDisabledTracingState();
		}

		TEST_F(TransactionTracingTest, addTransactions) {
		mTracing->enable();
		std::vector<TransactionState> transactions;
		transactions.reserve(100);
		for (uint64_t i = 0; i < 100; i++) {
		@@ -130,7 +74,7 @@ TEST_F(TransactionTracingTest, addTransactions) {
		transaction.id = i;
		transaction.originPid = static_cast<int32_t>(i);
		transactions.emplace_back(transaction);
		mTracing->addQueuedTransaction(transaction);
		mTracing.addQueuedTransaction(transaction);
		}

		// Split incoming transactions into two and commit them in reverse order to test out of order
		@@ -138,12 +82,12 @@ TEST_F(TransactionTracingTest, addTransactions) {
		std::vector<TransactionState> firstTransactionSet =
		std::vector<TransactionState>(transactions.begin() + 50, transactions.end());
		int64_t firstTransactionSetVsyncId = 42;
		mTracing->addCommittedTransactions(firstTransactionSet, firstTransactionSetVsyncId);
		mTracing.addCommittedTransactions(firstTransactionSet, firstTransactionSetVsyncId);

		int64_t secondTransactionSetVsyncId = 43;
		std::vector<TransactionState> secondTransactionSet =
		std::vector<TransactionState>(transactions.begin(), transactions.begin() + 50);
		mTracing->addCommittedTransactions(secondTransactionSet, secondTransactionSetVsyncId);
		mTracing.addCommittedTransactions(secondTransactionSet, secondTransactionSetVsyncId);
		flush(secondTransactionSetVsyncId);

		proto::TransactionTraceFile proto = writeToProto();
		@@ -151,23 +95,18 @@ TEST_F(TransactionTracingTest, addTransactions) {
		// skip starting entry
		verifyEntry(proto.entry(1), firstTransactionSet, firstTransactionSetVsyncId);
		verifyEntry(proto.entry(2), secondTransactionSet, secondTransactionSetVsyncId);

		mTracing->disable();
		verifyDisabledTracingState();
		}

		class TransactionTracingLayerHandlingTest : public TransactionTracingTest {
		protected:
		void SetUp() override {
		TransactionTracingTest::SetUp();
		mTracing->enable();
		// add layers
		mTracing->setBufferSize(SMALL_BUFFER_SIZE);
		mTracing.setBufferSize(SMALL_BUFFER_SIZE);
		const sp<IBinder> fakeLayerHandle = new BBinder();
		mTracing->onLayerAdded(fakeLayerHandle->localBinder(), mParentLayerId, "parent",
		mTracing.onLayerAdded(fakeLayerHandle->localBinder(), mParentLayerId, "parent",
		123 /* flags /, -1 / parentId */);
		const sp<IBinder> fakeChildLayerHandle = new BBinder();
		mTracing->onLayerAdded(fakeChildLayerHandle->localBinder(), mChildLayerId, "child",
		mTracing.onLayerAdded(fakeChildLayerHandle->localBinder(), mChildLayerId, "child",
		456 /* flags */, mParentLayerId);

		// add some layer transaction
		@@ -184,12 +123,12 @@ protected:
		childState.state.what = layer_state_t::eLayerChanged;
		childState.state.z = 43;
		transaction.states.add(childState);
		mTracing->addQueuedTransaction(transaction);
		mTracing.addQueuedTransaction(transaction);

		std::vector<TransactionState> transactions;
		transactions.emplace_back(transaction);
		VSYNC_ID_FIRST_LAYER_CHANGE = ++mVsyncId;
		mTracing->addCommittedTransactions(transactions, VSYNC_ID_FIRST_LAYER_CHANGE);
		mTracing.addCommittedTransactions(transactions, VSYNC_ID_FIRST_LAYER_CHANGE);
		flush(VSYNC_ID_FIRST_LAYER_CHANGE);
		}

		@@ -204,31 +143,25 @@ protected:
		layerState.state.z = 41;
		layerState.state.x = 22;
		transaction.states.add(layerState);
		mTracing->addQueuedTransaction(transaction);
		mTracing.addQueuedTransaction(transaction);

		std::vector<TransactionState> transactions;
		transactions.emplace_back(transaction);
		VSYNC_ID_SECOND_LAYER_CHANGE = ++mVsyncId;
		mTracing->addCommittedTransactions(transactions, VSYNC_ID_SECOND_LAYER_CHANGE);
		mTracing.addCommittedTransactions(transactions, VSYNC_ID_SECOND_LAYER_CHANGE);
		flush(VSYNC_ID_SECOND_LAYER_CHANGE);
		}

		// remove child layer
		mTracing->onLayerRemoved(2);
		mTracing.onLayerRemoved(2);
		VSYNC_ID_CHILD_LAYER_REMOVED = ++mVsyncId;
		queueAndCommitTransaction(VSYNC_ID_CHILD_LAYER_REMOVED);

		// remove layer
		mTracing->onLayerRemoved(1);
		mTracing.onLayerRemoved(1);
		queueAndCommitTransaction(++mVsyncId);
		}

		void TearDown() override {
		mTracing->disable();
		verifyDisabledTracingState();
		TransactionTracingTest::TearDown();
		}

		int mParentLayerId = 1;
		int mChildLayerId = 2;
		int64_t mVsyncId = 0;
		@@ -298,15 +231,13 @@ TEST_F(TransactionTracingLayerHandlingTest, startingStateSurvivesBufferFlush) {
		class TransactionTracingMirrorLayerTest : public TransactionTracingTest {
		protected:
		void SetUp() override {
		TransactionTracingTest::SetUp();
		mTracing->enable();
		// add layers
		mTracing->setBufferSize(SMALL_BUFFER_SIZE);
		mTracing.setBufferSize(SMALL_BUFFER_SIZE);
		const sp<IBinder> fakeLayerHandle = new BBinder();
		mTracing->onLayerAdded(fakeLayerHandle->localBinder(), mLayerId, "Test Layer",
		mTracing.onLayerAdded(fakeLayerHandle->localBinder(), mLayerId, "Test Layer",
		123 /* flags /, -1 / parentId */);
		const sp<IBinder> fakeMirrorLayerHandle = new BBinder();
		mTracing->onMirrorLayerAdded(fakeMirrorLayerHandle->localBinder(), mMirrorLayerId, "Mirror",
		mTracing.onMirrorLayerAdded(fakeMirrorLayerHandle->localBinder(), mMirrorLayerId, "Mirror",
		mLayerId);

		// add some layer transaction
		@@ -323,21 +254,15 @@ protected:
		mirrorState.state.what = layer_state_t::eLayerChanged;
		mirrorState.state.z = 43;
		transaction.states.add(mirrorState);
		mTracing->addQueuedTransaction(transaction);
		mTracing.addQueuedTransaction(transaction);

		std::vector<TransactionState> transactions;
		transactions.emplace_back(transaction);
		mTracing->addCommittedTransactions(transactions, ++mVsyncId);
		mTracing.addCommittedTransactions(transactions, ++mVsyncId);
		flush(mVsyncId);
		}
		}

		void TearDown() override {
		mTracing->disable();
		verifyDisabledTracingState();
		TransactionTracingTest::TearDown();
		}

		int mLayerId = 5;
		int mMirrorLayerId = 55;
		int64_t mVsyncId = 0;