Stop using finalizer in Typeface and FontFamily

Use NativeAllocationRegistry instead.

Verified this doesn't cause performance regressions:

StaticLayout creation time:
 MeasuredText Balanced Hyphenation  :    721,055 ->    723,812: (+0.4%)
 MeasuredText Balanced NoHyphenation:    527,880 ->    505,843: (-4.2%)
 MeasuredText Greedy Hyphenation    :    476,982 ->    460,241: (-3.5%)
 MeasuredText Greedy NoHyphenation  :    471,074 ->    459,013: (-2.6%)
 RandomText Balanced Hyphenation    : 18,566,915 -> 18,514,633: (-0.3%)
 RandomText Balanced NoHyphenation  :  7,697,772 ->  7,658,458: (-0.5%)
 RandomText Greedy Hyphenation      :  7,648,606 ->  7,602,026: (-0.6%)
 RandomText Greedy NoHyphenation    :  7,643,946 ->  7,618,898: (-0.3%)

MeasuredText creation time:
 NoStyled Hyphenation               : 18,377,293 -> 18,521,296: (+0.8%)
 NoStyled Hyphenation WidthOnly     : 17,852,099 -> 17,889,073: (+0.2%)
 NoStyled NoHyphenation             :  7,608,972 ->  7,614,311: (+0.1%)
 NoStyled NoHyphenation WidthOnly   :  7,194,461 ->  7,192,043: (-0.0%)
 Styled Hyphenation                 : 15,396,698 -> 15,352,501: (-0.3%)
 Styled Hyphenation WidthOnly       : 14,356,020 -> 14,379,205: (+0.2%)
 Styled NoHyphenation               : 14,926,657 -> 14,971,723: (+0.3%)
 Styled NoHyphenation WidthOnly     : 13,964,537 -> 13,874,324: (-0.6%)

StaticLayout draw time:
 MeasuredText NoStyled              :    644,795 ->    635,241: (-1.5%)
 MeasuredText NoStyled WithoutCache :    637,943 ->    616,369: (-3.4%)
 MeasuredText Styled                :    866,664 ->    860,322: (-0.7%)
 MeasuredText Styled WithoutCache   :    917,795 ->    897,164: (-2.2%)
 RandomText NoStyled                :    538,196 ->    533,253: (-0.9%)
 RandomText NoStyled WithoutCache   :  7,012,076 ->  7,047,498: (+0.5%)
 RandomText Styled                  :  3,011,941 ->  2,999,661: (-0.4%)
 RandomText Styled WithoutCache     :  3,486,225 ->  3,462,493: (-0.7%)

Bug: 70185027
Test: atest CtsWidgetTestCases:EditTextTest
    CtsWidgetTestCases:TextViewFadingEdgeTest
    FrameworksCoreTests:TextViewFallbackLineSpacingTest
    FrameworksCoreTests:TextViewTest FrameworksCoreTests:TypefaceTest
    CtsGraphicsTestCases:TypefaceTest CtsWidgetTestCases:TextViewTest
    CtsTextTestCases

Change-Id: I7ebb8a219a8eb07c8ad9fa432e8454e4e06be386

diff --git a/core/jni/android/graphics/FontFamily.cpp b/core/jni/android/graphics/FontFamily.cpp
index ed032c7..c7ad2a4 100644 (file)
--- a/core/jni/android/graphics/FontFamily.cpp
+++ b/core/jni/android/graphics/FontFamily.cpp
@@ -51,6 +51,18 @@ struct NativeFamilyBuilder {
     std::vector<minikin::FontVariation> axes;
 };
 
