From 890ceb5e5bcc76381376e280f2247f6807edcabd Mon Sep 17 00:00:00 2001 From: Tim Murray Date: Thu, 30 Jan 2020 10:12:40 -0800 Subject: [PATCH] Add caching for getDisplayInfo Cache DisplayInfo objects on clients to avoid unnecessary binder traffic. Test: boots, works, 60->90->60 works Bug: 140788621 Change-Id: I0dec0f6e293f1345fbe0a30c4a6e39305c94276d --- .../display/DisplayManagerGlobal.java | 81 +++++++++++++------ .../server/display/DisplayInfoProxy.java | 57 +++++++++++++ .../server/display/DisplayManagerService.java | 16 +++- .../server/display/LogicalDisplay.java | 49 +++++------ 4 files changed, 153 insertions(+), 50 deletions(-) create mode 100644 services/core/java/com/android/server/display/DisplayInfoProxy.java diff --git a/core/java/android/hardware/display/DisplayManagerGlobal.java b/core/java/android/hardware/display/DisplayManagerGlobal.java index 9d92c894f3f6d..526db85b47d4d 100644 --- a/core/java/android/hardware/display/DisplayManagerGlobal.java +++ b/core/java/android/hardware/display/DisplayManagerGlobal.java @@ -18,6 +18,7 @@ package android.hardware.display; import android.annotation.NonNull; import android.annotation.Nullable; +import android.app.PropertyInvalidatedCache; import android.compat.annotation.UnsupportedAppUsage; import android.content.Context; import android.content.pm.ParceledListSlice; @@ -99,6 +100,20 @@ public final class DisplayManagerGlobal { } } + private PropertyInvalidatedCache mDisplayCache = + new PropertyInvalidatedCache( + 8, // size of display cache + CACHE_KEY_DISPLAY_INFO_PROPERTY) { + @Override + protected DisplayInfo recompute(Integer id) { + try { + return mDm.getDisplayInfo(id); + } catch (RemoteException ex) { + throw ex.rethrowFromSystemServer(); + } + } + }; + /** * Gets an instance of the display manager global singleton. * @@ -127,33 +142,27 @@ public final class DisplayManagerGlobal { */ @UnsupportedAppUsage public DisplayInfo getDisplayInfo(int displayId) { - try { - synchronized (mLock) { - DisplayInfo info; - if (USE_CACHE) { - info = mDisplayInfoCache.get(displayId); - if (info != null) { - return info; - } + synchronized (mLock) { + DisplayInfo info = null; + if (mDisplayCache != null) { + info = mDisplayCache.query(displayId); + } else { + try { + info = mDm.getDisplayInfo(displayId); + } catch (RemoteException ex) { + ex.rethrowFromSystemServer(); } - - info = mDm.getDisplayInfo(displayId); - if (info == null) { - return null; - } - - if (USE_CACHE) { - mDisplayInfoCache.put(displayId, info); - } - registerCallbackIfNeededLocked(); - - if (DEBUG) { - Log.d(TAG, "getDisplayInfo: displayId=" + displayId + ", info=" + info); - } - return info; } - } catch (RemoteException ex) { - throw ex.rethrowFromSystemServer(); + if (info == null) { + return null; + } + + registerCallbackIfNeededLocked(); + + if (DEBUG) { + Log.d(TAG, "getDisplayInfo: displayId=" + displayId + ", info=" + info); + } + return info; } } @@ -777,4 +786,26 @@ public final class DisplayManagerGlobal { } } } + + /** + * Name of the property containing a unique token which changes every time we update the + * system's display configuration. + */ + public static final String CACHE_KEY_DISPLAY_INFO_PROPERTY = + "cache_key.display_info"; + + /** + * Invalidates the contents of the display info cache for all applications. Can only + * be called by system_server. + */ + public static void invalidateLocalDisplayInfoCaches() { + PropertyInvalidatedCache.invalidateCache(CACHE_KEY_DISPLAY_INFO_PROPERTY); + } + + /** + * Disables the binder call cache. + */ + public void disableLocalDisplayInfoCaches() { + mDisplayCache = null; + } } diff --git a/services/core/java/com/android/server/display/DisplayInfoProxy.java b/services/core/java/com/android/server/display/DisplayInfoProxy.java new file mode 100644 index 0000000000000..2854495bc8f18 --- /dev/null +++ b/services/core/java/com/android/server/display/DisplayInfoProxy.java @@ -0,0 +1,57 @@ +/* + * Copyright (C) 2020 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. + */ + +package com.android.server.display; + +import android.annotation.Nullable; +import android.hardware.display.DisplayManagerGlobal; +import android.view.DisplayInfo; + +/** + * Class for wrapping access of DisplayInfo objects by LogicalDisplay so that we can appropriately + * invalidate caches when they change. + */ +public class DisplayInfoProxy { + private DisplayInfo mInfo; + + public DisplayInfoProxy(@Nullable DisplayInfo info) { + mInfo = info; + } + + /** + * Set the current {@link DisplayInfo}. + * + * The also automatically invalidates the display info caches across the entire system. + * @param info the new {@link DisplayInfo}. + */ + public void set(@Nullable DisplayInfo info) { + mInfo = info; + DisplayManagerGlobal.invalidateLocalDisplayInfoCaches(); + } + + /** + * Returns the current {@link DisplayInfo}. + * + * This info must be treated as immutable. Modifying the returned object is undefined + * behavior that will result in inconsistent states across the system. + * + * @return the current {@link DisplayInfo} + */ + @Nullable + public DisplayInfo get() { + return mInfo; + } +} diff --git a/services/core/java/com/android/server/display/DisplayManagerService.java b/services/core/java/com/android/server/display/DisplayManagerService.java index 1ff8a94002763..a23205124f74f 100644 --- a/services/core/java/com/android/server/display/DisplayManagerService.java +++ b/services/core/java/com/android/server/display/DisplayManagerService.java @@ -215,6 +215,7 @@ public final class DisplayManagerService extends SystemService { private final ArrayList mDisplayDevices = new ArrayList(); // List of all logical displays indexed by logical display id. + // Any modification to mLogicalDisplays must invalidate the DisplayManagerGlobal cache. private final SparseArray mLogicalDisplays = new SparseArray(); private int mNextNonDefaultDisplayId = Display.DEFAULT_DISPLAY + 1; @@ -373,6 +374,10 @@ public final class DisplayManagerService extends SystemService { } mHandler.sendEmptyMessage(MSG_REGISTER_DEFAULT_DISPLAY_ADAPTERS); + // If there was a runtime restart then we may have stale caches left around, so we need to + // make sure to invalidate them upon every start. + DisplayManagerGlobal.invalidateLocalDisplayInfoCaches(); + publishBinderService(Context.DISPLAY_SERVICE, new BinderService(), true /*allowIsolated*/); publishLocalService(DisplayManagerInternal.class, new LocalService()); @@ -1005,9 +1010,17 @@ public final class DisplayManagerService extends SystemService { if (displayId == Display.DEFAULT_DISPLAY) { recordTopInsetLocked(display); } + // We don't bother invalidating the display info caches here because any changes to the + // display info will trigger a cache invalidation inside of LogicalDisplay before we hit + // this point. sendDisplayEventLocked(displayId, DisplayManagerGlobal.EVENT_DISPLAY_CHANGED); } + private void handleLogicalDisplayRemoved(int displayId) { + DisplayManagerGlobal.invalidateLocalDisplayInfoCaches(); + sendDisplayEventLocked(displayId, DisplayManagerGlobal.EVENT_DISPLAY_REMOVED); + } + private void applyGlobalDisplayStateLocked(List workQueue) { final int count = mDisplayDevices.size(); for (int i = 0; i < count; i++) { @@ -1066,6 +1079,7 @@ public final class DisplayManagerService extends SystemService { } mLogicalDisplays.put(displayId, display); + DisplayManagerGlobal.invalidateLocalDisplayInfoCaches(); // Wake up waitForDefaultDisplay. if (isDefault) { @@ -1206,7 +1220,7 @@ public final class DisplayManagerService extends SystemService { display.updateLocked(mDisplayDevices); if (!display.isValidLocked()) { mLogicalDisplays.removeAt(i); - sendDisplayEventLocked(displayId, DisplayManagerGlobal.EVENT_DISPLAY_REMOVED); + handleLogicalDisplayRemoved(displayId); changed = true; } else if (!mTempDisplayInfo.equals(display.getDisplayInfoLocked())) { handleLogicalDisplayChanged(displayId, display); diff --git a/services/core/java/com/android/server/display/LogicalDisplay.java b/services/core/java/com/android/server/display/LogicalDisplay.java index ac81a6c813f79..3a5aa93d205dc 100644 --- a/services/core/java/com/android/server/display/LogicalDisplay.java +++ b/services/core/java/com/android/server/display/LogicalDisplay.java @@ -77,7 +77,7 @@ final class LogicalDisplay { * needs to be updated. * @see #getDisplayInfoLocked() */ - private DisplayInfo mInfo; + private final DisplayInfoProxy mInfo = new DisplayInfoProxy(null); // The display device that this logical display is based on and which // determines the base metrics that it uses. @@ -141,26 +141,27 @@ final class LogicalDisplay { * the data changes. */ public DisplayInfo getDisplayInfoLocked() { - if (mInfo == null) { - mInfo = new DisplayInfo(); - mInfo.copyFrom(mBaseDisplayInfo); + if (mInfo.get() == null) { + DisplayInfo info = new DisplayInfo(); + info.copyFrom(mBaseDisplayInfo); if (mOverrideDisplayInfo != null) { - mInfo.appWidth = mOverrideDisplayInfo.appWidth; - mInfo.appHeight = mOverrideDisplayInfo.appHeight; - mInfo.smallestNominalAppWidth = mOverrideDisplayInfo.smallestNominalAppWidth; - mInfo.smallestNominalAppHeight = mOverrideDisplayInfo.smallestNominalAppHeight; - mInfo.largestNominalAppWidth = mOverrideDisplayInfo.largestNominalAppWidth; - mInfo.largestNominalAppHeight = mOverrideDisplayInfo.largestNominalAppHeight; - mInfo.logicalWidth = mOverrideDisplayInfo.logicalWidth; - mInfo.logicalHeight = mOverrideDisplayInfo.logicalHeight; - mInfo.rotation = mOverrideDisplayInfo.rotation; - mInfo.displayCutout = mOverrideDisplayInfo.displayCutout; - mInfo.logicalDensityDpi = mOverrideDisplayInfo.logicalDensityDpi; - mInfo.physicalXDpi = mOverrideDisplayInfo.physicalXDpi; - mInfo.physicalYDpi = mOverrideDisplayInfo.physicalYDpi; + info.appWidth = mOverrideDisplayInfo.appWidth; + info.appHeight = mOverrideDisplayInfo.appHeight; + info.smallestNominalAppWidth = mOverrideDisplayInfo.smallestNominalAppWidth; + info.smallestNominalAppHeight = mOverrideDisplayInfo.smallestNominalAppHeight; + info.largestNominalAppWidth = mOverrideDisplayInfo.largestNominalAppWidth; + info.largestNominalAppHeight = mOverrideDisplayInfo.largestNominalAppHeight; + info.logicalWidth = mOverrideDisplayInfo.logicalWidth; + info.logicalHeight = mOverrideDisplayInfo.logicalHeight; + info.rotation = mOverrideDisplayInfo.rotation; + info.displayCutout = mOverrideDisplayInfo.displayCutout; + info.logicalDensityDpi = mOverrideDisplayInfo.logicalDensityDpi; + info.physicalXDpi = mOverrideDisplayInfo.physicalXDpi; + info.physicalYDpi = mOverrideDisplayInfo.physicalYDpi; } + mInfo.set(info); } - return mInfo; + return mInfo.get(); } /** @@ -181,17 +182,16 @@ final class LogicalDisplay { if (info != null) { if (mOverrideDisplayInfo == null) { mOverrideDisplayInfo = new DisplayInfo(info); - mInfo = null; + mInfo.set(null); return true; - } - if (!mOverrideDisplayInfo.equals(info)) { + } else if (!mOverrideDisplayInfo.equals(info)) { mOverrideDisplayInfo.copyFrom(info); - mInfo = null; + mInfo.set(null); return true; } } else if (mOverrideDisplayInfo != null) { mOverrideDisplayInfo = null; - mInfo = null; + mInfo.set(null); return true; } return false; @@ -306,7 +306,7 @@ final class LogicalDisplay { mBaseDisplayInfo.displayId = mDisplayId; mPrimaryDisplayDeviceInfo = deviceInfo; - mInfo = null; + mInfo.set(null); } } @@ -327,6 +327,7 @@ final class LogicalDisplay { private static Rect getMaskingInsets(DisplayDeviceInfo deviceInfo) { boolean maskCutout = (deviceInfo.flags & DisplayDeviceInfo.FLAG_MASK_DISPLAY_CUTOUT) != 0; if (maskCutout && deviceInfo.displayCutout != null) { + // getSafeInsets is fixed at creation time and cannot change return deviceInfo.displayCutout.getSafeInsets(); } else { return new Rect();