From bcc0b914e3aee321a6d8cb35e8ecc9e89f80ae2a Mon Sep 17 00:00:00 2001 From: John Reck Date: Fri, 29 Mar 2019 14:49:05 -0700 Subject: [PATCH] Fix performance regression in textclassifier DeviceConfig is far too slow to be used here, so disable it. Test: trace calculator launch Change-Id: I6b7ab56e4015448ee068deb49e7f6fa133fea53c --- .../view/textclassifier/ConfigParser.java | 52 +++++++++++++------ .../TextClassificationManager.java | 14 +++-- .../view/textclassifier/ConfigParserTest.java | 5 ++ 3 files changed, 50 insertions(+), 21 deletions(-) diff --git a/core/java/android/view/textclassifier/ConfigParser.java b/core/java/android/view/textclassifier/ConfigParser.java index 8e0bdf9eaa851..b475412bf6699 100644 --- a/core/java/android/view/textclassifier/ConfigParser.java +++ b/core/java/android/view/textclassifier/ConfigParser.java @@ -33,6 +33,10 @@ public final class ConfigParser { private final KeyValueListParser mParser; + // TODO: Re-enable DeviceConfig when it has reasonable performance or just delete the + // option of using DeviceConfig entirely. + static final boolean ENABLE_DEVICE_CONFIG = false; + public ConfigParser(@Nullable String textClassifierConstants) { final KeyValueListParser parser = new KeyValueListParser(','); try { @@ -48,39 +52,55 @@ public final class ConfigParser { * Reads a boolean flag. */ public boolean getBoolean(String key, boolean defaultValue) { - return DeviceConfig.getBoolean( - DeviceConfig.NAMESPACE_TEXTCLASSIFIER, - key, - mParser.getBoolean(key, defaultValue)); + if (ENABLE_DEVICE_CONFIG) { + return DeviceConfig.getBoolean( + DeviceConfig.NAMESPACE_TEXTCLASSIFIER, + key, + mParser.getBoolean(key, defaultValue)); + } else { + return mParser.getBoolean(key, defaultValue); + } } /** * Reads an integer flag. */ public int getInt(String key, int defaultValue) { - return DeviceConfig.getInt( - DeviceConfig.NAMESPACE_TEXTCLASSIFIER, - key, - mParser.getInt(key, defaultValue)); + if (ENABLE_DEVICE_CONFIG) { + return DeviceConfig.getInt( + DeviceConfig.NAMESPACE_TEXTCLASSIFIER, + key, + mParser.getInt(key, defaultValue)); + } else { + return mParser.getInt(key, defaultValue); + } } /** * Reads a float flag. */ public float getFloat(String key, float defaultValue) { - return DeviceConfig.getFloat( - DeviceConfig.NAMESPACE_TEXTCLASSIFIER, - key, - mParser.getFloat(key, defaultValue)); + if (ENABLE_DEVICE_CONFIG) { + return DeviceConfig.getFloat( + DeviceConfig.NAMESPACE_TEXTCLASSIFIER, + key, + mParser.getFloat(key, defaultValue)); + } else { + return mParser.getFloat(key, defaultValue); + } } /** * Reads a string flag. */ public String getString(String key, String defaultValue) { - return DeviceConfig.getString( - DeviceConfig.NAMESPACE_TEXTCLASSIFIER, - key, - mParser.getString(key, defaultValue)); + if (ENABLE_DEVICE_CONFIG) { + return DeviceConfig.getString( + DeviceConfig.NAMESPACE_TEXTCLASSIFIER, + key, + mParser.getString(key, defaultValue)); + } else { + return mParser.getString(key, defaultValue); + } } } diff --git a/core/java/android/view/textclassifier/TextClassificationManager.java b/core/java/android/view/textclassifier/TextClassificationManager.java index 868cbb1ce58e9..fa898c3c89e01 100644 --- a/core/java/android/view/textclassifier/TextClassificationManager.java +++ b/core/java/android/view/textclassifier/TextClassificationManager.java @@ -197,7 +197,9 @@ public final class TextClassificationManager { if (mSettingsObserver != null) { getApplicationContext().getContentResolver() .unregisterContentObserver(mSettingsObserver); - DeviceConfig.removeOnPropertyChangedListener(mSettingsObserver); + if (ConfigParser.ENABLE_DEVICE_CONFIG) { + DeviceConfig.removeOnPropertyChangedListener(mSettingsObserver); + } } } finally { super.finalize(); @@ -292,10 +294,12 @@ public final class TextClassificationManager { Settings.Global.getUriFor(Settings.Global.TEXT_CLASSIFIER_CONSTANTS), false /* notifyForDescendants */, this); - DeviceConfig.addOnPropertyChangedListener( - DeviceConfig.NAMESPACE_TEXTCLASSIFIER, - ActivityThread.currentApplication().getMainExecutor(), - this); + if (ConfigParser.ENABLE_DEVICE_CONFIG) { + DeviceConfig.addOnPropertyChangedListener( + DeviceConfig.NAMESPACE_TEXTCLASSIFIER, + ActivityThread.currentApplication().getMainExecutor(), + this); + } } @Override diff --git a/core/tests/coretests/src/android/view/textclassifier/ConfigParserTest.java b/core/tests/coretests/src/android/view/textclassifier/ConfigParserTest.java index 1b3c724523391..f1cfe24762a89 100644 --- a/core/tests/coretests/src/android/view/textclassifier/ConfigParserTest.java +++ b/core/tests/coretests/src/android/view/textclassifier/ConfigParserTest.java @@ -26,6 +26,7 @@ import androidx.test.runner.AndroidJUnit4; import org.junit.After; import org.junit.Before; +import org.junit.Ignore; import org.junit.Test; import org.junit.runner.RunWith; @@ -58,6 +59,7 @@ public class ConfigParserTest { } @Test + @Ignore // TODO: Re-enable once ConfigParser#ENABLE_DEVICE_CONFIG is finalized public void getBoolean_deviceConfig() { DeviceConfig.setProperty( DeviceConfig.NAMESPACE_TEXTCLASSIFIER, @@ -77,6 +79,7 @@ public class ConfigParserTest { } @Test + @Ignore // TODO: Re-enable once ConfigParser#ENABLE_DEVICE_CONFIG is finalized public void getInt_deviceConfig() { DeviceConfig.setProperty( DeviceConfig.NAMESPACE_TEXTCLASSIFIER, @@ -94,6 +97,7 @@ public class ConfigParserTest { } @Test + @Ignore // TODO: Re-enable once ConfigParser#ENABLE_DEVICE_CONFIG is finalized public void getFloat_deviceConfig() { DeviceConfig.setProperty( DeviceConfig.NAMESPACE_TEXTCLASSIFIER, @@ -111,6 +115,7 @@ public class ConfigParserTest { } @Test + @Ignore // TODO: Re-enable once ConfigParser#ENABLE_DEVICE_CONFIG is finalized public void getString_deviceConfig() { DeviceConfig.setProperty( DeviceConfig.NAMESPACE_TEXTCLASSIFIER,