Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit abe50314 authored by Seigo Nonaka's avatar Seigo Nonaka
Browse files

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
parent 1e36211a

core/jni/android/graphics/FontFamily.cpp

+36 −20
Original line number Diff line number Diff line
@@ -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,7 +265,7 @@ 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});

}
@@ -258,8 +274,8 @@ static void FontFamily_addAxisValue(jlong builderPtr, jint tag, jfloat 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 },

    { "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 },

core/jni/android/graphics/Typeface.cpp

+32 −26
Original line number Diff line number Diff line
@@ -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",

graphics/java/android/graphics/FontFamily.java

+19 −17
Original line number Diff line number Diff line
@@ -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,

graphics/java/android/graphics/Typeface.java

+20 −14
Original line number Diff line number Diff line
@@ -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);

    }
@@ -1119,16 +1127,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;
@@ -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();

}