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

Commit c2867147 authored by Dominik Laskowski's avatar Dominik Laskowski
Browse files

SF: Simplify dumpsys flag parsing 

This CL removes sequential parsing for dumpsys flags, which was order-
dependent and boilerplate-heavy.

Note that flags cannot be combined for simplicity.

Bug: None
Test: dumpsys SurfaceFlinger [--FLAG [ARGS...]]
Change-Id: Id305ffee464c6cad4f228c7a7f603c33a2978d66
parent 8cf0bfa7

services/surfaceflinger/SurfaceFlinger.cpp

+40 −135
Original line number Diff line number Diff line
@@ -28,6 +28,7 @@ 
#include <functional>

#include <mutex>

#include <optional>

#include <unordered_map>



#include <cutils/properties.h>

#include <log/log.h>
@@ -4344,8 +4345,8 @@ void SurfaceFlinger::setPowerMode(const sp<IBinder>& displayToken, int mode) { 


// ---------------------------------------------------------------------------



status_t SurfaceFlinger::doDump(int fd, const Vector<String16>& args, bool asProto)

        NO_THREAD_SAFETY_ANALYSIS {

status_t SurfaceFlinger::doDump(int fd, const DumpArgs& args,

                                bool asProto) NO_THREAD_SAFETY_ANALYSIS {

    std::string result;



    IPCThreadState* ipc = IPCThreadState::self();
@@ -4369,114 +4370,35 @@ status_t SurfaceFlinger::doDump(int fd, const Vector<String16>& args, bool asPro 
                          strerror(-err), err);

        }



        bool dumpAll = true;

        size_t index = 0;

        size_t numArgs = args.size();



        if (numArgs) {

            if ((index < numArgs) &&

                    (args[index] == String16("--list"))) {

                index++;

                listLayersLocked(args, index, result);

                dumpAll = false;

            }



            if ((index < numArgs) &&

                    (args[index] == String16("--latency"))) {

                index++;

                dumpStatsLocked(args, index, result);

                dumpAll = false;

            }



            if ((index < numArgs) &&

                    (args[index] == String16("--latency-clear"))) {

                index++;

                clearStatsLocked(args, index, result);

                dumpAll = false;

            }



            if ((index < numArgs) &&

                    (args[index] == String16("--dispsync"))) {

                index++;

                mPrimaryDispSync->dump(result);

                dumpAll = false;

            }



            if ((index < numArgs) &&

                    (args[index] == String16("--static-screen"))) {

                index++;

                dumpStaticScreenStats(result);

                dumpAll = false;

            }



            if ((index < numArgs) &&

                    (args[index] == String16("--frame-events"))) {

                index++;

                dumpFrameEventsLocked(result);

                dumpAll = false;

            }



            if ((index < numArgs) && (args[index] == String16("--wide-color"))) {

                index++;

                dumpWideColorInfo(result);

                dumpAll = false;

            }



            if ((index < numArgs) &&

                (args[index] == String16("--enable-layer-stats"))) {

                index++;

                mLayerStats.enable();

                dumpAll = false;

            }



            if ((index < numArgs) &&

                (args[index] == String16("--disable-layer-stats"))) {

                index++;

                mLayerStats.disable();

                dumpAll = false;

            }



            if ((index < numArgs) &&

                (args[index] == String16("--clear-layer-stats"))) {

                index++;

                mLayerStats.clear();

                dumpAll = false;

            }



            if ((index < numArgs) &&

                (args[index] == String16("--dump-layer-stats"))) {

                index++;

                mLayerStats.dump(result);

                dumpAll = false;

            }



            if ((index < numArgs) &&

                    (args[index] == String16("--frame-composition"))) {

                index++;

                dumpFrameCompositionInfo(result);

                dumpAll = false;

            }



            if ((index < numArgs) &&

                (args[index] == String16("--display-identification"))) {

                index++;

                dumpDisplayIdentificationData(result);

                dumpAll = false;

            }

        using namespace std::string_literals;



        static const std::unordered_map<std::string, Dumper> dumpers = {

                {"--clear-layer-stats"s, dumper([this](std::string&) { mLayerStats.clear(); })},

                {"--disable-layer-stats"s, dumper([this](std::string&) { mLayerStats.disable(); })},

                {"--display-id"s, dumper(&SurfaceFlinger::dumpDisplayIdentificationData)},

                {"--dispsync"s, dumper([this](std::string& s) { mPrimaryDispSync->dump(s); })},

                {"--dump-layer-stats"s, dumper([this](std::string& s) { mLayerStats.dump(s); })},

                {"--enable-layer-stats"s, dumper([this](std::string&) { mLayerStats.enable(); })},

                {"--frame-composition"s, dumper(&SurfaceFlinger::dumpFrameCompositionInfo)},

                {"--frame-events"s, dumper(&SurfaceFlinger::dumpFrameEventsLocked)},

                {"--latency"s, argsDumper(&SurfaceFlinger::dumpStatsLocked)},

                {"--latency-clear"s, argsDumper(&SurfaceFlinger::clearStatsLocked)},

                {"--list"s, dumper(&SurfaceFlinger::listLayersLocked)},

                {"--static-screen"s, dumper(&SurfaceFlinger::dumpStaticScreenStats)},

                {"--timestats"s, protoDumper(&SurfaceFlinger::dumpTimeStats)},

                {"--wide-color"s, dumper(&SurfaceFlinger::dumpWideColorInfo)},

        };



            if ((index < numArgs) && (args[index] == String16("--timestats"))) {

                index++;

                mTimeStats->parseArgs(asProto, args, index, result);

                dumpAll = false;

            }

        }

        const auto flag = args.empty() ? ""s : std::string(String8(args[0]));



        if (dumpAll) {

        if (const auto it = dumpers.find(flag); it != dumpers.end()) {

            (it->second)(args, asProto, result);

        } else {

            if (asProto) {

                LayersProto layersProto = dumpProtoInfo(LayerVector::StateSet::Current);

                result.append(layersProto.SerializeAsString().c_str(), layersProto.ByteSize());

            } else {

                dumpAllLocked(args, index, result);

                dumpAllLocked(args, result);

            }

        }
@@ -4488,43 +4410,29 @@ status_t SurfaceFlinger::doDump(int fd, const Vector<String16>& args, bool asPro 
    return NO_ERROR;

}



void SurfaceFlinger::listLayersLocked(const Vector<String16>& /* args */, size_t& /* index */,

                                      std::string& result) const {

void SurfaceFlinger::listLayersLocked(std::string& result) const {

    mCurrentState.traverseInZOrder(

            [&](Layer* layer) { StringAppendF(&result, "%s\n", layer->getName().string()); });

}



void SurfaceFlinger::dumpStatsLocked(const Vector<String16>& args, size_t& index,

                                     std::string& result) const {

    String8 name;

    if (index < args.size()) {

        name = String8(args[index]);

        index++;

    }



void SurfaceFlinger::dumpStatsLocked(const DumpArgs& args, std::string& result) const {

    StringAppendF(&result, "%" PRId64 "\n", getVsyncPeriod());



    if (name.isEmpty()) {

        mAnimFrameTracker.dumpStats(result);

    } else {

    if (args.size() > 1) {

        const auto name = String8(args[1]);

        mCurrentState.traverseInZOrder([&](Layer* layer) {

            if (name == layer->getName()) {

                layer->dumpFrameStats(result);

            }

        });

    } else {

        mAnimFrameTracker.dumpStats(result);

    }

}



void SurfaceFlinger::clearStatsLocked(const Vector<String16>& args, size_t& index,

                                      std::string& /* result */) {

    String8 name;

    if (index < args.size()) {

        name = String8(args[index]);

        index++;

    }



void SurfaceFlinger::clearStatsLocked(const DumpArgs& args, std::string&) {

    mCurrentState.traverseInZOrder([&](Layer* layer) {

        if (name.isEmpty() || (name == layer->getName())) {

        if (args.size() < 2 || String8(args[1]) == layer->getName()) {

            layer->clearFrameStats();

        }

    });
@@ -4532,6 +4440,10 @@ void SurfaceFlinger::clearStatsLocked(const Vector<String16>& args, size_t& inde 
    mAnimFrameTracker.clearStats();

}



void SurfaceFlinger::dumpTimeStats(const DumpArgs& args, bool asProto, std::string& result) const {

    mTimeStats->parseArgs(asProto, args, result);

}



// This should only be called from the main thread.  Otherwise it would need

// the lock and should use mCurrentState rather than mDrawingState.

void SurfaceFlinger::logFrameStats() {
@@ -4760,15 +4672,8 @@ LayersProto SurfaceFlinger::dumpVisibleLayersProtoInfo(const DisplayDevice& disp 
    return layersProto;

}



void SurfaceFlinger::dumpAllLocked(const Vector<String16>& args, size_t& index,

                                   std::string& result) const {

    bool colorize = false;

    if (index < args.size()

            && (args[index] == String16("--color"))) {

        colorize = true;

        index++;

    }



void SurfaceFlinger::dumpAllLocked(const DumpArgs& args, std::string& result) const {

    const bool colorize = !args.empty() && args[0] == String16("--color");

    Colorizer colorizer(colorize);



    // figure out if we're stuck somewhere

services/surfaceflinger/SurfaceFlinger.h

+51 −20
Original line number Diff line number Diff line
@@ -68,6 +68,7 @@ 


#include <atomic>

#include <cstdint>

#include <functional>

#include <map>

#include <memory>

#include <mutex>
@@ -75,6 +76,7 @@ 
#include <set>

#include <string>

#include <thread>

#include <type_traits>

#include <unordered_map>

#include <utility>
@@ -786,19 +788,9 @@ private: 
        };

    }



    /* ------------------------------------------------------------------------

     * Debugging & dumpsys

    /*

     * Display identification

     */

public:

    status_t dumpCritical(int fd, const Vector<String16>& /*args*/, bool asProto) {

        return doDump(fd, Vector<String16>(), asProto);

    }



    status_t dumpAll(int fd, const Vector<String16>& args, bool asProto) {

        return doDump(fd, args, asProto);

    }



private:

    sp<IBinder> getPhysicalDisplayToken(DisplayId displayId) const {

        const auto it = mPhysicalDisplayTokens.find(displayId);

        return it != mPhysicalDisplayTokens.end() ? it->second : nullptr;
@@ -830,15 +822,45 @@ private: 
        return hwcDisplayId ? getHwComposer().toPhysicalDisplayId(*hwcDisplayId) : std::nullopt;

    }



    void listLayersLocked(const Vector<String16>& args, size_t& index, std::string& result) const;

    void dumpStatsLocked(const Vector<String16>& args, size_t& index, std::string& result) const

            REQUIRES(mStateLock);

    void clearStatsLocked(const Vector<String16>& args, size_t& index, std::string& result);

    void dumpAllLocked(const Vector<String16>& args, size_t& index, std::string& result) const

            REQUIRES(mStateLock);

    /*

     * Debugging & dumpsys

     */

    bool startDdmConnection();

    void appendSfConfigString(std::string& result) const;



    using DumpArgs = Vector<String16>;

    using Dumper = std::function<void(const DumpArgs&, bool asProto, std::string&)>;



    template <typename F, std::enable_if_t<!std::is_member_function_pointer_v<F>>* = nullptr>

    static Dumper dumper(F&& dump) {

        using namespace std::placeholders;

        return std::bind(std::forward<F>(dump), _3);

    }



    template <typename F, std::enable_if_t<std::is_member_function_pointer_v<F>>* = nullptr>

    Dumper dumper(F dump) {

        using namespace std::placeholders;

        return std::bind(dump, this, _3);

    }



    template <typename F>

    Dumper argsDumper(F dump) {

        using namespace std::placeholders;

        return std::bind(dump, this, _1, _3);

    }



    template <typename F>

    Dumper protoDumper(F dump) {

        using namespace std::placeholders;

        return std::bind(dump, this, _1, _2, _3);

    }



    void dumpAllLocked(const DumpArgs& args, std::string& result) const REQUIRES(mStateLock);



    void appendSfConfigString(std::string& result) const;

    void listLayersLocked(std::string& result) const;

    void dumpStatsLocked(const DumpArgs& args, std::string& result) const REQUIRES(mStateLock);

    void clearStatsLocked(const DumpArgs& args, std::string& result);

    void dumpTimeStats(const DumpArgs& args, bool asProto, std::string& result) const;

    void logFrameStats();



    void dumpStaticScreenStats(std::string& result) const;
@@ -857,7 +879,16 @@ private: 
    bool isLayerTripleBufferingDisabled() const {

        return this->mLayerTripleBufferingDisabled;

    }

    status_t doDump(int fd, const Vector<String16>& args, bool asProto);



    status_t doDump(int fd, const DumpArgs& args, bool asProto);



    status_t dumpCritical(int fd, const DumpArgs&, bool asProto) override {

        return doDump(fd, DumpArgs(), asProto);

    }



    status_t dumpAll(int fd, const DumpArgs& args, bool asProto) override {

        return doDump(fd, args, asProto);

    }



    /* ------------------------------------------------------------------------

     * VrFlinger

services/surfaceflinger/TimeStats/TimeStats.cpp

+2 −9
Original line number Diff line number Diff line
@@ -32,19 +32,12 @@ 


namespace android {



void TimeStats::parseArgs(bool asProto, const Vector<String16>& args, size_t& index,

                          std::string& result) {

void TimeStats::parseArgs(bool asProto, const Vector<String16>& args, std::string& result) {

    ATRACE_CALL();



    if (args.size() > index + 10) {

        ALOGD("Invalid args count");

        return;

    }



    std::unordered_map<std::string, int32_t> argsMap;

    while (index < args.size()) {

    for (size_t index = 0; index < args.size(); ++index) {

        argsMap[std::string(String8(args[index]).c_str())] = index;

        ++index;

    }



    if (argsMap.count("-disable")) {

services/surfaceflinger/TimeStats/TimeStats.h

+1 −1
Original line number Diff line number Diff line
@@ -77,7 +77,7 @@ public: 
    TimeStats() = default;

    ~TimeStats() = default;



    void parseArgs(bool asProto, const Vector<String16>& args, size_t& index, std::string& result);

    void parseArgs(bool asProto, const Vector<String16>& args, std::string& result);

    bool isEnabled();



    void incrementTotalFrames();

services/surfaceflinger/tests/unittests/TimeStatsTest.cpp

+1 −2
Original line number Diff line number Diff line
@@ -131,7 +131,6 @@ public: 
};



std::string TimeStatsTest::inputCommand(InputCommand cmd, bool useProto) {

    size_t index = 0;

    std::string result;

    Vector<String16> args;
@@ -162,7 +161,7 @@ std::string TimeStatsTest::inputCommand(InputCommand cmd, bool useProto) { 
            ALOGD("Invalid control command");

    }



    EXPECT_NO_FATAL_FAILURE(mTimeStats->parseArgs(useProto, args, index, result));

    EXPECT_NO_FATAL_FAILURE(mTimeStats->parseArgs(useProto, args, result));

    return result;

}