Offer to write Strings through Parcels as UTF-8.

Recently while investigating some Binder limits, I discovered that we're still sending Strings across Binder as UTF-16, which is very wasteful for two reasons: 1. The majority of data flowing through APIs like PackageManager is already limited to US-ASCII, and by sending UTF-16 we're wasting half of our transactions on null-byte overhead. 2. Internally ART is already "compressing" simple strings by storing them as US-ASCII instead of UTF-16, meaning every time we want to write a simple string to Binder, we're forced to first inflate it to UTF-16. This change first updates Parcel.cpp to accept char* UTF-8 strings, similar to how it accepts char16_t* for UTF-16. It then offers both UTF-8 and UTF-16 variants to Parcel.java via JNI. We also update the String8 handling to behave identical to String16. This change adds benchmarking to show that these new methods are about 50% faster for US-ASCII strings, and about 68% faster for complex strings that reference higher Unicode planes. (So an improvement in both cases!) Bug: 154436100 Test: atest FrameworksCoreTests:ParcelTest Test: make core-libart conscrypt okhttp bouncycastle vogar caliper && vogar --mode app_process --benchmark frameworks/base/core/tests/benchmarks/src/android/os/ParcelStringBenchmark.java Change-Id: I22a11d3497486d922ec8e14c85df66ca096b8f2a
2020-04-20 21:26:27 -06:00
parent 6d18727690
commit d387e79ae1
5 changed files with 221 additions and 18 deletions
--- a/core/java/android/content/pm/PackageParserCacheHelper.java
+++ b/core/java/android/content/pm/PackageParserCacheHelper.java
@@ -78,10 +78,19 @@ public class PackageParserCacheHelper {
        /**
         * Read an string index from a parcel, and returns the corresponding string from the pool.
         */
-        @Override
        public String readString(Parcel p) {
            return mStrings.get(p.readInt());
        }
+
+        @Override
+        public String readString8(Parcel p) {
+            return readString(p);
+        }
+
+        @Override
+        public String readString16(Parcel p) {
+            return readString(p);
+        }
    }

    /**
@@ -110,7 +119,6 @@ public class PackageParserCacheHelper {
         * Instead of writing a string directly to a parcel, this method adds it to the pool,
         * and write the index in the pool to the parcel.
         */
