Cache null binder results in PIC (d24ac92c) · Commits · e / os / android_frameworks_base

core/java/android/app/PropertyInvalidatedCache.java

+65 −13

Original line number	Original line	Diff line number	Diff line
	@@ -314,6 +314,14 @@ public class PropertyInvalidatedCache<Query, Result> {
	@GuardedBy("mLock")		@GuardedBy("mLock")
	private long mMisses = 0;		private long mMisses = 0;

			// This counter tracks the number of times {@link #recompute} returned a null value. Null
			// results are cached, or not, depending on instantiation arguments. Caching nulls when they
			// should not be cached is a functional error. Failing to cache nulls that can be cached is a
			// performance error. A non-zero value here means the cache should be examined to be sure
			// that nulls are correctly cached, or not.
			@GuardedBy("mLock")
			private long mNulls = 0;

	@GuardedBy("mLock")		@GuardedBy("mLock")
	private long[] mSkips = new long[MAX_RESERVED_NONCE + 1];		private long[] mSkips = new long[MAX_RESERVED_NONCE + 1];

	@@ -373,6 +381,11 @@ public class PropertyInvalidatedCache<Query, Result> {
	*/		*/
	private final String mCacheName;		private final String mCacheName;

			/**
			* True if nulls are valid returns from recompute().
			*/
			private final boolean mCacheNullResults;

	/**		/**
	* The function that computes a Result, given a Query. This function is called on a		* The function that computes a Result, given a Query. This function is called on a
	* cache miss.		* cache miss.
	@@ -508,6 +521,19 @@ public class PropertyInvalidatedCache<Query, Result> {
	}		}
	}		}

			/**
			* Return true if the entry is in the cache.
			*/
			boolean containsKey(Query query) {
			final int uid = callerUid();
			var map = mCache.get(uid);
			if (map != null) {
			return map.containsKey(query);
			} else {
			return false;
			}
			}

	/**		/**
	* Remove an entry from the cache.		* Remove an entry from the cache.
	*/		*/
	@@ -1101,7 +1127,7 @@ public class PropertyInvalidatedCache<Query, Result> {
	* @hide		* @hide
	*/		*/
	public static record Args(@NonNull String mModule, @Nullable String mApi,		public static record Args(@NonNull String mModule, @Nullable String mApi,
	int mMaxEntries, boolean mIsolateUids, boolean mTestMode) {		int mMaxEntries, boolean mIsolateUids, boolean mTestMode, boolean mCacheNulls) {

	// Validation: the module must be one of the known module strings and the maxEntries must		// Validation: the module must be one of the known module strings and the maxEntries must
	// be positive.		// be positive.
	@@ -1119,24 +1145,29 @@ public class PropertyInvalidatedCache<Query, Result> {
	null, // api		null, // api
	32, // maxEntries		32, // maxEntries
	true, // isolateUids		true, // isolateUids
	false // testMode		false, // testMode
			true // allowNulls
	);		);
	}		}

	public Args api(@NonNull String api) {		public Args api(@NonNull String api) {
	return new Args(mModule, api, mMaxEntries, mIsolateUids, mTestMode);		return new Args(mModule, api, mMaxEntries, mIsolateUids, mTestMode, mCacheNulls);
	}		}

	public Args maxEntries(int val) {		public Args maxEntries(int val) {
	return new Args(mModule, mApi, val, mIsolateUids, mTestMode);		return new Args(mModule, mApi, val, mIsolateUids, mTestMode, mCacheNulls);
	}		}

	public Args isolateUids(boolean val) {		public Args isolateUids(boolean val) {
	return new Args(mModule, mApi, mMaxEntries, val, mTestMode);		return new Args(mModule, mApi, mMaxEntries, val, mTestMode, mCacheNulls);
	}		}

	public Args testMode(boolean val) {		public Args testMode(boolean val) {
	return new Args(mModule, mApi, mMaxEntries, mIsolateUids, val);		return new Args(mModule, mApi, mMaxEntries, mIsolateUids, val, mCacheNulls);
			}

			public Args cacheNulls(boolean val) {
			return new Args(mModule, mApi, mMaxEntries, mIsolateUids, mTestMode, val);
	}		}
	}		}

	@@ -1153,6 +1184,7 @@ public class PropertyInvalidatedCache<Query, Result> {
	@Nullable QueryHandler<Query, Result> computer) {		@Nullable QueryHandler<Query, Result> computer) {
	mPropertyName = createPropertyName(args.mModule, args.mApi);		mPropertyName = createPropertyName(args.mModule, args.mApi);
	mCacheName = cacheName;		mCacheName = cacheName;
			mCacheNullResults = args.mCacheNulls && Flags.picCacheNulls();
	mNonce = getNonceHandler(mPropertyName);		mNonce = getNonceHandler(mPropertyName);
	mMaxEntries = args.mMaxEntries;		mMaxEntries = args.mMaxEntries;
	mCache = new CacheMap<>(args.mIsolateUids, args.mTestMode);		mCache = new CacheMap<>(args.mIsolateUids, args.mTestMode);
	@@ -1491,12 +1523,24 @@ public class PropertyInvalidatedCache<Query, Result> {
	}		}
	return recompute(query);		return recompute(query);
	}		}

			final boolean cacheHit;
	final Result cachedResult;		final Result cachedResult;
	synchronized (mLock) {		synchronized (mLock) {
	if (currentNonce == mLastSeenNonce) {		if (currentNonce == mLastSeenNonce) {
	cachedResult = mCache.get(query);		cachedResult = mCache.get(query);
			if (cachedResult == null) {
	if (cachedResult != null) mHits++;		if (mCacheNullResults) {
			cacheHit = mCache.containsKey(query);
			} else {
			cacheHit = false;
			}
			} else {
			cacheHit = true;
			}
			if (cacheHit) {
			mHits++;
			}
	} else {		} else {
	if (DEBUG) {		if (DEBUG) {
	Log.d(TAG, formatSimple(		Log.d(TAG, formatSimple(
	@@ -1506,16 +1550,18 @@ public class PropertyInvalidatedCache<Query, Result> {
	}		}
	clear();		clear();
	mLastSeenNonce = currentNonce;		mLastSeenNonce = currentNonce;
			cacheHit = false;
	cachedResult = null;		cachedResult = null;
	}		}
	}		}

	// Cache hit --- but we're not quite done yet. A value in the cache might need to		// Cache hit --- but we're not quite done yet. A value in the cache might need to
	// be augmented in a "refresh" operation. The refresh operation can combine the		// be augmented in a "refresh" operation. The refresh operation can combine the
	// old and the new nonce values. In order to make sure the new parts of the value		// old and the new nonce values. In order to make sure the new parts of the value
	// are consistent with the old, possibly-reused parts, we check the property value		// are consistent with the old, possibly-reused parts, we check the property value
	// again after the refresh and do the whole fetch again if the property invalidated		// again after the refresh and do the whole fetch again if the property invalidated
	// us while we were refreshing.		// us while we were refreshing.
	if (cachedResult != null) {		if (cacheHit) {
	final Result refreshedResult = refresh(cachedResult, query);		final Result refreshedResult = refresh(cachedResult, query);
	if (refreshedResult != cachedResult) {		if (refreshedResult != cachedResult) {
	if (DEBUG) {		if (DEBUG) {
	@@ -1550,6 +1596,7 @@ public class PropertyInvalidatedCache<Query, Result> {
	}		}
	return maybeCheckConsistency(query, cachedResult);		return maybeCheckConsistency(query, cachedResult);
	}		}

	// Cache miss: make the value from scratch.		// Cache miss: make the value from scratch.
	if (DEBUG) {		if (DEBUG) {
	Log.d(TAG, "cache miss for " + cacheName() + " " + queryToString(query));		Log.d(TAG, "cache miss for " + cacheName() + " " + queryToString(query));
	@@ -1558,9 +1605,14 @@ public class PropertyInvalidatedCache<Query, Result> {
	synchronized (mLock) {		synchronized (mLock) {
	// If someone else invalidated the cache while we did the recomputation, don't		// If someone else invalidated the cache while we did the recomputation, don't
	// update the cache with a potentially stale result.		// update the cache with a potentially stale result.
	if (mLastSeenNonce == currentNonce && result != null) {		if (mLastSeenNonce == currentNonce) {
			if (result != null \|\| mCacheNullResults) {
	mCache.put(query, result);		mCache.put(query, result);
	}		}
			if (result == null) {
			mNulls++;
			}
			}
	mMisses++;		mMisses++;
	}		}
	return maybeCheckConsistency(query, result);		return maybeCheckConsistency(query, result);
	@@ -1977,8 +2029,8 @@ public class PropertyInvalidatedCache<Query, Result> {
	pw.println(formatSimple(" Cache Name: %s", cacheName()));		pw.println(formatSimple(" Cache Name: %s", cacheName()));
	pw.println(formatSimple(" Property: %s", mPropertyName));		pw.println(formatSimple(" Property: %s", mPropertyName));
	pw.println(formatSimple(		pw.println(formatSimple(
	" Hits: %d, Misses: %d, Skips: %d, Clears: %d",		" Hits: %d, Misses: %d, Skips: %d, Clears: %d, Nulls: %d",
	mHits, mMisses, getSkipsLocked(), mClears));		mHits, mMisses, getSkipsLocked(), mClears, mNulls));

	// Print all the skip reasons.		// Print all the skip reasons.
	pw.format(" Skip-%s: %d", sNonceName[0], mSkips[0]);		pw.format(" Skip-%s: %d", sNonceName[0], mSkips[0]);

core/java/android/app/performance.aconfig

+7 −0

Original line number	Original line	Diff line number	Diff line
	@@ -35,3 +35,10 @@ flag {
	bug: "373752556"		bug: "373752556"
	}		}

			flag {
			namespace: "system_performance"
			name: "pic_cache_nulls"
			is_fixed_read_only: true
			description: "Cache null returns from binder calls"
			bug: "372923336"
			}

core/tests/coretests/src/android/app/PropertyInvalidatedCacheTests.java

+38 −1

Original line number	Original line	Diff line number	Diff line
	@@ -16,6 +16,7 @@

	package android.app;		package android.app;

			import static android.app.Flags.FLAG_PIC_CACHE_NULLS;
	import static android.app.Flags.FLAG_PIC_ISOLATE_CACHE_BY_UID;		import static android.app.Flags.FLAG_PIC_ISOLATE_CACHE_BY_UID;
	import static android.app.PropertyInvalidatedCache.NONCE_UNSET;		import static android.app.PropertyInvalidatedCache.NONCE_UNSET;
	import static android.app.PropertyInvalidatedCache.MODULE_BLUETOOTH;		import static android.app.PropertyInvalidatedCache.MODULE_BLUETOOTH;
	@@ -229,8 +230,13 @@ public class PropertyInvalidatedCacheTests {
	@Override		@Override
	public String apply(Integer qv) {		public String apply(Integer qv) {
	mRecomputeCount += 1;		mRecomputeCount += 1;
			// Special case for testing caches of nulls. Integers in the range 30-40 return null.
			if (qv >= 30 && qv < 40) {
			return null;
			} else {
	return "foo" + qv.toString();		return "foo" + qv.toString();
	}		}
			}

	int getRecomputeCount() {		int getRecomputeCount() {
	return mRecomputeCount;		return mRecomputeCount;
	@@ -643,4 +649,35 @@ public class PropertyInvalidatedCacheTests {
	Binder.restoreCallingWorkSource(token);		Binder.restoreCallingWorkSource(token);
	}		}
	}		}

			@RequiresFlagsEnabled(FLAG_PIC_CACHE_NULLS)
			@Test
			public void testCachingNulls() {
			TestCache cache = new TestCache(new Args(MODULE_TEST)
			.maxEntries(4).api("testCachingNulls").cacheNulls(true),
			new TestQuery());
			cache.invalidateCache();
			assertEquals("foo1", cache.query(1));
			assertEquals("foo2", cache.query(2));
			assertEquals(null, cache.query(30));
			assertEquals(3, cache.getRecomputeCount());
			assertEquals("foo1", cache.query(1));
			assertEquals("foo2", cache.query(2));
			assertEquals(null, cache.query(30));
			assertEquals(3, cache.getRecomputeCount());

			cache = new TestCache(new Args(MODULE_TEST)
			.maxEntries(4).api("testCachingNulls").cacheNulls(false),
			new TestQuery());
			cache.invalidateCache();
			assertEquals("foo1", cache.query(1));
			assertEquals("foo2", cache.query(2));
			assertEquals(null, cache.query(30));
			assertEquals(3, cache.getRecomputeCount());
			assertEquals("foo1", cache.query(1));
			assertEquals("foo2", cache.query(2));
			assertEquals(null, cache.query(30));
			// The recompute is 4 because nulls were not cached.
			assertEquals(4, cache.getRecomputeCount());
			}
	}		}