Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 3e40cddb authored by Vishnu Nair's avatar Vishnu Nair
Browse files

LayerTraceGenerator: Fix layer deletion 

Fixes a couple of issues with layer handle tracking which manifested
in the generated trace showing layers which were destroyed.
1. Destroyed handles were not written to proto correctly
2. When a handle was destroyed, it was removed from the tracing handle
to layer id map immediately. but we need to access this mapping in the
tracing thread when writing the transaction to proto.

Test: atest transactiontrace_testsuite
Bug: 235376060
Change-Id: I021be9a3864bdfc61422fc2f751f9bfd5e674059
parent 286f4f9e

services/surfaceflinger/Tracing/TransactionTracing.cpp

+20 −10
Original line number Diff line number Diff line
@@ -281,6 +281,14 @@ void TransactionTracing::addEntry(const std::vector<CommittedTransactions>& comm 
            }

        }



        entryProto.mutable_removed_layer_handles()->Reserve(

                static_cast<int32_t>(mRemovedLayerHandles.size()));

        for (auto& [handle, layerId] : mRemovedLayerHandles) {

            entryProto.mutable_removed_layer_handles()->Add(layerId);

            mLayerHandles.erase(handle);

        }

        mRemovedLayerHandles.clear();



        std::string serializedProto;

        entryProto.SerializeToString(&serializedProto);

        entryProto.Clear();
@@ -288,13 +296,6 @@ void TransactionTracing::addEntry(const std::vector<CommittedTransactions>& comm 
        removedEntries.reserve(removedEntries.size() + entries.size());

        removedEntries.insert(removedEntries.end(), std::make_move_iterator(entries.begin()),

                              std::make_move_iterator(entries.end()));



        entryProto.mutable_removed_layer_handles()->Reserve(

                static_cast<int32_t>(mRemovedLayerHandles.size()));

        for (auto& handle : mRemovedLayerHandles) {

            entryProto.mutable_removed_layer_handles()->Add(handle);

        }

        mRemovedLayerHandles.clear();

    }



    proto::TransactionTraceEntry removedEntryProto;
@@ -355,9 +356,7 @@ void TransactionTracing::onHandleRemoved(BBinder* layerHandle) { 
        ALOGW("handle not found. %p", layerHandle);

        return;

    }



    mRemovedLayerHandles.push_back(it->second);

    mLayerHandles.erase(it);

    mRemovedLayerHandles.emplace_back(layerHandle, it->second);

}



void TransactionTracing::tryPushToTracingThread() {
@@ -401,10 +400,15 @@ void TransactionTracing::updateStartingStateLocked( 
        }

    }



    for (const int32_t removedLayerHandleId : removedEntry.removed_layer_handles()) {

        mRemovedLayerHandlesAtStart.insert(removedLayerHandleId);

    }



    // Clean up stale starting states since the layer has been removed and the buffer does not

    // contain any references to the layer.

    for (const int32_t removedLayerId : removedEntry.removed_layers()) {

        mStartingStates.erase(removedLayerId);

        mRemovedLayerHandlesAtStart.erase(removedLayerId);

    }

}
@@ -426,6 +430,12 @@ void TransactionTracing::addStartingStateToProtoLocked(proto::TransactionTraceFi 
    transactionProto.set_vsync_id(0);

    transactionProto.set_post_time(mStartingTimestamp);

    entryProto->mutable_transactions()->Add(std::move(transactionProto));



    entryProto->mutable_removed_layer_handles()->Reserve(

            static_cast<int32_t>(mRemovedLayerHandlesAtStart.size()));

    for (const int32_t removedLayerHandleId : mRemovedLayerHandlesAtStart) {

        entryProto->mutable_removed_layer_handles()->Add(removedLayerHandleId);

    }

}



proto::TransactionTraceFile TransactionTracing::writeToProto() {

services/surfaceflinger/Tracing/TransactionTracing.h

+3 −1
Original line number Diff line number Diff line
@@ -85,8 +85,10 @@ private: 
    std::unordered_map<int, proto::LayerCreationArgs> mCreatedLayers GUARDED_BY(mTraceLock);

    std::unordered_map<BBinder* /* layerHandle */, int32_t /* layerId */> mLayerHandles

            GUARDED_BY(mTraceLock);

    std::vector<int32_t /* layerId */> mRemovedLayerHandles GUARDED_BY(mTraceLock);

    std::vector<std::pair<BBinder* /* layerHandle */, int32_t /* layerId */>> mRemovedLayerHandles

            GUARDED_BY(mTraceLock);

    std::map<int32_t /* layerId */, TracingLayerState> mStartingStates GUARDED_BY(mTraceLock);

    std::set<int32_t /* layerId */> mRemovedLayerHandlesAtStart GUARDED_BY(mTraceLock);

    TransactionProtoParser mProtoParser GUARDED_BY(mTraceLock);

    // Parses the transaction to proto without holding any tracing locks so we can generate proto

    // in the binder thread without any contention.

services/surfaceflinger/Tracing/tools/LayerTraceGenerator.cpp

+8 −6
Original line number Diff line number Diff line
@@ -16,6 +16,7 @@ 


#undef LOG_TAG

#define LOG_TAG "LayerTraceGenerator"

//#define LOG_NDEBUG 0



#include <TestableSurfaceFlinger.h>

#include <Tracing/TransactionProtoParser.h>
@@ -227,9 +228,10 @@ bool LayerTraceGenerator::generate(const proto::TransactionTraceFile& traceFile, 
    for (int i = 0; i < traceFile.entry_size(); i++) {

        proto::TransactionTraceEntry entry = traceFile.entry(i);

        ALOGV("    Entry %04d/%04d for time=%" PRId64 " vsyncid=%" PRId64

              " layers +%d -%d transactions=%d",

              " layers +%d -%d handles -%d transactions=%d",

              i, traceFile.entry_size(), entry.elapsed_realtime_nanos(), entry.vsync_id(),

              entry.added_layers_size(), entry.removed_layers_size(), entry.transactions_size());

              entry.added_layers_size(), entry.removed_layers_size(),

              entry.removed_layer_handles_size(), entry.transactions_size());



        for (int j = 0; j < entry.added_layers_size(); j++) {

            // create layers
@@ -275,13 +277,13 @@ bool LayerTraceGenerator::generate(const proto::TransactionTraceFile& traceFile, 
                                         transaction.listenerCallbacks, transaction.id);

        }



        for (int j = 0; j < entry.removed_layer_handles_size(); j++) {

            dataMapper->mLayerHandles.erase(entry.removed_layer_handles(j));

        }



        frameTime = entry.elapsed_realtime_nanos();

        vsyncId = entry.vsync_id();

        mFlinger.commit(frameTime, vsyncId);



        for (int j = 0; j < entry.removed_layer_handles_size(); j++) {

            dataMapper->mLayerHandles.erase(entry.removed_layer_handles(j));

        }

    }



    flinger->stopLayerTracing(outputLayersTracePath);