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

Commit ca68e181 authored by Shai Barack's avatar Shai Barack Committed by Android (Google) Code Review
Browse files

Revert "Don't call epoll_wait with zero timeout unless watching FDs" 

This reverts commit ea469023.

Reason for revert: b/410218466#comment9

Bug: 410218466
Change-Id: Ic8fb29334830cb6a12904f087ab1e2b1e7ac6fdb
parent ea469023

libutils/Looper.cpp

+6 −13
Original line number Diff line number Diff line
@@ -216,18 +216,14 @@ int Looper::pollInner(int timeoutMillis) { 
    mResponses.clear();

    mResponseIndex = 0;



    struct epoll_event eventItems[EPOLL_MAX_EVENTS];

    int eventCount = 0;

    // Poll if we need to wait or have requests that may have FD events.

    if (timeoutMillis != 0 || mHasRequests) {

    // We are about to idle.

    std::atomic_store_explicit(&mPolling, true, std::memory_order_relaxed);



        eventCount = epoll_wait(mEpollFd.get(), eventItems, EPOLL_MAX_EVENTS, timeoutMillis);

    struct epoll_event eventItems[EPOLL_MAX_EVENTS];

    int eventCount = epoll_wait(mEpollFd.get(), eventItems, EPOLL_MAX_EVENTS, timeoutMillis);



    // No longer idling.

    std::atomic_store_explicit(&mPolling, false, std::memory_order_relaxed);

    }



    // Acquire lock.

    mLock.lock();
@@ -497,8 +493,6 @@ int Looper::addFd(int fd, int ident, int events, const sp<LooperCallback>& callb 
            mRequests.emplace(seq, request);

            seq_it->second = seq;

        }



        mHasRequests = !mRequests.empty();

    } // release lock

    return 1;

}
@@ -566,7 +560,6 @@ int Looper::removeSequenceNumberLocked(SequenceNumber seq) { 
    // updating the epoll set so that we avoid accidentally leaking callbacks.

    mRequests.erase(request_it);

    mSequenceNumberByFd.erase(fd);

    mHasRequests = !mRequests.empty();



    int epollResult = epoll_ctl(mEpollFd.get(), EPOLL_CTL_DEL, fd, nullptr);

    if (epollResult < 0) {

libutils/Looper_test.cpp

+6 −3
Original line number Diff line number Diff line
@@ -708,15 +708,18 @@ TEST_F(LooperTest, RemoveMessage_WhenRemovingAllMessagesForHandler_ShouldRemoveT 
    mLooper->sendMessage(handler, Message(MSG_TEST3));

    mLooper->removeMessages(handler);



    constexpr int MIN_POLL_TIMEOUT_MS = 1;

    int result = mLooper->pollOnce(MIN_POLL_TIMEOUT_MS);

    StopWatch stopWatch("pollOnce");

    int result = mLooper->pollOnce(0);

    int32_t elapsedMillis = ns2ms(stopWatch.elapsedTime());



    EXPECT_NEAR(0, elapsedMillis, TIMING_TOLERANCE_MS)

            << "elapsed time should approx. zero because message was sent so looper was awoken";

    EXPECT_EQ(Looper::POLL_WAKE, result)

            << "pollOnce result should be Looper::POLL_WAKE because looper was awoken";

    EXPECT_EQ(size_t(0), handler->messages.size())

            << "no messages to handle";



    result = mLooper->pollOnce(MIN_POLL_TIMEOUT_MS);

    result = mLooper->pollOnce(0);



    EXPECT_EQ(Looper::POLL_TIMEOUT, result)

            << "pollOnce result should be Looper::POLL_TIMEOUT because there was nothing to do";

libutils/abi-dumps/arm64/source-based/libutils.so.lsdump

+256 −262

File changed.

Preview size limit exceeded, changes collapsed.

libutils/abi-dumps/arm_arm64/source-based/libutils.so.lsdump

+250 −256

File changed.

Preview size limit exceeded, changes collapsed.

libutils/include/utils/Looper.h

+0 −9
Original line number Diff line number Diff line
@@ -490,15 +490,6 @@ private: 
    std::unordered_map<SequenceNumber, Request> mRequests;               // guarded by mLock

    std::unordered_map<int /*fd*/, SequenceNumber> mSequenceNumberByFd;  // guarded by mLock



    // Whether mRequests has one or more elements.

    // Volatile to allow the reader (looper thread) to read it while the writer (any thread)

    // may be modifying it without acquiring a lock.

    // This is safe because the looper thread uses this value to determine if it may skip

    // epoll_wait() when there are no requests. If there is a race then it doesn't matter how the

    // race is resolved because the worst case is we'll skip one epoll_wait() call but won't skip

    // subsequent ones.

    volatile bool mHasRequests;



    // The sequence number to use for the next fd that is added to the looper.

    // The sequence number 0 is reserved for the WakeEventFd.

    SequenceNumber mNextRequestSeq;  // guarded by mLock