SF: increase precision in VSyncPredictor
An issue was observed where the sf-app timeline woke up 1ms later than
it should have. Issue was tracked down to an imprecision in some of the
math in VSyncPredictor, which this issue corrects.
Fixes: 151146131
Test: 2 new unit tests, one of which was from bug
Test: flash dogfood device with patch
Change-Id: I1994b5b532e292a140d4736c7090b68224f75a02
diff --git a/services/surfaceflinger/Scheduler/VSyncPredictor.cpp b/services/surfaceflinger/Scheduler/VSyncPredictor.cpp
index 399da19..257b8b1 100644
--- a/services/surfaceflinger/Scheduler/VSyncPredictor.cpp
+++ b/services/surfaceflinger/Scheduler/VSyncPredictor.cpp
@@ -115,10 +115,10 @@
auto it = mRateMap.find(mIdealPeriod);
auto const currentPeriod = std::get<0>(it->second);
// TODO (b/144707443): its important that there's some precision in the mean of the ordinals
- // for the intercept calculation, so scale the ordinals by 10 to continue
+ // for the intercept calculation, so scale the ordinals by 1000 to continue
// fixed point calculation. Explore expanding
// scheduler::utils::calculate_mean to have a fixed point fractional part.
- static constexpr int kScalingFactor = 10;
+ static constexpr int64_t kScalingFactor = 1000;
for (auto i = 0u; i < mTimestamps.size(); i++) {
traceInt64If("VSP-ts", mTimestamps[i]);
@@ -147,7 +147,7 @@
return false;
}
- nsecs_t const anticipatedPeriod = top / bottom * kScalingFactor;
+ nsecs_t const anticipatedPeriod = top * kScalingFactor / bottom;
nsecs_t const intercept = meanTS - (anticipatedPeriod * meanOrdinal / kScalingFactor);
auto const percent = std::abs(anticipatedPeriod - mIdealPeriod) * kMaxPercent / mIdealPeriod;
diff --git a/services/surfaceflinger/tests/unittests/VSyncPredictorTest.cpp b/services/surfaceflinger/tests/unittests/VSyncPredictorTest.cpp
index f834af8..bf2a889 100644
--- a/services/surfaceflinger/tests/unittests/VSyncPredictorTest.cpp
+++ b/services/surfaceflinger/tests/unittests/VSyncPredictorTest.cpp
@@ -204,7 +204,7 @@
};
auto idealPeriod = 2000000;
auto expectedPeriod = 1999892;
- auto expectedIntercept = 175409;
+ auto expectedIntercept = 86342;
tracker.setPeriod(idealPeriod);
for (auto const& timestamp : simulatedVsyncs) {
@@ -335,8 +335,8 @@
158929706370359,
};
auto const idealPeriod = 11111111;
- auto const expectedPeriod = 11113500;
- auto const expectedIntercept = -395335;
+ auto const expectedPeriod = 11113919;
+ auto const expectedIntercept = -1195945;
tracker.setPeriod(idealPeriod);
for (auto const& timestamp : simulatedVsyncs) {
@@ -355,6 +355,32 @@
EXPECT_THAT(prediction, Ge(timePoint));
}
+// See b/151146131
+TEST_F(VSyncPredictorTest, hasEnoughPrecision) {
+ VSyncPredictor tracker{mPeriod, 20, kMinimumSamplesForPrediction, kOutlierTolerancePercent};
+ std::vector<nsecs_t> const simulatedVsyncs{840873348817, 840890049444, 840906762675,
+ 840923581635, 840940161584, 840956868096,
+ 840973702473, 840990256277, 841007116851,
+ 841023722530, 841040452167, 841057073002,
+ 841073800920, 841090474360, 841107278632,
+ 841123898634, 841140750875, 841157287127,
+ 841591357014, 840856664232
+
+ };
+ auto const idealPeriod = 16666666;
+ auto const expectedPeriod = 16698426;
+ auto const expectedIntercept = 58055;
+
+ tracker.setPeriod(idealPeriod);
+ for (auto const& timestamp : simulatedVsyncs) {
+ tracker.addVsyncTimestamp(timestamp);
+ }
+
+ auto [slope, intercept] = tracker.getVSyncPredictionModel();
+ EXPECT_THAT(slope, IsCloseTo(expectedPeriod, mMaxRoundingError));
+ EXPECT_THAT(intercept, IsCloseTo(expectedIntercept, mMaxRoundingError));
+}
+
TEST_F(VSyncPredictorTest, resetsWhenInstructed) {
auto const idealPeriod = 10000;
auto const realPeriod = 10500;
@@ -390,6 +416,22 @@
}
}
+constexpr nsecs_t operator""_years(unsigned long long years) noexcept {
+ using namespace std::chrono_literals;
+ return years * 365 * 24 * 3600 *
+ std::chrono::duration_cast<std::chrono::nanoseconds>(1s).count();
+}
+TEST_F(VSyncPredictorTest, aPhoneThatHasBeenAroundAWhileCanStillComputePeriod) {
+ constexpr nsecs_t timeBase = 100_years;
+
+ for (auto i = 0; i < kHistorySize; i++) {
+ tracker.addVsyncTimestamp(timeBase + i * mPeriod);
+ }
+ auto [slope, intercept] = tracker.getVSyncPredictionModel();
+ EXPECT_THAT(slope, IsCloseTo(mPeriod, mMaxRoundingError));
+ EXPECT_THAT(intercept, Eq(0));
+}
+
} // namespace android::scheduler
// TODO(b/129481165): remove the #pragma below and fix conversion issues