-        @Override
        public void writeString(Parcel p, String s) {
            final Integer cur = mIndexes.get(s);
            if (cur != null) {
@@ -133,6 +141,16 @@ public class PackageParserCacheHelper {
            }
        }

+        @Override
+        public void writeString8(Parcel p, String s) {
+            writeString(p, s);
+        }
+
+        @Override
+        public void writeString16(Parcel p, String s) {
+            writeString(p, s);
+        }
+
        /**
         * Closes a parcel by appending the string pool at the end and updating the pool offset,
         * which it assumes is at the first byte.  It also uninstalls itself as a read-write helper.
--- a/core/java/android/os/Parcel.java
+++ b/core/java/android/os/Parcel.java
@@ -307,7 +307,9 @@ public final class Parcel {
    @FastNative
    private static native void nativeWriteDouble(long nativePtr, double val);
    @FastNative
-    static native void nativeWriteString(long nativePtr, String val);
+    private static native void nativeWriteString8(long nativePtr, String val);
+    @FastNative
+    private static native void nativeWriteString16(long nativePtr, String val);
    @FastNative
    private static native void nativeWriteStrongBinder(long nativePtr, IBinder val);
    @FastNative
@@ -325,7 +327,9 @@ public final class Parcel {
    @CriticalNative
    private static native double nativeReadDouble(long nativePtr);
    @FastNative
-    static native String nativeReadString(long nativePtr);
+    private static native String nativeReadString8(long nativePtr);
+    @FastNative
+    private static native String nativeReadString16(long nativePtr);
    @FastNative
    private static native IBinder nativeReadStrongBinder(long nativePtr);
    @FastNative
@@ -386,8 +390,12 @@ public final class Parcel {
         * must use {@link #writeStringNoHelper(String)} to avoid
         * infinity recursive calls.
         */
-        public void writeString(Parcel p, String s) {
-            nativeWriteString(p.mNativePtr, s);
+        public void writeString8(Parcel p, String s) {
+            p.writeString8NoHelper(s);
+        }
+
+        public void writeString16(Parcel p, String s) {
+            p.writeString16NoHelper(s);
        }

        /**
@@ -395,8 +403,12 @@ public final class Parcel {
         * must use {@link #readStringNoHelper()} to avoid
         * infinity recursive calls.
         */
-        public String readString(Parcel p) {
-            return nativeReadString(p.mNativePtr);
+        public String readString8(Parcel p) {
+            return p.readString8NoHelper();
+        }
+
+        public String readString16(Parcel p) {
+            return p.readString16NoHelper();
        }
    }

@@ -759,7 +771,17 @@ public final class Parcel {
     * growing dataCapacity() if needed.
     */
    public final void writeString(@Nullable String val) {
-        mReadWriteHelper.writeString(this, val);
+        writeString16(val);
+    }
+
+    /** {@hide} */
+    public final void writeString8(@Nullable String val) {
+        mReadWriteHelper.writeString8(this, val);
+    }
+
+    /** {@hide} */
+    public final void writeString16(@Nullable String val) {
+        mReadWriteHelper.writeString16(this, val);
    }

    /**
@@ -770,7 +792,17 @@ public final class Parcel {
     * @hide
     */
    public void writeStringNoHelper(@Nullable String val) {
-        nativeWriteString(mNativePtr, val);
+        writeString16NoHelper(val);
+    }
+
+    /** {@hide} */
+    public void writeString8NoHelper(@Nullable String val) {
+        nativeWriteString8(mNativePtr, val);
+    }
+
+    /** {@hide} */
+    public void writeString16NoHelper(@Nullable String val) {
+        nativeWriteString16(mNativePtr, val);
    }

    /**
@@ -2337,7 +2369,17 @@ public final class Parcel {
     */
    @Nullable
    public final String readString() {
-        return mReadWriteHelper.readString(this);
+        return readString16();
+    }
+
+    /** {@hide} */
+    public final @Nullable String readString8() {
+        return mReadWriteHelper.readString8(this);
+    }
+
+    /** {@hide} */
+    public final @Nullable String readString16() {
+        return mReadWriteHelper.readString16(this);
    }

    /**
@@ -2347,9 +2389,18 @@ public final class Parcel {
     *
     * @hide
     */
-    @Nullable
-    public String readStringNoHelper() {
-        return nativeReadString(mNativePtr);
+    public @Nullable String readStringNoHelper() {
+        return readString16NoHelper();
+    }
+
+    /** {@hide} */
+    public @Nullable String readString8NoHelper() {
+        return nativeReadString8(mNativePtr);
+    }
+
+    /** {@hide} */
+    public @Nullable String readString16NoHelper() {
+        return nativeReadString16(mNativePtr);
    }

    /**
--- a/core/jni/android_os_Parcel.cpp
+++ b/core/jni/android_os_Parcel.cpp
@@ -273,7 +273,28 @@ static void android_os_Parcel_writeDouble(JNIEnv* env, jclass clazz, jlong nativ
    }
 }

-static void android_os_Parcel_writeString(JNIEnv* env, jclass clazz, jlong nativePtr, jstring val)
+static void android_os_Parcel_writeString8(JNIEnv* env, jclass clazz, jlong nativePtr, jstring val)
+{
+    Parcel* parcel = reinterpret_cast<Parcel*>(nativePtr);
+    if (parcel != NULL) {
+        status_t err = NO_MEMORY;
+        if (val) {
+            const size_t len = env->GetStringUTFLength(val);
+            const char* str = env->GetStringUTFChars(val, 0);
+            if (str) {
+                err = parcel->writeString8(str, len);
+                env->ReleaseStringUTFChars(val, str);
+            }
+        } else {
+            err = parcel->writeString8(NULL, 0);
+        }
+        if (err != NO_ERROR) {
+            signalExceptionForError(env, clazz, err);
+        }
+    }
+}
+
+static void android_os_Parcel_writeString16(JNIEnv* env, jclass clazz, jlong nativePtr, jstring val)
 {
    Parcel* parcel = reinterpret_cast<Parcel*>(nativePtr);
    if (parcel != NULL) {
@@ -444,7 +465,21 @@ static jdouble android_os_Parcel_readDouble(jlong nativePtr)
    return 0;
 }

-static jstring android_os_Parcel_readString(JNIEnv* env, jclass clazz, jlong nativePtr)
+static jstring android_os_Parcel_readString8(JNIEnv* env, jclass clazz, jlong nativePtr)
+{
+    Parcel* parcel = reinterpret_cast<Parcel*>(nativePtr);
+    if (parcel != NULL) {
+        size_t len;
+        const char* str = parcel->readString8Inplace(&len);
+        if (str) {
+            return env->NewStringUTF(str);
+        }
+        return NULL;
+    }
+    return NULL;
+}
+
+static jstring android_os_Parcel_readString16(JNIEnv* env, jclass clazz, jlong nativePtr)
 {
    Parcel* parcel = reinterpret_cast<Parcel*>(nativePtr);
    if (parcel != NULL) {
@@ -722,7 +757,9 @@ static const JNINativeMethod gParcelMethods[] = {
    // @FastNative
    {"nativeWriteDouble",         "(JD)V", (void*)android_os_Parcel_writeDouble},
    // @FastNative
-    {"nativeWriteString",         "(JLjava/lang/String;)V", (void*)android_os_Parcel_writeString},
+    {"nativeWriteString8",        "(JLjava/lang/String;)V", (void*)android_os_Parcel_writeString8},
+    // @FastNative
+    {"nativeWriteString16",       "(JLjava/lang/String;)V", (void*)android_os_Parcel_writeString16},
    // @FastNative
    {"nativeWriteStrongBinder",   "(JLandroid/os/IBinder;)V", (void*)android_os_Parcel_writeStrongBinder},
    // @FastNative
@@ -740,7 +777,9 @@ static const JNINativeMethod gParcelMethods[] = {
    // @CriticalNative
    {"nativeReadDouble",          "(J)D", (void*)android_os_Parcel_readDouble},
    // @FastNative
-    {"nativeReadString",          "(J)Ljava/lang/String;", (void*)android_os_Parcel_readString},
+    {"nativeReadString8",         "(J)Ljava/lang/String;", (void*)android_os_Parcel_readString8},
+    // @FastNative
+    {"nativeReadString16",        "(J)Ljava/lang/String;", (void*)android_os_Parcel_readString16},
    // @FastNative
    {"nativeReadStrongBinder",    "(J)Landroid/os/IBinder;", (void*)android_os_Parcel_readStrongBinder},
    // @FastNative
--- a/core/tests/benchmarks/src/android/os/ParcelStringBenchmark.java
+++ b/core/tests/benchmarks/src/android/os/ParcelStringBenchmark.java
@@ -0,0 +1,72 @@
+/*
+ * 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 android.os;
+
+import com.google.caliper.AfterExperiment;
+import com.google.caliper.BeforeExperiment;
+import com.google.caliper.Param;
+
+public class ParcelStringBenchmark {
+
+    @Param({"com.example.typical_package_name", "從不喜歡孤單一個 - 蘇永康／吳雨霏"})
+    String mValue;
+
+    private Parcel mParcel;
+
+    @BeforeExperiment
+    protected void setUp() {
+        mParcel = Parcel.obtain();
+    }
+
+    @AfterExperiment
+    protected void tearDown() {
+        mParcel.recycle();
+        mParcel = null;
+    }
+
+    public void timeWriteString8(int reps) {
+        for (int i = 0; i < reps; i++) {
+            mParcel.setDataPosition(0);
+            mParcel.writeString8(mValue);
+        }
+    }
+
+    public void timeReadString8(int reps) {
+        mParcel.writeString8(mValue);
+
+        for (int i = 0; i < reps; i++) {
+            mParcel.setDataPosition(0);
+            mParcel.readString8();
+        }
+    }
+
+    public void timeWriteString16(int reps) {
+        for (int i = 0; i < reps; i++) {
+            mParcel.setDataPosition(0);
+            mParcel.writeString16(mValue);
+        }
+    }
+
+    public void timeReadString16(int reps) {
+        mParcel.writeString16(mValue);
+
+        for (int i = 0; i < reps; i++) {
+            mParcel.setDataPosition(0);
+            mParcel.readString16();
+        }
+    }
+}
--- a/core/tests/coretests/src/android/os/ParcelTest.java
+++ b/core/tests/coretests/src/android/os/ParcelTest.java
@@ -87,4 +87,27 @@ public class ParcelTest {

        p.recycle();
    }
+
+    /**
+     * Verify that writing/reading UTF-8 and UTF-16 strings works well.
+     */
+    @Test
+    public void testStrings() {
+        final String[] strings = {
+                null, "", "abc\0def", "com.example.typical_package_name",
+                "從不喜歡孤單一個 - 蘇永康／吳雨霏", "example"
+        };
+
+        final Parcel p = Parcel.obtain();
+        for (String string : strings) {
+            p.writeString8(string);
+            p.writeString16(string);
+        }
+
+        p.setDataPosition(0);
+        for (String string : strings) {
+            assertEquals(string, p.readString8());
+            assertEquals(string, p.readString16());
+        }
+    }
 }