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

Commit 88660d77 authored by Pablo Gamito's avatar Pablo Gamito
Browse files

Stop reporting frame stats from frames completed before observer was attached

Test: Run app from bug report
Fixes: 195699687
Change-Id: If80825dfb41467917b7b9b1e8c9ead1a0dcbffae
parent ab1d302c
Loading
Loading
Loading
Loading
+1 −0
Original line number Diff line number Diff line
@@ -587,6 +587,7 @@ cc_defaults {
                "HardwareBitmapUploader.cpp",
                "HWUIProperties.sysprop",
                "JankTracker.cpp",
                "FrameMetricsReporter.cpp",
                "Layer.cpp",
                "LayerUpdateQueue.cpp",
                "ProfileData.cpp",
+23 −0
Original line number Diff line number Diff line
@@ -26,6 +26,13 @@ public:
    virtual void notify(const int64_t* buffer) = 0;
    bool waitForPresentTime() const { return mWaitForPresentTime; };

    void reportMetricsFrom(int64_t frameNumber, int32_t surfaceControlId) {
        mAttachedFrameNumber = frameNumber;
        mSurfaceControlId = surfaceControlId;
    };
    int64_t attachedFrameNumber() const { return mAttachedFrameNumber; };
    int32_t attachedSurfaceControlId() const { return mSurfaceControlId; };

    /**
     * Create a new metrics observer. An observer that watches present time gets notified at a
     * different time than the observer that doesn't.
@@ -42,6 +49,22 @@ public:

private:
    const bool mWaitForPresentTime;

    // The id of the surface control (mSurfaceControlGenerationId in CanvasContext)
    // for which the mAttachedFrameNumber applies to. We rely on this value being
    // an increasing counter. We will report metrics:
    // - for all frames if the frame comes from a surface with a surfaceControlId
    //   that is strictly greater than mSurfaceControlId.
    // - for all frames with a frame number greater than or equal to mAttachedFrameNumber
    //   if the frame comes from a surface with a surfaceControlId that is equal to the
    //   mSurfaceControlId.
    // We will never report metrics if the frame comes from a surface with a surfaceControlId
    // that is strictly smaller than mSurfaceControlId.
    int32_t mSurfaceControlId;

    // The frame number the metrics observer was attached on. Metrics will be sent from this frame
    // number (inclusive) onwards in the case that the surface id is equal to mSurfaceControlId.
    int64_t mAttachedFrameNumber;
};

}  // namespace uirenderer
+56 −0
Original line number Diff line number Diff line
/*
 * Copyright (C) 2021 The Android Open Source Project
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *      http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */

#include "FrameMetricsReporter.h"

namespace android {
namespace uirenderer {

void FrameMetricsReporter::reportFrameMetrics(const int64_t* stats, bool hasPresentTime,
                                              int64_t frameNumber, int32_t surfaceControlId) {
    FatVector<sp<FrameMetricsObserver>, 10> copy;
    {
        std::lock_guard lock(mObserversLock);
        copy.reserve(mObservers.size());
        for (size_t i = 0; i < mObservers.size(); i++) {
            auto observer = mObservers[i];

            if (CC_UNLIKELY(surfaceControlId < observer->attachedSurfaceControlId())) {
                // Don't notify if the metrics are from a frame that was run on an old
                // surface (one from before the observer was attached).
                ALOGV("skipped reporting metrics from old surface %d", surfaceControlId);
                continue;
            } else if (CC_UNLIKELY(surfaceControlId == observer->attachedSurfaceControlId() &&
                                   frameNumber < observer->attachedFrameNumber())) {
                // Don't notify if the metrics are from a frame that was queued by the
                // BufferQueueProducer on the render thread before the observer was attached.
                ALOGV("skipped reporting metrics from old frame %ld", (long)frameNumber);
                continue;
            }

            const bool wantsPresentTime = observer->waitForPresentTime();
            if (hasPresentTime == wantsPresentTime) {
                copy.push_back(observer);
            }
        }
    }
    for (size_t i = 0; i < copy.size(); i++) {
        copy[i]->notify(stats);
    }
}

}  // namespace uirenderer
}  // namespace android
+7 −16
Original line number Diff line number Diff line
@@ -63,23 +63,14 @@ public:
     * If an observer does not want present time, only notify when 'hasPresentTime' is false.
     * Never notify both types of observers from the same callback, because the callback with
     * 'hasPresentTime' is sent at a different time than the one without.
     *
     * The 'frameNumber' and 'surfaceControlId' associated to the frame whose's stats are being
     * reported are used to determine whether or not the stats should be reported. We won't report
     * stats of frames that are from "old" surfaces (i.e. with surfaceControlIds older than the one
     * the observer was attached on) nor those that are from "old" frame numbers.
     */
    void reportFrameMetrics(const int64_t* stats, bool hasPresentTime) {
        FatVector<sp<FrameMetricsObserver>, 10> copy;
        {
            std::lock_guard lock(mObserversLock);
            copy.reserve(mObservers.size());
            for (size_t i = 0; i < mObservers.size(); i++) {
                const bool wantsPresentTime = mObservers[i]->waitForPresentTime();
                if (hasPresentTime == wantsPresentTime) {
                    copy.push_back(mObservers[i]);
                }
            }
        }
        for (size_t i = 0; i < copy.size(); i++) {
            copy[i]->notify(stats);
        }
    }
    void reportFrameMetrics(const int64_t* stats, bool hasPresentTime, int64_t frameNumber,
                            int32_t surfaceControlId);

private:
    FatVector<sp<FrameMetricsObserver>, 10> mObservers GUARDED_BY(mObserversLock);
+4 −2
Original line number Diff line number Diff line
@@ -164,7 +164,8 @@ void JankTracker::calculateLegacyJank(FrameInfo& frame) REQUIRES(mDataMutex) {
            - lastFrameOffset + mFrameIntervalLegacy;
}

void JankTracker::finishFrame(FrameInfo& frame, std::unique_ptr<FrameMetricsReporter>& reporter) {
void JankTracker::finishFrame(FrameInfo& frame, std::unique_ptr<FrameMetricsReporter>& reporter,
                              int64_t frameNumber, int32_t surfaceControlId) {
    std::lock_guard lock(mDataMutex);

    calculateLegacyJank(frame);
@@ -253,7 +254,8 @@ void JankTracker::finishFrame(FrameInfo& frame, std::unique_ptr<FrameMetricsRepo
    }

    if (CC_UNLIKELY(reporter.get() != nullptr)) {
        reporter->reportFrameMetrics(frame.data(), false /* hasPresentTime */);
        reporter->reportFrameMetrics(frame.data(), false /* hasPresentTime */, frameNumber,
                                     surfaceControlId);
    }
}

Loading