diff options
28 files changed, 573 insertions, 61 deletions
diff --git a/libs/binder/BackendUnifiedServiceManager.cpp b/libs/binder/BackendUnifiedServiceManager.cpp index fee36da911..5680798d0d 100644 --- a/libs/binder/BackendUnifiedServiceManager.cpp +++ b/libs/binder/BackendUnifiedServiceManager.cpp @@ -24,11 +24,111 @@ namespace android { +#ifdef LIBBINDER_CLIENT_CACHE +constexpr bool kUseCache = true; +#else +constexpr bool kUseCache = false; +#endif + using AidlServiceManager = android::os::IServiceManager; using IAccessor = android::os::IAccessor; +static const char* kStaticCachableList[] = { + "activity", + "android.hardware.thermal.IThermal/default", + "android.hardware.power.IPower/default", + "android.frameworks.stats.IStats/default", + "android.system.suspend.ISystemSuspend/default", + "appops", + "audio", + "batterystats", + "carrier_config", + "connectivity", + "content_capture", + "device_policy", + "display", + "dropbox", + "econtroller", + "isub", + "legacy_permission", + "location", + "media.extractor", + "media.metrics", + "media.player", + "media.resource_manager", + "netd_listener", + "netstats", + "network_management", + "nfc", + "package_native", + "performance_hint", + "permission", + "permissionmgr", + "permission_checker", + "phone", + "platform_compat", + "power", + "role", + "sensorservice", + "statscompanion", + "telephony.registry", + "thermalservice", + "time_detector", + "trust", + "uimode", + "virtualdevice", + "virtualdevice_native", + "webviewupdate", +}; + +bool BinderCacheWithInvalidation::isClientSideCachingEnabled(const std::string& serviceName) { + if (ProcessState::self()->getThreadPoolMaxTotalThreadCount() <= 0) { + ALOGW("Thread Pool max thread count is 0. Cannot cache binder as linkToDeath cannot be " + "implemented. serviceName: %s", + serviceName.c_str()); + return false; + } + for (const char* name : kStaticCachableList) { + if (name == serviceName) { + return true; + } + } + return false; +} + +binder::Status BackendUnifiedServiceManager::updateCache(const std::string& serviceName, + const os::Service& service) { + if (!kUseCache) { + return binder::Status::ok(); + } + if (service.getTag() == os::Service::Tag::binder) { + sp<IBinder> binder = service.get<os::Service::Tag::binder>(); + if (binder && mCacheForGetService->isClientSideCachingEnabled(serviceName) && + binder->isBinderAlive()) { + return mCacheForGetService->setItem(serviceName, binder); + } + } + return binder::Status::ok(); +} + +bool BackendUnifiedServiceManager::returnIfCached(const std::string& serviceName, + os::Service* _out) { + if (!kUseCache) { + return false; + } + sp<IBinder> item = mCacheForGetService->getItem(serviceName); + // TODO(b/363177618): Enable caching for binders which are always null. + if (item != nullptr && item->isBinderAlive()) { + *_out = os::Service::make<os::Service::Tag::binder>(item); + return true; + } + return false; +} + BackendUnifiedServiceManager::BackendUnifiedServiceManager(const sp<AidlServiceManager>& impl) - : mTheRealServiceManager(impl) {} + : mTheRealServiceManager(impl) { + mCacheForGetService = std::make_shared<BinderCacheWithInvalidation>(); +} sp<AidlServiceManager> BackendUnifiedServiceManager::getImpl() { return mTheRealServiceManager; @@ -44,10 +144,17 @@ binder::Status BackendUnifiedServiceManager::getService(const ::std::string& nam binder::Status BackendUnifiedServiceManager::getService2(const ::std::string& name, os::Service* _out) { + if (returnIfCached(name, _out)) { + return binder::Status::ok(); + } os::Service service; binder::Status status = mTheRealServiceManager->getService2(name, &service); + if (status.isOk()) { - return toBinderService(name, service, _out); + status = toBinderService(name, service, _out); + if (status.isOk()) { + return updateCache(name, service); + } } return status; } @@ -55,9 +162,16 @@ binder::Status BackendUnifiedServiceManager::getService2(const ::std::string& na binder::Status BackendUnifiedServiceManager::checkService(const ::std::string& name, os::Service* _out) { os::Service service; + if (returnIfCached(name, _out)) { + return binder::Status::ok(); + } + binder::Status status = mTheRealServiceManager->checkService(name, &service); if (status.isOk()) { - return toBinderService(name, service, _out); + status = toBinderService(name, service, _out); + if (status.isOk()) { + return updateCache(name, service); + } } return status; } diff --git a/libs/binder/BackendUnifiedServiceManager.h b/libs/binder/BackendUnifiedServiceManager.h index 387eb35346..47b2ec9f2c 100644 --- a/libs/binder/BackendUnifiedServiceManager.h +++ b/libs/binder/BackendUnifiedServiceManager.h @@ -18,9 +18,87 @@ #include <android/os/BnServiceManager.h> #include <android/os/IServiceManager.h> #include <binder/IPCThreadState.h> +#include <map> +#include <memory> namespace android { +class BinderCacheWithInvalidation + : public std::enable_shared_from_this<BinderCacheWithInvalidation> { + class BinderInvalidation : public IBinder::DeathRecipient { + public: + BinderInvalidation(std::weak_ptr<BinderCacheWithInvalidation> cache, const std::string& key) + : mCache(cache), mKey(key) {} + + void binderDied(const wp<IBinder>& who) override { + sp<IBinder> binder = who.promote(); + if (std::shared_ptr<BinderCacheWithInvalidation> cache = mCache.lock()) { + cache->removeItem(mKey, binder); + } else { + ALOGI("Binder Cache pointer expired: %s", mKey.c_str()); + } + } + + private: + std::weak_ptr<BinderCacheWithInvalidation> mCache; + std::string mKey; + }; + struct Entry { + sp<IBinder> service; + sp<BinderInvalidation> deathRecipient; + }; + +public: + sp<IBinder> getItem(const std::string& key) const { + std::lock_guard<std::mutex> lock(mCacheMutex); + + if (auto it = mCache.find(key); it != mCache.end()) { + return it->second.service; + } + return nullptr; + } + + bool removeItem(const std::string& key, const sp<IBinder>& who) { + std::lock_guard<std::mutex> lock(mCacheMutex); + if (auto it = mCache.find(key); it != mCache.end()) { + if (it->second.service == who) { + status_t result = who->unlinkToDeath(it->second.deathRecipient); + if (result != DEAD_OBJECT) { + ALOGW("Unlinking to dead binder resulted in: %d", result); + } + mCache.erase(key); + return true; + } + } + return false; + } + + binder::Status setItem(const std::string& key, const sp<IBinder>& item) { + sp<BinderInvalidation> deathRecipient = + sp<BinderInvalidation>::make(shared_from_this(), key); + + // linkToDeath if binder is a remote binder. + if (item->localBinder() == nullptr) { + status_t status = item->linkToDeath(deathRecipient); + if (status != android::OK) { + ALOGE("Failed to linkToDeath binder for service %s. Error: %d", key.c_str(), + status); + return binder::Status::fromStatusT(status); + } + } + std::lock_guard<std::mutex> lock(mCacheMutex); + Entry entry = {.service = item, .deathRecipient = deathRecipient}; + mCache[key] = entry; + return binder::Status::ok(); + } + + bool isClientSideCachingEnabled(const std::string& serviceName); + +private: + std::map<std::string, Entry> mCache; + mutable std::mutex mCacheMutex; +}; + class BackendUnifiedServiceManager : public android::os::BnServiceManager { public: explicit BackendUnifiedServiceManager(const sp<os::IServiceManager>& impl); @@ -58,9 +136,12 @@ public: } private: + std::shared_ptr<BinderCacheWithInvalidation> mCacheForGetService; sp<os::IServiceManager> mTheRealServiceManager; binder::Status toBinderService(const ::std::string& name, const os::Service& in, os::Service* _out); + binder::Status updateCache(const std::string& serviceName, const os::Service& service); + bool returnIfCached(const std::string& serviceName, os::Service* _out); }; sp<BackendUnifiedServiceManager> getBackendUnifiedServiceManager(); diff --git a/libs/binder/IServiceManager.cpp b/libs/binder/IServiceManager.cpp index cba21b2094..88761d772f 100644 --- a/libs/binder/IServiceManager.cpp +++ b/libs/binder/IServiceManager.cpp @@ -18,6 +18,7 @@ #define LOG_TAG "ServiceManagerCppClient" #include <binder/IServiceManager.h> +#include <binder/IServiceManagerUnitTestHelper.h> #include "BackendUnifiedServiceManager.h" #include <inttypes.h> @@ -156,7 +157,7 @@ protected: class AccessorProvider { public: - AccessorProvider(RpcAccessorProvider&& provider) : mProvider(provider) {} + AccessorProvider(RpcAccessorProvider&& provider) : mProvider(std::move(provider)) {} sp<IBinder> provide(const String16& name) { return mProvider(name); } private: @@ -167,7 +168,8 @@ private: class AccessorProviderEntry { public: - AccessorProviderEntry(std::shared_ptr<AccessorProvider>&& provider) : mProvider(provider) {} + AccessorProviderEntry(std::shared_ptr<AccessorProvider>&& provider) + : mProvider(std::move(provider)) {} std::shared_ptr<AccessorProvider> mProvider; private: @@ -182,7 +184,7 @@ private: class LocalAccessor : public android::os::BnAccessor { public: LocalAccessor(const String16& instance, RpcSocketAddressProvider&& connectionInfoProvider) - : mInstance(instance), mConnectionInfoProvider(connectionInfoProvider) { + : mInstance(instance), mConnectionInfoProvider(std::move(connectionInfoProvider)) { LOG_ALWAYS_FATAL_IF(!mConnectionInfoProvider, "LocalAccessor object needs a valid connection info provider"); } @@ -311,13 +313,19 @@ void setDefaultServiceManager(const sp<IServiceManager>& sm) { } } +sp<IServiceManager> getServiceManagerShimFromAidlServiceManagerForTests( + const sp<AidlServiceManager>& sm) { + return sp<CppBackendShim>::make(sp<BackendUnifiedServiceManager>::make(sm)); +} + std::weak_ptr<AccessorProvider> addAccessorProvider(RpcAccessorProvider&& providerCallback) { std::lock_guard<std::mutex> lock(gAccessorProvidersMutex); std::shared_ptr<AccessorProvider> provider = std::make_shared<AccessorProvider>(std::move(providerCallback)); + std::weak_ptr<AccessorProvider> receipt = provider; gAccessorProviders.push_back(AccessorProviderEntry(std::move(provider))); - return provider; + return receipt; } status_t removeAccessorProvider(std::weak_ptr<AccessorProvider> wProvider) { diff --git a/libs/binder/TEST_MAPPING b/libs/binder/TEST_MAPPING index 125617363a..95a5da20ae 100644 --- a/libs/binder/TEST_MAPPING +++ b/libs/binder/TEST_MAPPING @@ -133,6 +133,9 @@ { "name": "binder_sdk_test", "host": true + }, + { + "name": "binderCacheUnitTest" } ], "imports": [ diff --git a/libs/binder/include/binder/IServiceManagerUnitTestHelper.h b/libs/binder/include/binder/IServiceManagerUnitTestHelper.h new file mode 100644 index 0000000000..ff25163ddc --- /dev/null +++ b/libs/binder/include/binder/IServiceManagerUnitTestHelper.h @@ -0,0 +1,29 @@ +/* + * Copyright (C) 2024 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. + */ + +#pragma once + +#include <android/os/IServiceManager.h> +#include "IServiceManager.h" +namespace android { + +/** + * Encapsulate an AidlServiceManager in a CppBackendShim. Only used for testing. + */ +LIBBINDER_EXPORTED sp<IServiceManager> getServiceManagerShimFromAidlServiceManagerForTests( + const sp<os::IServiceManager>& sm); + +} // namespace android
\ No newline at end of file diff --git a/libs/binder/rust/src/proxy.rs b/libs/binder/rust/src/proxy.rs index 340014aeaa..04f1517556 100644 --- a/libs/binder/rust/src/proxy.rs +++ b/libs/binder/rust/src/proxy.rs @@ -195,7 +195,7 @@ impl PartialOrd for SpIBinder { impl PartialEq for SpIBinder { fn eq(&self, other: &Self) -> bool { - ptr::eq(self.0.as_ptr(), other.0.as_ptr()) + self.cmp(other) == Ordering::Equal } } diff --git a/libs/binder/tests/Android.bp b/libs/binder/tests/Android.bp index 1e463a45cc..0e653af707 100644 --- a/libs/binder/tests/Android.bp +++ b/libs/binder/tests/Android.bp @@ -51,6 +51,30 @@ cc_test { ], } +cc_test { + name: "binderCacheUnitTest", + target: { + darwin: { + enabled: false, + }, + }, + srcs: [ + "binderCacheUnitTest.cpp", + ], + shared_libs: [ + "liblog", + "libbinder", + "libcutils", + "libutils", + ], + static_libs: [ + "libfakeservicemanager", + ], + defaults: ["libbinder_client_cache_flag"], + test_suites: ["general-tests"], + require_root: true, +} + // unit test only, which can run on host and doesn't use /dev/binder cc_test { name: "binderUnitTest", diff --git a/libs/binder/tests/binderCacheUnitTest.cpp b/libs/binder/tests/binderCacheUnitTest.cpp new file mode 100644 index 0000000000..92dab19a63 --- /dev/null +++ b/libs/binder/tests/binderCacheUnitTest.cpp @@ -0,0 +1,214 @@ +/* + * Copyright (C) 2024 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 <gtest/gtest.h> + +#include <android-base/logging.h> +#include <android/os/IServiceManager.h> +#include <binder/IBinder.h> +#include <binder/IPCThreadState.h> +#include <binder/IServiceManager.h> +#include <binder/IServiceManagerUnitTestHelper.h> +#include "fakeservicemanager/FakeServiceManager.h" + +#include <sys/prctl.h> +#include <thread> + +using namespace android; + +#ifdef LIBBINDER_CLIENT_CACHE +constexpr bool kUseLibbinderCache = true; +#else +constexpr bool kUseLibbinderCache = false; +#endif + +// A service name which is in the static list of cachable services +const String16 kCachedServiceName = String16("isub"); + +#define EXPECT_OK(status) \ + do { \ + binder::Status stat = (status); \ + EXPECT_TRUE(stat.isOk()) << stat; \ + } while (false) + +const String16 kServerName = String16("binderCacheUnitTest"); + +class FooBar : public BBinder { +public: + status_t onTransact(uint32_t, const Parcel&, Parcel*, uint32_t) { + // exit the server + std::thread([] { exit(EXIT_FAILURE); }).detach(); + return OK; + } + void killServer(sp<IBinder> binder) { + Parcel data, reply; + binder->transact(0, data, &reply, 0); + } +}; + +class MockAidlServiceManager : public os::IServiceManagerDefault { +public: + MockAidlServiceManager() : innerSm() {} + + binder::Status checkService(const ::std::string& name, os::Service* _out) override { + sp<IBinder> binder = innerSm.getService(String16(name.c_str())); + *_out = os::Service::make<os::Service::Tag::binder>(binder); + return binder::Status::ok(); + } + + binder::Status addService(const std::string& name, const sp<IBinder>& service, + bool allowIsolated, int32_t dumpPriority) override { + return binder::Status::fromStatusT( + innerSm.addService(String16(name.c_str()), service, allowIsolated, dumpPriority)); + } + + FakeServiceManager innerSm; +}; + +class LibbinderCacheTest : public ::testing::Test { +protected: + void SetUp() override { + sp<MockAidlServiceManager> sm = sp<MockAidlServiceManager>::make(); + mServiceManager = getServiceManagerShimFromAidlServiceManagerForTests(sm); + } + + void TearDown() override {} + +public: + void cacheAndConfirmCacheHit(const sp<IBinder>& binder1, const sp<IBinder>& binder2) { + // Add a service + EXPECT_EQ(OK, mServiceManager->addService(kCachedServiceName, binder1)); + // Get the service. This caches it. + sp<IBinder> result = mServiceManager->checkService(kCachedServiceName); + ASSERT_EQ(binder1, result); + + // Add the different binder and replace the service. + // The cache should still hold the original binder. + EXPECT_EQ(OK, mServiceManager->addService(kCachedServiceName, binder2)); + + result = mServiceManager->checkService(kCachedServiceName); + if (kUseLibbinderCache) { + // If cache is enabled, we should get the binder to Service Manager. + EXPECT_EQ(binder1, result); + } else { + // If cache is disabled, then we should get the newer binder + EXPECT_EQ(binder2, result); + } + } + + sp<android::IServiceManager> mServiceManager; +}; + +TEST_F(LibbinderCacheTest, AddLocalServiceAndConfirmCacheHit) { + sp<IBinder> binder1 = sp<BBinder>::make(); + sp<IBinder> binder2 = sp<BBinder>::make(); + + cacheAndConfirmCacheHit(binder1, binder2); +} + +TEST_F(LibbinderCacheTest, AddRemoteServiceAndConfirmCacheHit) { + sp<IBinder> binder1 = defaultServiceManager()->checkService(kServerName); + ASSERT_NE(binder1, nullptr); + sp<IBinder> binder2 = IInterface::asBinder(mServiceManager); + + cacheAndConfirmCacheHit(binder1, binder2); +} + +TEST_F(LibbinderCacheTest, RemoveFromCacheOnServerDeath) { + sp<IBinder> binder1 = defaultServiceManager()->checkService(kServerName); + FooBar foo = FooBar(); + + EXPECT_EQ(OK, mServiceManager->addService(kCachedServiceName, binder1)); + + // Check Service, this caches the binder + sp<IBinder> result = mServiceManager->checkService(kCachedServiceName); + ASSERT_EQ(binder1, result); + + // Kill the server, this should remove from cache. + foo.killServer(binder1); + pid_t pid; + ASSERT_EQ(OK, binder1->getDebugPid(&pid)); + system(("kill -9 " + std::to_string(pid)).c_str()); + + sp<IBinder> binder2 = sp<BBinder>::make(); + + // Add new service with the same name. + // This will replace the service in FakeServiceManager. + EXPECT_EQ(OK, mServiceManager->addService(kCachedServiceName, binder2)); + + // Confirm that new service is returned instead of old. + sp<IBinder> result2 = mServiceManager->checkService(kCachedServiceName); + ASSERT_EQ(binder2, result2); +} + +TEST_F(LibbinderCacheTest, NullBinderNotCached) { + sp<IBinder> binder1 = nullptr; + sp<IBinder> binder2 = sp<BBinder>::make(); + + // Check for a cacheble service which isn't registered. + // FakeServiceManager should return nullptr. + // This shouldn't be cached. + sp<IBinder> result = mServiceManager->checkService(kCachedServiceName); + ASSERT_EQ(binder1, result); + + // Add the same service + EXPECT_EQ(OK, mServiceManager->addService(kCachedServiceName, binder2)); + + // This should return the newly added service. + result = mServiceManager->checkService(kCachedServiceName); + EXPECT_EQ(binder2, result); +} + +TEST_F(LibbinderCacheTest, DoNotCacheServiceNotInList) { + sp<IBinder> binder1 = sp<BBinder>::make(); + sp<IBinder> binder2 = sp<BBinder>::make(); + String16 serviceName = String16("NewLibbinderCacheTest"); + // Add a service + EXPECT_EQ(OK, mServiceManager->addService(serviceName, binder1)); + // Get the service. This shouldn't caches it. + sp<IBinder> result = mServiceManager->checkService(serviceName); + ASSERT_EQ(binder1, result); + + // Add the different binder and replace the service. + EXPECT_EQ(OK, mServiceManager->addService(serviceName, binder2)); + + // Confirm that we get the new service + result = mServiceManager->checkService(serviceName); + EXPECT_EQ(binder2, result); +} + +int main(int argc, char** argv) { + ::testing::InitGoogleTest(&argc, argv); + + if (fork() == 0) { + prctl(PR_SET_PDEATHSIG, SIGHUP); + + // Start a FooBar service and add it to the servicemanager. + sp<IBinder> server = new FooBar(); + defaultServiceManager()->addService(kServerName, server); + + IPCThreadState::self()->joinThreadPool(true); + exit(1); // should not reach + } + + status_t err = ProcessState::self()->setThreadPoolMaxThreadCount(3); + ProcessState::self()->startThreadPool(); + CHECK_EQ(ProcessState::self()->isThreadPoolStarted(), true); + CHECK_GT(ProcessState::self()->getThreadPoolMaxTotalThreadCount(), 0); + + auto binder = defaultServiceManager()->waitForService(kServerName); + CHECK_NE(nullptr, binder.get()); + return RUN_ALL_TESTS(); +} diff --git a/libs/input/rust/lib.rs b/libs/input/rust/lib.rs index 9b6fe3c9ca..4f4ea8568b 100644 --- a/libs/input/rust/lib.rs +++ b/libs/input/rust/lib.rs @@ -31,6 +31,7 @@ pub use input_verifier::InputVerifier; pub use keyboard_classifier::KeyboardClassifier; #[cxx::bridge(namespace = "android::input")] +#[allow(clippy::needless_maybe_sized)] #[allow(unsafe_op_in_unsafe_fn)] mod ffi { #[namespace = "android"] diff --git a/libs/renderengine/skia/AutoBackendTexture.h b/libs/renderengine/skia/AutoBackendTexture.h index 74daf471fa..a570ad01a2 100644 --- a/libs/renderengine/skia/AutoBackendTexture.h +++ b/libs/renderengine/skia/AutoBackendTexture.h @@ -16,9 +16,9 @@ #pragma once -#include <GrDirectContext.h> #include <SkImage.h> #include <SkSurface.h> +#include <include/gpu/ganesh/GrDirectContext.h> #include <sys/types.h> #include <ui/GraphicTypes.h> diff --git a/libs/renderengine/skia/SkiaGLRenderEngine.cpp b/libs/renderengine/skia/SkiaGLRenderEngine.cpp index af24600ade..4ef7d5bccb 100644 --- a/libs/renderengine/skia/SkiaGLRenderEngine.cpp +++ b/libs/renderengine/skia/SkiaGLRenderEngine.cpp @@ -21,16 +21,15 @@ #include "SkiaGLRenderEngine.h" -#include "compat/SkiaGpuContext.h" - #include <EGL/egl.h> #include <EGL/eglext.h> -#include <GrContextOptions.h> -#include <GrTypes.h> #include <android-base/stringprintf.h> #include <common/trace.h> -#include <gl/GrGLInterface.h> +#include <include/gpu/ganesh/GrContextOptions.h> +#include <include/gpu/ganesh/GrTypes.h> #include <include/gpu/ganesh/gl/GrGLDirectContext.h> +#include <include/gpu/ganesh/gl/GrGLInterface.h> +#include <log/log_main.h> #include <sync/sync.h> #include <ui/DebugUtils.h> @@ -40,7 +39,7 @@ #include <numeric> #include "GLExtensions.h" -#include "log/log_main.h" +#include "compat/SkiaGpuContext.h" namespace android { namespace renderengine { diff --git a/libs/renderengine/skia/SkiaGLRenderEngine.h b/libs/renderengine/skia/SkiaGLRenderEngine.h index bd177e60ba..765103889e 100644 --- a/libs/renderengine/skia/SkiaGLRenderEngine.h +++ b/libs/renderengine/skia/SkiaGLRenderEngine.h @@ -20,9 +20,10 @@ #include <EGL/egl.h> #include <EGL/eglext.h> #include <GLES2/gl2.h> -#include <GrDirectContext.h> #include <SkSurface.h> #include <android-base/thread_annotations.h> +#include <include/gpu/ganesh/GrContextOptions.h> +#include <include/gpu/ganesh/GrDirectContext.h> #include <renderengine/ExternalTexture.h> #include <renderengine/RenderEngine.h> #include <sys/types.h> @@ -32,7 +33,6 @@ #include "AutoBackendTexture.h" #include "EGL/egl.h" -#include "GrContextOptions.h" #include "SkImageInfo.h" #include "SkiaRenderEngine.h" #include "android-base/macros.h" diff --git a/libs/renderengine/skia/SkiaRenderEngine.cpp b/libs/renderengine/skia/SkiaRenderEngine.cpp index d58f303e70..056e8fecc3 100644 --- a/libs/renderengine/skia/SkiaRenderEngine.cpp +++ b/libs/renderengine/skia/SkiaRenderEngine.cpp @@ -20,9 +20,6 @@ #include "SkiaRenderEngine.h" -#include <GrBackendSemaphore.h> -#include <GrContextOptions.h> -#include <GrTypes.h> #include <SkBlendMode.h> #include <SkCanvas.h> #include <SkColor.h> @@ -56,6 +53,9 @@ #include <common/FlagManager.h> #include <common/trace.h> #include <gui/FenceMonitor.h> +#include <include/gpu/ganesh/GrBackendSemaphore.h> +#include <include/gpu/ganesh/GrContextOptions.h> +#include <include/gpu/ganesh/GrTypes.h> #include <include/gpu/ganesh/SkSurfaceGanesh.h> #include <pthread.h> #include <src/core/SkTraceEventCommon.h> diff --git a/libs/renderengine/skia/SkiaRenderEngine.h b/libs/renderengine/skia/SkiaRenderEngine.h index 224a1cad92..721dbce0e8 100644 --- a/libs/renderengine/skia/SkiaRenderEngine.h +++ b/libs/renderengine/skia/SkiaRenderEngine.h @@ -18,11 +18,12 @@ #define SF_SKIARENDERENGINE_H_ #include <renderengine/RenderEngine.h> -#include <sys/types.h> -#include <GrBackendSemaphore.h> -#include <SkSurface.h> #include <android-base/thread_annotations.h> +#include <include/core/SkImageInfo.h> +#include <include/core/SkSurface.h> +#include <include/gpu/ganesh/GrBackendSemaphore.h> +#include <include/gpu/ganesh/GrContextOptions.h> #include <renderengine/ExternalTexture.h> #include <renderengine/RenderEngine.h> #include <sys/types.h> @@ -32,8 +33,6 @@ #include <unordered_map> #include "AutoBackendTexture.h" -#include "GrContextOptions.h" -#include "SkImageInfo.h" #include "android-base/macros.h" #include "compat/SkiaGpuContext.h" #include "debug/SkiaCapture.h" diff --git a/libs/renderengine/skia/SkiaVkRenderEngine.cpp b/libs/renderengine/skia/SkiaVkRenderEngine.cpp index d89e818cdc..677a2b63b2 100644 --- a/libs/renderengine/skia/SkiaVkRenderEngine.cpp +++ b/libs/renderengine/skia/SkiaVkRenderEngine.cpp @@ -24,12 +24,12 @@ #include "GaneshVkRenderEngine.h" #include "compat/SkiaGpuContext.h" -#include <GrBackendSemaphore.h> -#include <GrContextOptions.h> -#include <GrDirectContext.h> +#include <include/gpu/ganesh/GrBackendSemaphore.h> +#include <include/gpu/ganesh/GrContextOptions.h> +#include <include/gpu/ganesh/GrDirectContext.h> #include <include/gpu/ganesh/vk/GrVkBackendSemaphore.h> #include <include/gpu/ganesh/vk/GrVkDirectContext.h> -#include <vk/GrVkTypes.h> +#include <include/gpu/ganesh/vk/GrVkTypes.h> #include <android-base/stringprintf.h> #include <common/trace.h> diff --git a/libs/renderengine/skia/compat/GaneshBackendTexture.cpp b/libs/renderengine/skia/compat/GaneshBackendTexture.cpp index 3fbc6ca22e..88282e7f5a 100644 --- a/libs/renderengine/skia/compat/GaneshBackendTexture.cpp +++ b/libs/renderengine/skia/compat/GaneshBackendTexture.cpp @@ -21,12 +21,12 @@ #define ATRACE_TAG ATRACE_TAG_GRAPHICS #include <include/core/SkImage.h> -#include <include/gpu/GrDirectContext.h> +#include <include/gpu/ganesh/GrDirectContext.h> #include <include/gpu/ganesh/SkImageGanesh.h> #include <include/gpu/ganesh/SkSurfaceGanesh.h> #include <include/gpu/ganesh/gl/GrGLBackendSurface.h> #include <include/gpu/ganesh/vk/GrVkBackendSurface.h> -#include <include/gpu/vk/GrVkTypes.h> +#include <include/gpu/ganesh/vk/GrVkTypes.h> #include "skia/ColorSpaces.h" #include "skia/compat/SkiaBackendTexture.h" diff --git a/libs/renderengine/skia/compat/GaneshBackendTexture.h b/libs/renderengine/skia/compat/GaneshBackendTexture.h index 5cf8647801..4337df18d4 100644 --- a/libs/renderengine/skia/compat/GaneshBackendTexture.h +++ b/libs/renderengine/skia/compat/GaneshBackendTexture.h @@ -21,7 +21,7 @@ #include <include/android/GrAHardwareBufferUtils.h> #include <include/core/SkColorSpace.h> -#include <include/gpu/GrDirectContext.h> +#include <include/gpu/ganesh/GrDirectContext.h> #include <android-base/macros.h> diff --git a/libs/renderengine/skia/compat/GaneshGpuContext.cpp b/libs/renderengine/skia/compat/GaneshGpuContext.cpp index b121fe83eb..931f8433da 100644 --- a/libs/renderengine/skia/compat/GaneshGpuContext.cpp +++ b/libs/renderengine/skia/compat/GaneshGpuContext.cpp @@ -19,12 +19,12 @@ #include <include/core/SkImageInfo.h> #include <include/core/SkSurface.h> #include <include/core/SkTraceMemoryDump.h> -#include <include/gpu/GrDirectContext.h> -#include <include/gpu/GrTypes.h> +#include <include/gpu/ganesh/GrDirectContext.h> +#include <include/gpu/ganesh/GrTypes.h> #include <include/gpu/ganesh/SkSurfaceGanesh.h> #include <include/gpu/ganesh/gl/GrGLDirectContext.h> +#include <include/gpu/ganesh/gl/GrGLInterface.h> #include <include/gpu/ganesh/vk/GrVkDirectContext.h> -#include <include/gpu/gl/GrGLInterface.h> #include <include/gpu/vk/VulkanBackendContext.h> #include "../AutoBackendTexture.h" diff --git a/libs/renderengine/skia/compat/SkiaBackendTexture.h b/libs/renderengine/skia/compat/SkiaBackendTexture.h index 09877a5ede..fa12624ff7 100644 --- a/libs/renderengine/skia/compat/SkiaBackendTexture.h +++ b/libs/renderengine/skia/compat/SkiaBackendTexture.h @@ -18,7 +18,7 @@ #include <include/android/GrAHardwareBufferUtils.h> #include <include/core/SkColorSpace.h> -#include <include/gpu/GrDirectContext.h> +#include <include/gpu/ganesh/GrDirectContext.h> #include <android/hardware_buffer.h> #include <ui/GraphicTypes.h> diff --git a/libs/renderengine/skia/compat/SkiaGpuContext.h b/libs/renderengine/skia/compat/SkiaGpuContext.h index 9fa6fb8521..0bd8283987 100644 --- a/libs/renderengine/skia/compat/SkiaGpuContext.h +++ b/libs/renderengine/skia/compat/SkiaGpuContext.h @@ -20,10 +20,10 @@ #define LOG_TAG "RenderEngine" #include <include/core/SkSurface.h> -#include <include/gpu/GrDirectContext.h> -#include <include/gpu/gl/GrGLInterface.h> +#include <include/gpu/ganesh/GrDirectContext.h> +#include <include/gpu/ganesh/gl/GrGLInterface.h> #include <include/gpu/graphite/Context.h> -#include "include/gpu/vk/VulkanBackendContext.h" +#include <include/gpu/vk/VulkanBackendContext.h> #include "SkiaBackendTexture.h" diff --git a/services/sensorservice/SensorService.cpp b/services/sensorservice/SensorService.cpp index 3895ffe55e..57b473bd37 100644 --- a/services/sensorservice/SensorService.cpp +++ b/services/sensorservice/SensorService.cpp @@ -1583,7 +1583,11 @@ sp<ISensorEventConnection> SensorService::createSensorEventConnection(const Stri // Only 4 modes supported for a SensorEventConnection ... NORMAL, DATA_INJECTION, // REPLAY_DATA_INJECTION and HAL_BYPASS_REPLAY_DATA_INJECTION if (requestedMode != NORMAL && !isInjectionMode(requestedMode)) { - return nullptr; + ALOGE( + "Failed to create sensor event connection: invalid request mode. " + "requestMode: %d", + requestedMode); + return nullptr; } resetTargetSdkVersionCache(opPackageName); @@ -1591,8 +1595,19 @@ sp<ISensorEventConnection> SensorService::createSensorEventConnection(const Stri // To create a client in DATA_INJECTION mode to inject data, SensorService should already be // operating in DI mode. if (requestedMode == DATA_INJECTION) { - if (mCurrentOperatingMode != DATA_INJECTION) return nullptr; - if (!isAllowListedPackage(packageName)) return nullptr; + if (mCurrentOperatingMode != DATA_INJECTION) { + ALOGE( + "Failed to create sensor event connection: sensor service not in " + "DI mode when creating a client in DATA_INJECTION mode"); + return nullptr; + } + if (!isAllowListedPackage(packageName)) { + ALOGE( + "Failed to create sensor event connection: package %s not in " + "allowed list for DATA_INJECTION mode", + packageName.c_str()); + return nullptr; + } } uid_t uid = IPCThreadState::self()->getCallingUid(); diff --git a/services/surfaceflinger/Scheduler/EventThread.cpp b/services/surfaceflinger/Scheduler/EventThread.cpp index d31fceab7e..218c56ef3d 100644 --- a/services/surfaceflinger/Scheduler/EventThread.cpp +++ b/services/surfaceflinger/Scheduler/EventThread.cpp @@ -320,7 +320,8 @@ void EventThread::setDuration(std::chrono::nanoseconds workDuration, mVsyncRegistration.update({.workDuration = mWorkDuration.get().count(), .readyDuration = mReadyDuration.count(), - .lastVsync = mLastVsyncCallbackTime.ns()}); + .lastVsync = mLastVsyncCallbackTime.ns(), + .committedVsyncOpt = mLastCommittedVsyncTime.ns()}); } sp<EventThreadConnection> EventThread::createEventConnection( @@ -527,10 +528,11 @@ void EventThread::threadMain(std::unique_lock<std::mutex>& lock) { } if (mState == State::VSync) { - const auto scheduleResult = - mVsyncRegistration.schedule({.workDuration = mWorkDuration.get().count(), - .readyDuration = mReadyDuration.count(), - .lastVsync = mLastVsyncCallbackTime.ns()}); + const auto scheduleResult = mVsyncRegistration.schedule( + {.workDuration = mWorkDuration.get().count(), + .readyDuration = mReadyDuration.count(), + .lastVsync = mLastVsyncCallbackTime.ns(), + .committedVsyncOpt = mLastCommittedVsyncTime.ns()}); LOG_ALWAYS_FATAL_IF(!scheduleResult, "Error scheduling callback"); } else { mVsyncRegistration.cancel(); @@ -725,8 +727,9 @@ void EventThread::dispatchEvent(const DisplayEventReceiver::Event& event, } if (event.header.type == DisplayEventReceiver::DISPLAY_EVENT_VSYNC && FlagManager::getInstance().vrr_config()) { - mCallback.onExpectedPresentTimePosted( - TimePoint::fromNs(event.vsync.vsyncData.preferredExpectedPresentationTime())); + mLastCommittedVsyncTime = + TimePoint::fromNs(event.vsync.vsyncData.preferredExpectedPresentationTime()); + mCallback.onExpectedPresentTimePosted(mLastCommittedVsyncTime); } } @@ -744,9 +747,12 @@ void EventThread::dump(std::string& result) const { const auto relativeLastCallTime = ticks<std::milli, float>(mLastVsyncCallbackTime - TimePoint::now()); + const auto relativeLastCommittedTime = + ticks<std::milli, float>(mLastCommittedVsyncTime - TimePoint::now()); StringAppendF(&result, "mWorkDuration=%.2f mReadyDuration=%.2f last vsync time ", mWorkDuration.get().count() / 1e6f, mReadyDuration.count() / 1e6f); StringAppendF(&result, "%.2fms relative to now\n", relativeLastCallTime); + StringAppendF(&result, " with vsync committed at %.2fms", relativeLastCommittedTime); StringAppendF(&result, " pending events (count=%zu):\n", mPendingEvents.size()); for (const auto& event : mPendingEvents) { @@ -794,7 +800,8 @@ scheduler::VSyncCallbackRegistration EventThread::onNewVsyncScheduleInternal( if (reschedule) { mVsyncRegistration.schedule({.workDuration = mWorkDuration.get().count(), .readyDuration = mReadyDuration.count(), - .lastVsync = mLastVsyncCallbackTime.ns()}); + .lastVsync = mLastVsyncCallbackTime.ns(), + .committedVsyncOpt = mLastCommittedVsyncTime.ns()}); } return oldRegistration; } diff --git a/services/surfaceflinger/Scheduler/EventThread.h b/services/surfaceflinger/Scheduler/EventThread.h index f772126349..bbe4f9d899 100644 --- a/services/surfaceflinger/Scheduler/EventThread.h +++ b/services/surfaceflinger/Scheduler/EventThread.h @@ -220,6 +220,7 @@ private: std::chrono::nanoseconds mReadyDuration GUARDED_BY(mMutex); std::shared_ptr<scheduler::VsyncSchedule> mVsyncSchedule GUARDED_BY(mMutex); TimePoint mLastVsyncCallbackTime GUARDED_BY(mMutex) = TimePoint::now(); + TimePoint mLastCommittedVsyncTime GUARDED_BY(mMutex) = TimePoint::now(); scheduler::VSyncCallbackRegistration mVsyncRegistration GUARDED_BY(mMutex); frametimeline::TokenManager* const mTokenManager; diff --git a/services/surfaceflinger/Scheduler/VSyncDispatch.h b/services/surfaceflinger/Scheduler/VSyncDispatch.h index 0c43ffbc04..8993c38d37 100644 --- a/services/surfaceflinger/Scheduler/VSyncDispatch.h +++ b/services/surfaceflinger/Scheduler/VSyncDispatch.h @@ -93,6 +93,8 @@ public: * readyDuration will typically be 0. * @lastVsync: The targeted display time. This will be snapped to the closest * predicted vsync time after lastVsync. + * @committedVsyncOpt: The display time that is committed to the callback as the + * target vsync time. * * callback will be dispatched at 'workDuration + readyDuration' nanoseconds before a vsync * event. @@ -101,10 +103,11 @@ public: nsecs_t workDuration = 0; nsecs_t readyDuration = 0; nsecs_t lastVsync = 0; + std::optional<nsecs_t> committedVsyncOpt; bool operator==(const ScheduleTiming& other) const { return workDuration == other.workDuration && readyDuration == other.readyDuration && - lastVsync == other.lastVsync; + lastVsync == other.lastVsync && committedVsyncOpt == other.committedVsyncOpt; } bool operator!=(const ScheduleTiming& other) const { return !(*this == other); } diff --git a/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.cpp b/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.cpp index 900bce0aa9..1925f1165c 100644 --- a/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.cpp +++ b/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.cpp @@ -103,7 +103,8 @@ ScheduleResult VSyncDispatchTimerQueueEntry::schedule(VSyncDispatch::ScheduleTim tracker.nextAnticipatedVSyncTimeFrom(std::max(timing.lastVsync, now + timing.workDuration + timing.readyDuration), - timing.lastVsync); + timing.committedVsyncOpt.value_or( + timing.lastVsync)); auto nextWakeupTime = nextVsyncTime - timing.workDuration - timing.readyDuration; bool const wouldSkipAVsyncTarget = @@ -208,9 +209,12 @@ void VSyncDispatchTimerQueueEntry::update(VSyncTracker& tracker, nsecs_t now) { const auto workDelta = mWorkloadUpdateInfo->workDuration - mScheduleTiming.workDuration; const auto readyDelta = mWorkloadUpdateInfo->readyDuration - mScheduleTiming.readyDuration; const auto lastVsyncDelta = mWorkloadUpdateInfo->lastVsync - mScheduleTiming.lastVsync; + const auto lastCommittedVsyncDelta = + mWorkloadUpdateInfo->committedVsyncOpt.value_or(mWorkloadUpdateInfo->lastVsync) - + mScheduleTiming.committedVsyncOpt.value_or(mScheduleTiming.lastVsync); SFTRACE_FORMAT_INSTANT("Workload updated workDelta=%" PRId64 " readyDelta=%" PRId64 - " lastVsyncDelta=%" PRId64, - workDelta, readyDelta, lastVsyncDelta); + " lastVsyncDelta=%" PRId64 " committedVsyncDelta=%" PRId64, + workDelta, readyDelta, lastVsyncDelta, lastCommittedVsyncDelta); mScheduleTiming = *mWorkloadUpdateInfo; mWorkloadUpdateInfo.reset(); } @@ -261,10 +265,14 @@ void VSyncDispatchTimerQueueEntry::dump(std::string& result) const { StringAppendF(&result, "\t\t%s: %s %s\n", mName.c_str(), mRunning ? "(in callback function)" : "", armedInfo.c_str()); StringAppendF(&result, - "\t\t\tworkDuration: %.2fms readyDuration: %.2fms lastVsync: %.2fms relative " - "to now\n", + "\t\t\tworkDuration: %.2fms readyDuration: %.2fms " + "lastVsync: %.2fms relative to now " + "committedVsync: %.2fms relative to now\n", mScheduleTiming.workDuration / 1e6f, mScheduleTiming.readyDuration / 1e6f, - (mScheduleTiming.lastVsync - systemTime()) / 1e6f); + (mScheduleTiming.lastVsync - systemTime()) / 1e6f, + (mScheduleTiming.committedVsyncOpt.value_or(mScheduleTiming.lastVsync) - + systemTime()) / + 1e6f); if (mLastDispatchTime) { StringAppendF(&result, "\t\t\tmLastDispatchTime: %.2fms ago\n", diff --git a/services/surfaceflinger/Scheduler/VSyncPredictor.cpp b/services/surfaceflinger/Scheduler/VSyncPredictor.cpp index 4a7cff58dd..6e36f02463 100644 --- a/services/surfaceflinger/Scheduler/VSyncPredictor.cpp +++ b/services/surfaceflinger/Scheduler/VSyncPredictor.cpp @@ -695,9 +695,12 @@ std::optional<TimePoint> VSyncPredictor::VsyncTimeline::nextAnticipatedVSyncTime if (lastFrameMissed) { // If the last frame missed is the last vsync, we already shifted the timeline. Depends // on whether we skipped the frame (onFrameMissed) or not (onFrameBegin) we apply a - // different fixup. There is no need to to shift the vsync timeline again. - vsyncTime += missedVsync.fixup.ns(); - SFTRACE_FORMAT_INSTANT("lastFrameMissed"); + // different fixup if we are violating the minFramePeriod. + // There is no need to shift the vsync timeline again. + if (vsyncTime - missedVsync.vsync.ns() < minFramePeriodOpt->ns()) { + vsyncTime += missedVsync.fixup.ns(); + SFTRACE_FORMAT_INSTANT("lastFrameMissed"); + } } else if (mightBackpressure && lastVsyncOpt) { if (!FlagManager::getInstance().vrr_bugfix_24q4()) { // lastVsyncOpt does not need to be corrected with the new rate, and diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index f4bff8f972..e13c74fe69 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -4231,6 +4231,8 @@ void SurfaceFlinger::scheduleNotifyExpectedPresentHint(PhysicalDisplayId display if (data.hintStatus.compare_exchange_strong(scheduleHintOnTx, NotifyExpectedPresentHintStatus::Sent)) { sendHint(); + constexpr bool kAllowToEnable = true; + mScheduler->resyncToHardwareVsync(displayId, kAllowToEnable); } })); } diff --git a/services/surfaceflinger/tests/unittests/VSyncPredictorTest.cpp b/services/surfaceflinger/tests/unittests/VSyncPredictorTest.cpp index 7c678bd811..918107d270 100644 --- a/services/surfaceflinger/tests/unittests/VSyncPredictorTest.cpp +++ b/services/surfaceflinger/tests/unittests/VSyncPredictorTest.cpp @@ -915,7 +915,8 @@ TEST_F(VSyncPredictorTest, adjustsVrrTimeline) { vrrTracker.onFrameBegin(TimePoint::fromNs(7000), {TimePoint::fromNs(6500), TimePoint::fromNs(6500)}); - EXPECT_EQ(10500, vrrTracker.nextAnticipatedVSyncTimeFrom(9000, 7000)); + EXPECT_EQ(8500, vrrTracker.nextAnticipatedVSyncTimeFrom(8000, 7000)); + EXPECT_EQ(9500, vrrTracker.nextAnticipatedVSyncTimeFrom(9000, 7000)); } TEST_F(VSyncPredictorTest, adjustsVrrTimelineTwoClients) { |