+static inline NativeFamilyBuilder* toNativeBuilder(jlong ptr) {
+    return reinterpret_cast<NativeFamilyBuilder*>(ptr);
+}
+
+static inline FontFamilyWrapper* toFamily(jlong ptr) {
+    return reinterpret_cast<FontFamilyWrapper*>(ptr);
+}
+
+template<typename Ptr> static inline jlong toJLong(Ptr ptr) {
+    return reinterpret_cast<jlong>(ptr);
+}
+
 static jlong FontFamily_initBuilder(JNIEnv* env, jobject clazz, jstring langs, jint variant) {
     NativeFamilyBuilder* builder;
     if (langs != nullptr) {
@@ -59,15 +71,14 @@ static jlong FontFamily_initBuilder(JNIEnv* env, jobject clazz, jstring langs, j
     } else {
         builder = new NativeFamilyBuilder(minikin::registerLocaleList(""), variant);
     }
-    return reinterpret_cast<jlong>(builder);
+    return toJLong(builder);
 }
 
 static jlong FontFamily_create(jlong builderPtr) {
     if (builderPtr == 0) {
         return 0;
     }
-    std::unique_ptr<NativeFamilyBuilder> builder(
-            reinterpret_cast<NativeFamilyBuilder*>(builderPtr));
+    NativeFamilyBuilder* builder = toNativeBuilder(builderPtr);
     if (builder->fonts.empty()) {
         return 0;
     }
@@ -76,17 +87,23 @@ static jlong FontFamily_create(jlong builderPtr) {
     if (family->getCoverage().length() == 0) {
         return 0;
     }
-    return reinterpret_cast<jlong>(new FontFamilyWrapper(std::move(family)));
+    return toJLong(new FontFamilyWrapper(std::move(family)));
 }
 
-static void FontFamily_abort(jlong builderPtr) {
-    NativeFamilyBuilder* builder = reinterpret_cast<NativeFamilyBuilder*>(builderPtr);
-    delete builder;
+static void releaseBuilder(jlong builderPtr) {
+    delete toNativeBuilder(builderPtr);
 }
 
-static void FontFamily_unref(jlong familyPtr) {
-    FontFamilyWrapper* family = reinterpret_cast<FontFamilyWrapper*>(familyPtr);
-    delete family;
+static jlong FontFamily_getBuilderReleaseFunc() {
+    return toJLong(&releaseBuilder);
+}
+
+static void releaseFamily(jlong familyPtr) {
+    delete toFamily(familyPtr);
+}
+
+static jlong FontFamily_getFamilyReleaseFunc() {
+    return toJLong(&releaseFamily);
 }
 
 static bool addSkTypeface(NativeFamilyBuilder* builder, sk_sp<SkData>&& data, int ttcIndex,
@@ -175,7 +192,7 @@ static jboolean FontFamily_addFont(JNIEnv* env, jobject clazz, jlong builderPtr,
 static jboolean FontFamily_addFontWeightStyle(JNIEnv* env, jobject clazz, jlong builderPtr,
         jobject font, jint ttcIndex, jint weight, jint isItalic) {
     NPE_CHECK_RETURN_ZERO(env, font);
-    NativeFamilyBuilder* builder = reinterpret_cast<NativeFamilyBuilder*>(builderPtr);
+    NativeFamilyBuilder* builder = toNativeBuilder(builderPtr);
     const void* fontPtr = env->GetDirectBufferAddress(font);
     if (fontPtr == NULL) {
         ALOGE("addFont failed to create font, buffer invalid");
@@ -204,8 +221,7 @@ static jboolean FontFamily_addFontFromAssetManager(JNIEnv* env, jobject, jlong b
     NPE_CHECK_RETURN_ZERO(env, jassetMgr);
     NPE_CHECK_RETURN_ZERO(env, jpath);
 
-    NativeFamilyBuilder* builder = reinterpret_cast<NativeFamilyBuilder*>(builderPtr);
-
+    NativeFamilyBuilder* builder = toNativeBuilder(builderPtr);
     Guarded<AssetManager2>* mgr = AssetManagerForJavaObject(env, jassetMgr);
     if (NULL == mgr) {
         builder->axes.clear();
@@ -249,19 +265,19 @@ static jboolean FontFamily_addFontFromAssetManager(JNIEnv* env, jobject, jlong b
 }
 
 static void FontFamily_addAxisValue(jlong builderPtr, jint tag, jfloat value) {
-    NativeFamilyBuilder* builder = reinterpret_cast<NativeFamilyBuilder*>(builderPtr);
+    NativeFamilyBuilder* builder = toNativeBuilder(builderPtr);
     builder->axes.push_back({static_cast<minikin::AxisTag>(tag), value});
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 
 static const JNINativeMethod gFontFamilyMethods[] = {
-    { "nInitBuilder",          "(Ljava/lang/String;I)J", (void*)FontFamily_initBuilder },
-    { "nCreateFamily",         "(J)J", (void*)FontFamily_create },
-    { "nAbort",                "(J)V", (void*)FontFamily_abort },
-    { "nUnrefFamily",          "(J)V", (void*)FontFamily_unref },
-    { "nAddFont",              "(JLjava/nio/ByteBuffer;III)Z", (void*)FontFamily_addFont },
-    { "nAddFontWeightStyle",   "(JLjava/nio/ByteBuffer;III)Z",
+    { "nInitBuilder",           "(Ljava/lang/String;I)J", (void*)FontFamily_initBuilder },
+    { "nCreateFamily",          "(J)J", (void*)FontFamily_create },
+    { "nGetBuilderReleaseFunc", "()J", (void*)FontFamily_getBuilderReleaseFunc },
+    { "nGetFamilyReleaseFunc",  "()J", (void*)FontFamily_getFamilyReleaseFunc },
+    { "nAddFont",               "(JLjava/nio/ByteBuffer;III)Z", (void*)FontFamily_addFont },
+    { "nAddFontWeightStyle",    "(JLjava/nio/ByteBuffer;III)Z",
             (void*)FontFamily_addFontWeightStyle },
     { "nAddFontFromAssetManager",    "(JLandroid/content/res/AssetManager;Ljava/lang/String;IZIII)Z",
             (void*)FontFamily_addFontFromAssetManager },

diff --git a/core/jni/android/graphics/Typeface.cpp b/core/jni/android/graphics/Typeface.cpp
index d67c0b0..34ec365 100644 (file)
--- a/core/jni/android/graphics/Typeface.cpp
+++ b/core/jni/android/graphics/Typeface.cpp
@@ -28,8 +28,16 @@
 
 using namespace android;
 
+static inline Typeface* toTypeface(jlong ptr) {
+    return reinterpret_cast<Typeface*>(ptr);
+}
+
+template<typename Ptr> static inline jlong toJLong(Ptr ptr) {
+    return reinterpret_cast<jlong>(ptr);
+}
+
 static jlong Typeface_createFromTypeface(JNIEnv* env, jobject, jlong familyHandle, jint style) {
-    Typeface* family = reinterpret_cast<Typeface*>(familyHandle);
+    Typeface* family = toTypeface(familyHandle);
     Typeface* face = Typeface::createRelative(family, (Typeface::Style)style);
     // TODO: the following logic shouldn't be necessary, the above should always succeed.
     // Try to find the closest matching font, using the standard heuristic
@@ -39,13 +47,12 @@ static jlong Typeface_createFromTypeface(JNIEnv* env, jobject, jlong familyHandl
     for (int i = 0; NULL == face && i < 4; i++) {
         face = Typeface::createRelative(family, (Typeface::Style)i);
     }
-    return reinterpret_cast<jlong>(face);
+    return toJLong(face);
 }
 
 static jlong Typeface_createFromTypefaceWithExactStyle(JNIEnv* env, jobject, jlong nativeInstance,
         jint weight, jboolean italic) {
-    Typeface* baseTypeface = reinterpret_cast<Typeface*>(nativeInstance);
-    return reinterpret_cast<jlong>(Typeface::createAbsolute(baseTypeface, weight, italic));
+    return toJLong(Typeface::createAbsolute(toTypeface(nativeInstance), weight, italic));
 }
 
 static jlong Typeface_createFromTypefaceWithVariation(JNIEnv* env, jobject, jlong familyHandle,
@@ -60,30 +67,30 @@ static jlong Typeface_createFromTypefaceWithVariation(JNIEnv* env, jobject, jlon
         AxisHelper axis(env, axisObject);
         variations.push_back(minikin::FontVariation(axis.getTag(), axis.getStyleValue()));
     }
-    Typeface* baseTypeface = reinterpret_cast<Typeface*>(familyHandle);
-    Typeface* result = Typeface::createFromTypefaceWithVariation(baseTypeface, variations);
-    return reinterpret_cast<jlong>(result);
+    return toJLong(Typeface::createFromTypefaceWithVariation(toTypeface(familyHandle), variations));
 }
 
 static jlong Typeface_createWeightAlias(JNIEnv* env, jobject, jlong familyHandle, jint weight) {
-    Typeface* family = reinterpret_cast<Typeface*>(familyHandle);
-    Typeface* face = Typeface::createWithDifferentBaseWeight(family, weight);
-    return reinterpret_cast<jlong>(face);
+    return toJLong(Typeface::createWithDifferentBaseWeight(toTypeface(familyHandle), weight));
+}
+
+static void releaseFunc(jlong ptr) {
+    delete toTypeface(ptr);
 }
 
-static void Typeface_unref(JNIEnv* env, jobject obj, jlong faceHandle) {
-    Typeface* face = reinterpret_cast<Typeface*>(faceHandle);
-    delete face;
+// CriticalNative
+static jlong Typeface_getReleaseFunc() {
+    return toJLong(&releaseFunc);
 }
 
-static jint Typeface_getStyle(JNIEnv* env, jobject obj, jlong faceHandle) {
-    Typeface* face = reinterpret_cast<Typeface*>(faceHandle);
-    return face->fAPIStyle;
+// CriticalNative
+static jint Typeface_getStyle(jlong faceHandle) {
+    return toTypeface(faceHandle)->fAPIStyle;
 }
 
-static jint Typeface_getWeight(JNIEnv* env, jobject obj, jlong faceHandle) {
-    Typeface* face = reinterpret_cast<Typeface*>(faceHandle);
-    return face->fStyle.weight();
+// CriticalNative
+static jint Typeface_getWeight(jlong faceHandle) {
+    return toTypeface(faceHandle)->fStyle.weight();
 }
 
 static jlong Typeface_createFromArray(JNIEnv *env, jobject, jlongArray familyArray,
@@ -95,17 +102,16 @@ static jlong Typeface_createFromArray(JNIEnv *env, jobject, jlongArray familyArr
         FontFamilyWrapper* family = reinterpret_cast<FontFamilyWrapper*>(families[i]);
         familyVec.emplace_back(family->family);
     }
-    return reinterpret_cast<jlong>(
-            Typeface::createFromFamilies(std::move(familyVec), weight, italic));
+    return toJLong(Typeface::createFromFamilies(std::move(familyVec), weight, italic));
 }
 
-static void Typeface_setDefault(JNIEnv *env, jobject, jlong faceHandle) {
-    Typeface* face = reinterpret_cast<Typeface*>(faceHandle);
-    Typeface::setDefault(face);
+// CriticalNative
+static void Typeface_setDefault(jlong faceHandle) {
+    Typeface::setDefault(toTypeface(faceHandle));
 }
 
 static jobject Typeface_getSupportedAxes(JNIEnv *env, jobject, jlong faceHandle) {
-    Typeface* face = reinterpret_cast<Typeface*>(faceHandle);
+    Typeface* face = toTypeface(faceHandle);
     const std::unordered_set<minikin::AxisTag>& tagSet = face->fFontCollection->getSupportedTags();
     const size_t length = tagSet.size();
     if (length == 0) {
@@ -131,7 +137,7 @@ static const JNINativeMethod gTypefaceMethods[] = {
     { "nativeCreateFromTypefaceWithVariation", "(JLjava/util/List;)J",
             (void*)Typeface_createFromTypefaceWithVariation },
     { "nativeCreateWeightAlias",  "(JI)J", (void*)Typeface_createWeightAlias },
-    { "nativeUnref",              "(J)V",  (void*)Typeface_unref },
+    { "nativeGetReleaseFunc",     "()J",  (void*)Typeface_getReleaseFunc },
     { "nativeGetStyle",           "(J)I",  (void*)Typeface_getStyle },
     { "nativeGetWeight",      "(J)I",  (void*)Typeface_getWeight },
     { "nativeCreateFromArray",    "([JII)J",

diff --git a/graphics/java/android/graphics/FontFamily.java b/graphics/java/android/graphics/FontFamily.java
index e7cfcfd..c69eb32 100644 (file)
--- a/graphics/java/android/graphics/FontFamily.java
+++ b/graphics/java/android/graphics/FontFamily.java
@@ -24,6 +24,8 @@ import android.util.Log;
 
 import dalvik.annotation.optimization.CriticalNative;
 
+import libcore.util.NativeAllocationRegistry;
+
 import java.io.FileInputStream;
 import java.io.IOException;
 import java.nio.ByteBuffer;
@@ -38,6 +40,14 @@ public class FontFamily {
 
     private static String TAG = "FontFamily";
 
+    private static final NativeAllocationRegistry sBuilderRegistry = new NativeAllocationRegistry(
+            FontFamily.class.getClassLoader(), nGetBuilderReleaseFunc(), 64);
+
+    private @Nullable Runnable mNativeBuilderCleaner;
+
+    private static final NativeAllocationRegistry sFamilyRegistry = new NativeAllocationRegistry(
+            FontFamily.class.getClassLoader(), nGetFamilyReleaseFunc(), 64);
+
     /**
      * @hide
      */
@@ -48,6 +58,7 @@ public class FontFamily {
 
     public FontFamily() {
         mBuilderPtr = nInitBuilder(null, 0);
+        mNativeBuilderCleaner = sBuilderRegistry.registerNativeAllocation(this, mBuilderPtr);
     }
 
     public FontFamily(@Nullable String[] langs, int variant) {
@@ -60,6 +71,7 @@ public class FontFamily {
             langsString = TextUtils.join(",", langs);
         }
         mBuilderPtr = nInitBuilder(langsString, variant);
+        mNativeBuilderCleaner = sBuilderRegistry.registerNativeAllocation(this, mBuilderPtr);
     }
 
     /**
@@ -73,7 +85,11 @@ public class FontFamily {
             throw new IllegalStateException("This FontFamily is already frozen");
         }
         mNativePtr = nCreateFamily(mBuilderPtr);
+        mNativeBuilderCleaner.run();
         mBuilderPtr = 0;
+        if (mNativePtr != 0) {
+            sFamilyRegistry.registerNativeAllocation(this, mNativePtr);
+        }
         return mNativePtr != 0;
     }
 
@@ -81,24 +97,10 @@ public class FontFamily {
         if (mBuilderPtr == 0) {
             throw new IllegalStateException("This FontFamily is already frozen or abandoned");
         }
-        nAbort(mBuilderPtr);
+        mNativeBuilderCleaner.run();
         mBuilderPtr = 0;
     }
 
-    @Override
-    protected void finalize() throws Throwable {
-        try {
-            if (mNativePtr != 0) {
-                nUnrefFamily(mNativePtr);
-            }
-            if (mBuilderPtr != 0) {
-                nAbort(mBuilderPtr);
-            }
-        } finally {
-            super.finalize();
-        }
-    }
-
     public boolean addFont(String path, int ttcIndex, FontVariationAxis[] axes, int weight,
             int italic) {
         if (mBuilderPtr == 0) {
@@ -171,10 +173,10 @@ public class FontFamily {
     private static native long nCreateFamily(long mBuilderPtr);
 
     @CriticalNative
-    private static native void nAbort(long mBuilderPtr);
+    private static native long nGetBuilderReleaseFunc();
 
     @CriticalNative
-    private static native void nUnrefFamily(long nativePtr);
+    private static native long nGetFamilyReleaseFunc();
     // By passing -1 to weigth argument, the weight value is resolved by OS/2 table in the font.
     // By passing -1 to italic argument, the italic value is resolved by OS/2 table in the font.
     private static native boolean nAddFont(long builderPtr, ByteBuffer font, int ttcIndex,

diff --git a/graphics/java/android/graphics/Typeface.java b/graphics/java/android/graphics/Typeface.java
index 38beebd..f27c11d 100644 (file)
--- a/graphics/java/android/graphics/Typeface.java
+++ b/graphics/java/android/graphics/Typeface.java
@@ -42,6 +42,10 @@ import com.android.internal.annotations.GuardedBy;
 import com.android.internal.annotations.VisibleForTesting;
 import com.android.internal.util.Preconditions;
 
+import dalvik.annotation.optimization.CriticalNative;
+
+import libcore.util.NativeAllocationRegistry;
+
 import org.xmlpull.v1.XmlPullParserException;
 
 import java.io.File;
@@ -71,6 +75,9 @@ public class Typeface {
 
     private static String TAG = "Typeface";
 
+    private static final NativeAllocationRegistry sRegistry = new NativeAllocationRegistry(
+            Typeface.class.getClassLoader(), nativeGetReleaseFunc(), 64);
+
     /** The default NORMAL typeface object */
     public static final Typeface DEFAULT;
     /**
@@ -911,6 +918,7 @@ public class Typeface {
         }
 
         native_instance = ni;
+        sRegistry.registerNativeAllocation(this, native_instance);
         mStyle = nativeGetStyle(ni);
         mWeight = nativeGetWeight(ni);
     }
@@ -1120,16 +1128,6 @@ public class Typeface {
     }
 
     @Override
-    protected void finalize() throws Throwable {
-        try {
-            nativeUnref(native_instance);
-            native_instance = 0;  // Other finalizers can still call us.
-        } finally {
-            super.finalize();
-        }
-    }
-
-    @Override
     public boolean equals(Object o) {
         if (this == o) return true;
         if (o == null || getClass() != o.getClass()) return false;
@@ -1173,10 +1171,18 @@ public class Typeface {
     private static native long nativeCreateFromTypefaceWithVariation(
             long native_instance, List<FontVariationAxis> axes);
     private static native long nativeCreateWeightAlias(long native_instance, int weight);
-    private static native void nativeUnref(long native_instance);
-    private static native int  nativeGetStyle(long native_instance);
-    private static native int  nativeGetWeight(long native_instance);
     private static native long nativeCreateFromArray(long[] familyArray, int weight, int italic);
-    private static native void nativeSetDefault(long native_instance);
     private static native int[] nativeGetSupportedAxes(long native_instance);
+
+    @CriticalNative
+    private static native void nativeSetDefault(long nativePtr);
+
+    @CriticalNative
+    private static native int  nativeGetStyle(long nativePtr);
+
+    @CriticalNative
+    private static native int  nativeGetWeight(long nativePtr);
+
+    @CriticalNative
+    private static native long nativeGetReleaseFunc();
 }