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

Commit 755e319d authored by Lloyd Pique's avatar Lloyd Pique
Browse files

SF: Cleanup EventControlThread 

Primarily the goal was to eliminate the use of RefBase in various forms
from EventControlThread.

1) SurfaceFlinger only needs a std::unique_ptr<> and not an
android::sp<> to own the created instance.
2) Convert from android::Thread to std::thread, along with using
std::mutex and std::condition_variable to keep consistency.
3) The code only needs a reference to a function to call, rather than a
reference to all of SurfaceFlinger. This removes an unnecessary full
dependency.
4) Switch the header to #pragma once.
5) Added Clang thread annotations and enabled the corresponding warning.
6) Simplified the thread function to eliminate unnecessary locals and
indentation.
7) Added proper thread shutdown handling (invoked by dtor).

Bug: None
Test: Verified event control thread still works on Pixel XL

Change-Id: I2d5621b0cbbfb9e0f8c5831ccfc94704c95a4a55
parent 78ce418e

services/surfaceflinger/Android.bp

+1 −0
Original line number Diff line number Diff line
@@ -4,6 +4,7 @@ cc_defaults { 
        "-DLOG_TAG=\"SurfaceFlinger\"",

        "-Wall",

        "-Werror",

        "-Wthread-safety",

        "-Wunused",

        "-Wunreachable-code",

    ],

services/surfaceflinger/EventControlThread.cpp

+41 −29
Original line number Diff line number Diff line
@@ -14,45 +14,57 @@ 
 * limitations under the License.

 */



#include <pthread.h>

#include <sched.h>

#include <sys/resource.h>



#include <cutils/sched_policy.h>

#include <log/log.h>

#include <system/thread_defs.h>



#include "EventControlThread.h"

#include "SurfaceFlinger.h"



namespace android {



EventControlThread::EventControlThread(const sp<SurfaceFlinger>& flinger)

      : mFlinger(flinger), mVsyncEnabled(false) {}

EventControlThread::EventControlThread(EventControlThread::SetVSyncEnabledFunction function)

