From 1877d0158b529663b8315482e7346a7bcaa96166 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Thu, 4 Mar 2010 17:48:13 -0800 Subject: [PATCH] Add "call" method on ContentProvider. This permits implementing interfaces which are faster than using remote Cursors. It then uses it for Settings & SettingProvider, which together account for ~50% of total ContentProvider event loop stalls across Froyo dogfooders. For fetching Settings this looks like it should reduce average Settings lookup from 10 ms to 0.4 ms on Sholes, once the SettingsProvider serves most gets from in-memory cache. Currently it brings the Sholes average down from 10ms to 2.5 ms while still using SQLite queries on each get. --- .../java/android/content/ContentProvider.java | 24 ++++- .../content/ContentProviderNative.java | 34 ++++++- .../java/android/content/ContentResolver.java | 2 +- .../android/content/IContentProvider.java | 15 +++- core/java/android/os/Bundle.java | 39 ++++++++ core/java/android/provider/Settings.java | 88 +++++++++++++++---- .../providers/settings/SettingsProvider.java | 42 +++++++++ .../test/mock/MockContentProvider.java | 18 ++++ .../test/mock/MockIContentProvider.java | 8 +- 9 files changed, 250 insertions(+), 20 deletions(-) diff --git a/core/java/android/content/ContentProvider.java b/core/java/android/content/ContentProvider.java index 91b1c4e1d5337..5fb2aaec83d7f 100644 --- a/core/java/android/content/ContentProvider.java +++ b/core/java/android/content/ContentProvider.java @@ -29,6 +29,7 @@ import android.database.IContentObserver; import android.database.SQLException; import android.net.Uri; import android.os.Binder; +import android.os.Bundle; import android.os.ParcelFileDescriptor; import android.os.Process; @@ -217,6 +218,13 @@ public abstract class ContentProvider implements ComponentCallbacks { return ContentProvider.this.openAssetFile(uri, mode); } + /** + * @hide + */ + public Bundle call(String method, String request, Bundle args) { + return ContentProvider.this.call(method, request, args); + } + private void enforceReadPermission(Uri uri) { final int uid = Binder.getCallingUid(); if (uid == mMyUid) { @@ -748,4 +756,18 @@ public abstract class ContentProvider implements ComponentCallbacks { } return results; } -} \ No newline at end of file + + /** + * @hide -- until interface has proven itself + * + * Call an provider-defined method. This can be used to implement + * interfaces that are cheaper than using a Cursor. + * + * @param method Method name to call. Opaque to framework. + * @param request Nullable String argument passed to method. + * @param args Nullable Bundle argument passed to method. + */ + public Bundle call(String method, String request, Bundle args) { + return null; + } +} diff --git a/core/java/android/content/ContentProviderNative.java b/core/java/android/content/ContentProviderNative.java index bacb6849e11ff..81b8055649216 100644 --- a/core/java/android/content/ContentProviderNative.java +++ b/core/java/android/content/ContentProviderNative.java @@ -26,6 +26,7 @@ import android.database.IBulkCursor; import android.database.IContentObserver; import android.net.Uri; import android.os.Binder; +import android.os.Bundle; import android.os.RemoteException; import android.os.IBinder; import android.os.Parcel; @@ -222,6 +223,21 @@ abstract public class ContentProviderNative extends Binder implements IContentPr } return true; } + + case CALL_TRANSACTION: + { + data.enforceInterface(IContentProvider.descriptor); + + String method = data.readString(); + String stringArg = data.readString(); + Bundle args = data.readBundle(); + + Bundle responseBundle = call(method, stringArg, args); + + reply.writeNoException(); + reply.writeBundle(responseBundle); + return true; + } } } catch (Exception e) { DatabaseUtils.writeExceptionToParcel(reply, e); @@ -485,6 +501,22 @@ final class ContentProviderProxy implements IContentProvider return fd; } + public Bundle call(String method, String request, Bundle args) + throws RemoteException { + Parcel data = Parcel.obtain(); + Parcel reply = Parcel.obtain(); + + data.writeInterfaceToken(IContentProvider.descriptor); + + data.writeString(method); + data.writeString(request); + data.writeBundle(args); + + mRemote.transact(IContentProvider.CALL_TRANSACTION, data, reply, 0); + + DatabaseUtils.readExceptionFromParcel(reply); + return reply.readBundle(); + } + private IBinder mRemote; } - diff --git a/core/java/android/content/ContentResolver.java b/core/java/android/content/ContentResolver.java index 1b0ef340a1ccd..bcef75edf17ad 100644 --- a/core/java/android/content/ContentResolver.java +++ b/core/java/android/content/ContentResolver.java @@ -736,7 +736,7 @@ public abstract class ContentResolver { * @hide */ public final IContentProvider acquireProvider(String name) { - if(name == null) { + if (name == null) { return null; } return acquireProvider(mContext, name); diff --git a/core/java/android/content/IContentProvider.java b/core/java/android/content/IContentProvider.java index 1b0ca5859540e..67e7581e5ffde 100644 --- a/core/java/android/content/IContentProvider.java +++ b/core/java/android/content/IContentProvider.java @@ -22,10 +22,11 @@ import android.database.CursorWindow; import android.database.IBulkCursor; import android.database.IContentObserver; import android.net.Uri; -import android.os.RemoteException; +import android.os.Bundle; import android.os.IBinder; import android.os.IInterface; import android.os.ParcelFileDescriptor; +import android.os.RemoteException; import java.io.FileNotFoundException; import java.util.ArrayList; @@ -58,6 +59,17 @@ public interface IContentProvider extends IInterface { throws RemoteException, FileNotFoundException; public ContentProviderResult[] applyBatch(ArrayList operations) throws RemoteException, OperationApplicationException; + /** + * @hide -- until interface has proven itself + * + * Call an provider-defined method. This can be used to implement + * interfaces that are cheaper than using a Cursor. + * + * @param method Method name to call. Opaque to framework. + * @param request Nullable String argument passed to method. + * @param args Nullable Bundle argument passed to method. + */ + public Bundle call(String method, String request, Bundle args) throws RemoteException; /* IPC constants */ static final String descriptor = "android.content.IContentProvider"; @@ -71,4 +83,5 @@ public interface IContentProvider extends IInterface { static final int OPEN_FILE_TRANSACTION = IBinder.FIRST_CALL_TRANSACTION + 13; static final int OPEN_ASSET_FILE_TRANSACTION = IBinder.FIRST_CALL_TRANSACTION + 14; static final int APPLY_BATCH_TRANSACTION = IBinder.FIRST_CALL_TRANSACTION + 19; + static final int CALL_TRANSACTION = IBinder.FIRST_CALL_TRANSACTION + 20; } diff --git a/core/java/android/os/Bundle.java b/core/java/android/os/Bundle.java index d6fe107b10b24..0ec1c74135529 100644 --- a/core/java/android/os/Bundle.java +++ b/core/java/android/os/Bundle.java @@ -131,6 +131,45 @@ public final class Bundle implements Parcelable, Cloneable { mClassLoader = b.mClassLoader; } + /** + * Make a Bundle for a single key/value pair. + * + * @hide + */ + public static Bundle forPair(String key, String value) { + // TODO: optimize this case. + Bundle b = new Bundle(1); + b.putString(key, value); + return b; + } + + /** + * TODO: optimize this later (getting just the value part of a Bundle + * with a single pair) once Bundle.forPair() above is implemented + * with a special single-value Map implementation/serialization. + * + * Note: value in single-pair Bundle may be null. + * + * @hide + */ + public String getPairValue() { + unparcel(); + int size = mMap.size(); + if (size > 1) { + Log.w(LOG_TAG, "getPairValue() used on Bundle with multiple pairs."); + } + if (size == 0) { + return null; + } + Object o = mMap.values().iterator().next(); + try { + return (String) o; + } catch (ClassCastException e) { + typeWarning("getPairValue()", o, "String", e); + return null; + } + } + /** * Changes the ClassLoader this Bundle uses when instantiating objects. * diff --git a/core/java/android/provider/Settings.java b/core/java/android/provider/Settings.java index 7df509ff8006c..726f98af600c0 100644 --- a/core/java/android/provider/Settings.java +++ b/core/java/android/provider/Settings.java @@ -27,6 +27,7 @@ import android.content.ContentQueryMap; import android.content.ContentResolver; import android.content.ContentValues; import android.content.Context; +import android.content.IContentProvider; import android.content.Intent; import android.content.pm.PackageManager; import android.content.pm.ResolveInfo; @@ -491,6 +492,16 @@ public final class Settings { // End of Intent actions for Settings + /** + * @hide - Private call() method on SettingsProvider to read from 'system' table. + */ + public static final String CALL_METHOD_GET_SYSTEM = "GET_system"; + + /** + * @hide - Private call() method on SettingsProvider to read from 'secure' table. + */ + public static final String CALL_METHOD_GET_SECURE = "GET_secure"; + /** * Activity Extra: Limit available options in launched activity based on the given authority. *

@@ -544,23 +555,36 @@ public final class Settings { } } + // Thread-safe. private static class NameValueCache { private final String mVersionSystemProperty; private final Uri mUri; - // Must synchronize(mValues) to access mValues and mValuesVersion. + private static final String[] SELECT_VALUE = + new String[] { Settings.NameValueTable.VALUE }; + private static final String NAME_EQ_PLACEHOLDER = "name=?"; + + // Must synchronize on 'this' to access mValues and mValuesVersion. private final HashMap mValues = new HashMap(); private long mValuesVersion = 0; - public NameValueCache(String versionSystemProperty, Uri uri) { + // Initially null; set lazily and held forever. Synchronized on 'this'. + private IContentProvider mContentProvider = null; + + // The method we'll call (or null, to not use) on the provider + // for the fast path of retrieving settings. + private final String mCallCommand; + + public NameValueCache(String versionSystemProperty, Uri uri, String callCommand) { mVersionSystemProperty = versionSystemProperty; mUri = uri; + mCallCommand = callCommand; } public String getString(ContentResolver cr, String name) { long newValuesVersion = SystemProperties.getLong(mVersionSystemProperty, 0); - synchronized (mValues) { + synchronized (this) { if (mValuesVersion != newValuesVersion) { if (LOCAL_LOGV) { Log.v(TAG, "invalidate [" + mUri.getLastPathSegment() + "]: current " + @@ -576,17 +600,47 @@ public final class Settings { } } + IContentProvider cp = null; + synchronized (this) { + cp = mContentProvider; + if (cp == null) { + cp = mContentProvider = cr.acquireProvider(mUri.getAuthority()); + } + } + + // Try the fast path first, not using query(). If this + // fails (alternate Settings provider that doesn't support + // this interface?) then we fall back to the query/table + // interface. + if (mCallCommand != null) { + try { + Bundle b = cp.call(mCallCommand, name, null); + if (b != null) { + String value = b.getPairValue(); + synchronized (this) { + mValues.put(name, value); + } + return value; + } + // If the response Bundle is null, we fall through + // to the query interface below. + } catch (RemoteException e) { + // Not supported by the remote side? Fall through + // to query(). + } + } + Cursor c = null; try { - c = cr.query(mUri, new String[] { Settings.NameValueTable.VALUE }, - Settings.NameValueTable.NAME + "=?", new String[]{name}, null); + c = cp.query(mUri, SELECT_VALUE, NAME_EQ_PLACEHOLDER, + new String[]{name}, null); if (c == null) { Log.w(TAG, "Can't get key " + name + " from " + mUri); return null; } String value = c.moveToNext() ? c.getString(0) : null; - synchronized (mValues) { + synchronized (this) { mValues.put(name, value); } if (LOCAL_LOGV) { @@ -594,7 +648,7 @@ public final class Settings { name + " = " + (value == null ? "(null)" : value)); } return value; - } catch (SQLException e) { + } catch (RemoteException e) { Log.w(TAG, "Can't get key " + name + " from " + mUri, e); return null; // Return null, but don't cache it. } finally { @@ -611,7 +665,8 @@ public final class Settings { public static final class System extends NameValueTable { public static final String SYS_PROP_SETTING_VERSION = "sys.settings_system_version"; - private static volatile NameValueCache mNameValueCache = null; + // Populated lazily, guarded by class object: + private static NameValueCache sNameValueCache = null; private static final HashSet MOVED_TO_SECURE; static { @@ -660,10 +715,11 @@ public final class Settings { + " to android.provider.Settings.Secure, returning read-only value."); return Secure.getString(resolver, name); } - if (mNameValueCache == null) { - mNameValueCache = new NameValueCache(SYS_PROP_SETTING_VERSION, CONTENT_URI); + if (sNameValueCache == null) { + sNameValueCache = new NameValueCache(SYS_PROP_SETTING_VERSION, CONTENT_URI, + CALL_METHOD_GET_SYSTEM); } - return mNameValueCache.getString(resolver, name); + return sNameValueCache.getString(resolver, name); } /** @@ -1905,7 +1961,8 @@ public final class Settings { public static final class Secure extends NameValueTable { public static final String SYS_PROP_SETTING_VERSION = "sys.settings_secure_version"; - private static volatile NameValueCache mNameValueCache = null; + // Populated lazily, guarded by class object: + private static NameValueCache sNameValueCache = null; /** * Look up a name in the database. @@ -1914,10 +1971,11 @@ public final class Settings { * @return the corresponding value, or null if not present */ public synchronized static String getString(ContentResolver resolver, String name) { - if (mNameValueCache == null) { - mNameValueCache = new NameValueCache(SYS_PROP_SETTING_VERSION, CONTENT_URI); + if (sNameValueCache == null) { + sNameValueCache = new NameValueCache(SYS_PROP_SETTING_VERSION, CONTENT_URI, + CALL_METHOD_GET_SECURE); } - return mNameValueCache.getString(resolver, name); + return sNameValueCache.getString(resolver, name); } /** diff --git a/packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java b/packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java index db802d33cf1d8..4f1146b7ca7f3 100644 --- a/packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java +++ b/packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java @@ -30,9 +30,11 @@ import android.content.pm.PackageManager; import android.content.res.AssetFileDescriptor; import android.database.Cursor; import android.database.sqlite.SQLiteDatabase; +import android.database.sqlite.SQLiteException; import android.database.sqlite.SQLiteQueryBuilder; import android.media.RingtoneManager; import android.net.Uri; +import android.os.Bundle; import android.os.ParcelFileDescriptor; import android.os.SystemProperties; import android.provider.DrmStore; @@ -48,6 +50,8 @@ public class SettingsProvider extends ContentProvider { private static final String TABLE_FAVORITES = "favorites"; private static final String TABLE_OLD_FAVORITES = "old_favorites"; + private static final String[] COLUMN_VALUE = new String[] { "value" }; + protected DatabaseHelper mOpenHelper; private BackupManager mBackupManager; @@ -220,6 +224,44 @@ public class SettingsProvider extends ContentProvider { } } + /** + * Fast path that avoids the use of chatty remoted Cursors. + */ + @Override + public Bundle call(String method, String request, Bundle args) { + if (Settings.CALL_METHOD_GET_SYSTEM.equals(method)) { + return lookupValue("system", request); + } + + if (Settings.CALL_METHOD_GET_SECURE.equals(method)) { + return lookupValue("secure", request); + } + return null; + } + + // Looks up value 'key' in 'table' and returns either a single-pair Bundle, + // possibly with a null value, or null on failure. + private Bundle lookupValue(String table, String key) { + // TODO: avoid database lookup and serve from in-process cache. + SQLiteDatabase db = mOpenHelper.getReadableDatabase(); + Cursor cursor = null; + try { + cursor = db.query(table, COLUMN_VALUE, "name=?", new String[]{key}, + null, null, null, null); + if (cursor != null && cursor.getCount() == 1) { + cursor.moveToFirst(); + String value = cursor.getString(0); + return Bundle.forPair("value", value); + } + } catch (SQLiteException e) { + Log.w(TAG, "settings lookup error", e); + return null; + } finally { + if (cursor != null) cursor.close(); + } + return Bundle.forPair("value", null); + } + @Override public Cursor query(Uri url, String[] select, String where, String[] whereArgs, String sort) { SqlArguments args = new SqlArguments(url, where, whereArgs); diff --git a/test-runner/src/android/test/mock/MockContentProvider.java b/test-runner/src/android/test/mock/MockContentProvider.java index 4078622333a3a..3fd71c8426d0a 100644 --- a/test-runner/src/android/test/mock/MockContentProvider.java +++ b/test-runner/src/android/test/mock/MockContentProvider.java @@ -32,6 +32,7 @@ import android.database.CursorWindow; import android.database.IBulkCursor; import android.database.IContentObserver; import android.net.Uri; +import android.os.Bundle; import android.os.IBinder; import android.os.ParcelFileDescriptor; import android.os.RemoteException; @@ -113,6 +114,15 @@ public class MockContentProvider extends ContentProvider { return MockContentProvider.this.update(url, values, selection, selectionArgs); } + /** + * @hide + */ + @SuppressWarnings("unused") + public Bundle call(String method, String request, Bundle args) + throws RemoteException { + return MockContentProvider.this.call(method, request, args); + } + public IBinder asBinder() { throw new UnsupportedOperationException(); } @@ -204,6 +214,14 @@ public class MockContentProvider extends ContentProvider { throw new UnsupportedOperationException("unimplemented mock method"); } + /** + * @hide + */ + @Override + public Bundle call(String method, String request, Bundle args) { + throw new UnsupportedOperationException("unimplemented mock method call"); + } + /** * Returns IContentProvider which calls back same methods in this class. * By overriding this class, we avoid the mechanism hidden behind ContentProvider diff --git a/test-runner/src/android/test/mock/MockIContentProvider.java b/test-runner/src/android/test/mock/MockIContentProvider.java index 7c0a1e21ae700..0be5bea276f3a 100644 --- a/test-runner/src/android/test/mock/MockIContentProvider.java +++ b/test-runner/src/android/test/mock/MockIContentProvider.java @@ -27,6 +27,7 @@ import android.database.CursorWindow; import android.database.IBulkCursor; import android.database.IContentObserver; import android.net.Uri; +import android.os.Bundle; import android.os.IBinder; import android.os.ParcelFileDescriptor; import android.os.RemoteException; @@ -38,7 +39,7 @@ import java.util.ArrayList; * {@link java.lang.UnsupportedOperationException}. Tests can extend this class to * implement behavior needed for tests. * - * @hide - @hide because this exposes bulkQuery(), which must also be hidden. + * @hide - @hide because this exposes bulkQuery() and call(), which must also be hidden. */ public class MockIContentProvider implements IContentProvider { public int bulkInsert(Uri url, ContentValues[] initialValues) { @@ -93,6 +94,11 @@ public class MockIContentProvider implements IContentProvider { throw new UnsupportedOperationException("unimplemented mock method"); } + public Bundle call(String method, String request, Bundle args) + throws RemoteException { + throw new UnsupportedOperationException("unimplemented mock method"); + } + public IBinder asBinder() { throw new UnsupportedOperationException("unimplemented mock method"); }