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

Commit 8eda69eb authored by Vishnu Nair's avatar Vishnu Nair
Browse files

Update pending buffer counts immediately

Increment any buffer updates in the binder thread so systrace
has an accurate idea of when the buffers arrived on the server.
The buffer count will continue to be dropped when the buffers
are dropped or latched in the main thread.

Test: capture systrace and check pending buffer counts
Change-Id: Ided1d60197db29cd490691d515443acaab864a48
parent d2bed277
Loading
Loading
Loading
Loading
+4 −9
Original line number Diff line number Diff line
@@ -871,18 +871,13 @@ bool BufferStateLayer::bufferNeedsFiltering() const {
    return layerSize.width() != bufferWidth || layerSize.height() != bufferHeight;
}

void BufferStateLayer::incrementPendingBufferCount() {
    mPendingBufferTransactions++;
    tracePendingBufferCount();
}

void BufferStateLayer::decrementPendingBufferCount() {
    mPendingBufferTransactions--;
    tracePendingBufferCount();
    int32_t pendingBuffers = --mPendingBufferTransactions;
    tracePendingBufferCount(pendingBuffers);
}

void BufferStateLayer::tracePendingBufferCount() {
    ATRACE_INT(mBlastTransactionName.c_str(), mPendingBufferTransactions);
void BufferStateLayer::tracePendingBufferCount(int32_t pendingBuffers) {
    ATRACE_INT(mBlastTransactionName.c_str(), pendingBuffers);
}

uint32_t BufferStateLayer::doTransaction(uint32_t flags) {
+4 −3
Original line number Diff line number Diff line
@@ -113,9 +113,10 @@ public:
    uint32_t getEffectiveScalingMode() const override;

    // See mPendingBufferTransactions
    void incrementPendingBufferCount() override;
    void decrementPendingBufferCount();
    uint32_t doTransaction(uint32_t flags) override;
    std::atomic<int32_t>* getPendingBufferCounter() override { return &mPendingBufferTransactions; }
    std::string getPendingBufferCounterName() override { return mBlastTransactionName; }

protected:
    void gatherBufferInfo() override;
@@ -127,7 +128,7 @@ private:
    friend class TransactionFrameTracerTest;
    friend class TransactionSurfaceFrameTest;

    inline void tracePendingBufferCount();
    inline void tracePendingBufferCount(int32_t pendingBuffers);

    bool updateFrameEventHistory(const sp<Fence>& acquireFence, nsecs_t postedTime,
                                 nsecs_t requestedPresentTime);
@@ -184,7 +185,7 @@ private:
    //     - If the integer increases, a buffer arrived at the server.
    //     - If the integer decreases in latchBuffer, that buffer was latched
    //     - If the integer decreases in setBuffer or doTransaction, a buffer was dropped
    uint64_t mPendingBufferTransactions{0};
    std::atomic<int32_t> mPendingBufferTransactions{0};

    // TODO(marissaw): support sticky transform for LEGACY camera mode

+3 −2
Original line number Diff line number Diff line
@@ -503,8 +503,6 @@ public:
    virtual void useEmptyDamage() {}
    Region getVisibleRegion(const DisplayDevice*) const;

    virtual void incrementPendingBufferCount() {}

    /*
     * isOpaque - true if this surface is opaque
     *
@@ -946,6 +944,9 @@ public:
    bool setStretchEffect(const StretchEffect& effect);
    StretchEffect getStretchEffect() const;

    virtual std::atomic<int32_t>* getPendingBufferCounter() { return nullptr; }
    virtual std::string getPendingBufferCounterName() { return ""; }

protected:
    class SyncPoint {
    public:
+16 −12
Original line number Diff line number Diff line
@@ -3268,7 +3268,6 @@ void SurfaceFlinger::flushTransactionQueues() {
                    if (!transactionIsReadyToBeApplied(transaction.isAutoTimestamp,
                                                       transaction.desiredPresentTime,
                                                       transaction.states,
                                                       false /* updateTransactionCounters*/,
                                                       pendingBuffers)) {
                        setTransactionFlags(eTransactionFlushNeeded);
                        break;
@@ -3293,13 +3292,9 @@ void SurfaceFlinger::flushTransactionQueues() {
                const auto& transaction = mTransactionQueue.front();
                bool pendingTransactions = mPendingTransactionQueues.find(transaction.applyToken) !=
                        mPendingTransactionQueues.end();
                // Call transactionIsReadyToBeApplied first in case we need to
                // incrementPendingBufferCount and keep track of pending buffers
                // if the transaction contains a buffer.
                if (!transactionIsReadyToBeApplied(transaction.isAutoTimestamp,
                                                   transaction.desiredPresentTime,
                                                   transaction.states,
                                                   true /* updateTransactionCounters */,
                                                   pendingBuffers) ||
                    pendingTransactions) {
                    mPendingTransactionQueues[transaction.applyToken].push(transaction);
@@ -3331,7 +3326,6 @@ bool SurfaceFlinger::transactionFlushNeeded() {

bool SurfaceFlinger::transactionIsReadyToBeApplied(
        bool isAutoTimestamp, int64_t desiredPresentTime, const Vector<ComposerState>& states,
        bool updateTransactionCounters,
        std::unordered_set<sp<IBinder>, ISurfaceComposer::SpHash<IBinder>>& pendingBuffers) {
    const nsecs_t expectedPresentTime = mExpectedPresentTime.load();
    bool ready = true;
@@ -3366,10 +3360,6 @@ bool SurfaceFlinger::transactionIsReadyToBeApplied(
            ATRACE_NAME("!isVsyncValidForUid");
            ready = false;
        }
        if (updateTransactionCounters) {
            // See BufferStateLayer::mPendingBufferTransactions
            layer->incrementPendingBufferCount();
        }

        // If backpressure is enabled and we already have a buffer to commit, keep the transaction
        // in the queue.
@@ -3460,6 +3450,13 @@ status_t SurfaceFlinger::setTransactionState(
        const std::vector<ListenerCallbacks>& listenerCallbacks, uint64_t transactionId) {
    ATRACE_CALL();

    // Check for incoming buffer updates and increment the pending buffer count.
    for (const auto& state : states) {
        if ((state.state.what & layer_state_t::eAcquireFenceChanged) && (state.state.surface)) {
            mBufferCountTracker.increment(state.state.surface->localBinder());
        }
    }

    uint32_t permissions =
            callingThreadHasUnscopedSurfaceFlingerAccess() ? Permission::ACCESS_SURFACE_FLINGER : 0;
    // Avoid checking for rotation permissions if the caller already has ACCESS_SURFACE_FLINGER
@@ -4037,10 +4034,16 @@ status_t SurfaceFlinger::createLayer(const String8& name, const sp<Client>& clie
                                            std::move(metadata), format, handle, gbp, &layer);

            break;
        case ISurfaceComposerClient::eFXSurfaceBufferState:
        case ISurfaceComposerClient::eFXSurfaceBufferState: {
            result = createBufferStateLayer(client, std::move(uniqueName), w, h, flags,
                                            std::move(metadata), handle, &layer);
            break;
            std::atomic<int32_t>* pendingBufferCounter = layer->getPendingBufferCounter();
            if (pendingBufferCounter) {
                std::string counterName = layer->getPendingBufferCounterName();
                mBufferCountTracker.add((*handle)->localBinder(), counterName,
                                        pendingBufferCounter);
            }
        } break;
        case ISurfaceComposerClient::eFXSurfaceEffect:
            // check if buffer size is set for color layer.
            if (w > 0 || h > 0) {
@@ -4207,6 +4210,7 @@ void SurfaceFlinger::onHandleDestroyed(sp<Layer>& layer) {
    auto it = mLayersByLocalBinderToken.begin();
    while (it != mLayersByLocalBinderToken.end()) {
        if (it->second == layer) {
            mBufferCountTracker.remove(it->first->localBinder());
            it = mLayersByLocalBinderToken.erase(it);
        } else {
            it++;
+39 −1
Original line number Diff line number Diff line
@@ -416,6 +416,43 @@ private:
        void traverseInReverseZOrder(const LayerVector::Visitor& visitor) const;
    };

    // Keeps track of pending buffers per layer handle in the transaction queue or current/drawing
    // state before the buffers are latched. The layer owns the atomic counters and decrements the
    // count in the main thread when dropping or latching a buffer.
    //
    // The binder threads increment the same counter when a new transaction containing a buffer is
    // added to the transaction queue. The map is updated with the layer handle lifecycle updates.
    // This is done to avoid lock contention with the main thread.
    class BufferCountTracker {
    public:
        void increment(BBinder* layerHandle) {
            std::lock_guard<std::mutex> lock(mLock);
            auto it = mCounterByLayerHandle.find(layerHandle);
            if (it != mCounterByLayerHandle.end()) {
                auto [name, pendingBuffers] = it->second;
                int32_t count = ++(*pendingBuffers);
                ATRACE_INT(name.c_str(), count);
            } else {
                ALOGW("Handle not found! %p", layerHandle);
            }
        }

        void add(BBinder* layerHandle, const std::string& name, std::atomic<int32_t>* counter) {
            std::lock_guard<std::mutex> lock(mLock);
            mCounterByLayerHandle[layerHandle] = std::make_pair(name, counter);
        }

        void remove(BBinder* layerHandle) {
            std::lock_guard<std::mutex> lock(mLock);
            mCounterByLayerHandle.erase(layerHandle);
        }

    private:
        std::mutex mLock;
        std::unordered_map<BBinder*, std::pair<std::string, std::atomic<int32_t>*>>
                mCounterByLayerHandle GUARDED_BY(mLock);
    };

    struct ActiveModeInfo {
        DisplayModeId modeId;
        Scheduler::ModeEvent event = Scheduler::ModeEvent::None;
@@ -764,7 +801,6 @@ private:
    void commitOffscreenLayers();
    bool transactionIsReadyToBeApplied(
            bool isAutoTimestamp, int64_t desiredPresentTime, const Vector<ComposerState>& states,
            bool updateTransactionCounters,
            std::unordered_set<sp<IBinder>, ISurfaceComposer::SpHash<IBinder>>& pendingBuffers)
            REQUIRES(mStateLock);
    uint32_t setDisplayStateLocked(const DisplayState& s) REQUIRES(mStateLock);
@@ -1314,6 +1350,8 @@ private:
    int mFrameRateFlexibilityTokenCount = 0;

    sp<IBinder> mDebugFrameRateFlexibilityToken;

    BufferCountTracker mBufferCountTracker;
};

} // namespace android