      : mSetVSyncEnabled(function) {

    pthread_setname_np(mThread.native_handle(), "EventControlThread");



void EventControlThread::setVsyncEnabled(bool enabled) {

    Mutex::Autolock lock(mMutex);

    mVsyncEnabled = enabled;

    mCond.signal();

    pid_t tid = pthread_gettid_np(mThread.native_handle());

    setpriority(PRIO_PROCESS, tid, ANDROID_PRIORITY_URGENT_DISPLAY);

    set_sched_policy(tid, SP_FOREGROUND);

}



bool EventControlThread::threadLoop() {

    enum class VsyncState { Unset, On, Off };

    auto currentVsyncState = VsyncState::Unset;



    while (true) {

        auto requestedVsyncState = VsyncState::On;

EventControlThread::~EventControlThread() {

    {

            Mutex::Autolock lock(mMutex);

            requestedVsyncState = mVsyncEnabled ? VsyncState::On : VsyncState::Off;

            while (currentVsyncState == requestedVsyncState) {

                status_t err = mCond.wait(mMutex);

                if (err != NO_ERROR) {

                    ALOGE("error waiting for new events: %s (%d)", strerror(-err), err);

                    return false;

                }

                requestedVsyncState = mVsyncEnabled ? VsyncState::On : VsyncState::Off;

        std::lock_guard<std::mutex> lock(mMutex);

        mKeepRunning = false;

        mCondition.notify_all();

    }

    mThread.join();

}



        bool enable = requestedVsyncState == VsyncState::On;

        mFlinger->setVsyncEnabled(HWC_DISPLAY_PRIMARY, enable);

        currentVsyncState = requestedVsyncState;

void EventControlThread::setVsyncEnabled(bool enabled) {

    std::lock_guard<std::mutex> lock(mMutex);

    mVsyncEnabled = enabled;

    mCondition.notify_all();

}



    return false;

// Unfortunately std::unique_lock gives warnings with -Wthread-safety

void EventControlThread::threadMain() NO_THREAD_SAFETY_ANALYSIS {

    auto keepRunning = true;

    auto currentVsyncEnabled = false;



    while (keepRunning) {

        mSetVSyncEnabled(currentVsyncEnabled);



        std::unique_lock<std::mutex> lock(mMutex);

        mCondition.wait(lock, [this, currentVsyncEnabled, keepRunning]() NO_THREAD_SAFETY_ANALYSIS {

            return currentVsyncEnabled != mVsyncEnabled || keepRunning != mKeepRunning;

        });

        currentVsyncEnabled = mVsyncEnabled;

        keepRunning = mKeepRunning;

    }

}



} // namespace android

services/surfaceflinger/EventControlThread.h

+22 −16
Original line number Diff line number Diff line
@@ -14,35 +14,41 @@ 
 * limitations under the License.

 */



#ifndef ANDROID_EVENTCONTROLTHREAD_H

#define ANDROID_EVENTCONTROLTHREAD_H

#pragma once



#include <stddef.h>

#include <cstddef>

#include <condition_variable>

#include <functional>

#include <mutex>

#include <thread>



#include <utils/Mutex.h>

#include <utils/Thread.h>

#include <android-base/thread_annotations.h>



namespace android {



class SurfaceFlinger;



class EventControlThread: public Thread {

class EventControlThread {

public:

    using SetVSyncEnabledFunction = std::function<void(bool)>;



    explicit EventControlThread(const sp<SurfaceFlinger>& flinger);

    virtual ~EventControlThread() {}

    explicit EventControlThread(SetVSyncEnabledFunction function);

    ~EventControlThread();



    void setVsyncEnabled(bool enabled);

    virtual bool threadLoop();



private:

    sp<SurfaceFlinger> mFlinger;

    bool mVsyncEnabled;

    void threadMain();



    Mutex mMutex;

    Condition mCond;

};

    std::mutex mMutex;

    std::condition_variable mCondition;



    const SetVSyncEnabledFunction mSetVSyncEnabled;

    bool mVsyncEnabled GUARDED_BY(mMutex) = false;

    bool mKeepRunning GUARDED_BY(mMutex) = true;



}

    // Must be last so that everything is initialized before the thread starts.

    std::thread mThread{&EventControlThread::threadMain, this};

};



#endif // ANDROID_EVENTCONTROLTHREAD_H
 
} // namespace android

services/surfaceflinger/SurfaceFlinger.cpp

+7 −4
Original line number Diff line number Diff line
@@ -636,8 +636,9 @@ void SurfaceFlinger::init() { 
        }

    }



    mEventControlThread = new EventControlThread(this);

    mEventControlThread->run("EventControl", PRIORITY_URGENT_DISPLAY);

    mEventControlThread = std::make_unique<EventControlThread>([this](bool enabled) {

        setVsyncEnabled(HWC_DISPLAY_PRIMARY, enabled);

    });



    // initialize our drawing state

    mDrawingState = mCurrentState;
@@ -1101,7 +1102,8 @@ status_t SurfaceFlinger::injectVSync(nsecs_t when) { 
    return NO_ERROR;

}



status_t SurfaceFlinger::getLayerDebugInfo(std::vector<LayerDebugInfo>* outLayers) const {

status_t SurfaceFlinger::getLayerDebugInfo(std::vector<LayerDebugInfo>* outLayers) const

        NO_THREAD_SAFETY_ANALYSIS {

    IPCThreadState* ipc = IPCThreadState::self();

    const int pid = ipc->getCallingPid();

    const int uid = ipc->getCallingUid();
@@ -3567,7 +3569,8 @@ void SurfaceFlinger::setPowerMode(const sp<IBinder>& display, int mode) { 


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



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

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

        NO_THREAD_SAFETY_ANALYSIS {

    String8 result;



    IPCThreadState* ipc = IPCThreadState::self();

services/surfaceflinger/SurfaceFlinger.h

+1 −1
Original line number Diff line number Diff line
@@ -723,7 +723,7 @@ private: 
    sp<EventThread> mSFEventThread;

    sp<EventThread> mInjectorEventThread;

    sp<InjectVSyncSource> mVSyncInjector;

    sp<EventControlThread> mEventControlThread;

    std::unique_ptr<EventControlThread> mEventControlThread;

    sp<IBinder> mBuiltinDisplays[DisplayDevice::NUM_BUILTIN_DISPLAY_TYPES];



    // Can only accessed from the main thread, these members