From f32adf447511d54c2aa0948d3c1ef44d461538ac Mon Sep 17 00:00:00 2001 From: John Reck Date: Wed, 23 Nov 2016 10:39:40 -0800 Subject: [PATCH] Clean up ApplyStyle JNI Bug: 32573798 Mark input uint32_t[] as const. Use Read-only JNI array access for input as it's faster than critical access. Use non-movable arrays for TypedArray so that the address can be resolved and stored, avoiding the need to do JNI array access for the output. Indicies is always non-null, so remove the optional checks. Eliminate unused return value. Benchmark results: twelveKeyInflate 4963us -> 4713us simpleViewInflate 73us -> 60us Test: Device boots, benchmarks show faster Change-Id: Ic3bde5aee31407d8903913f97f2218daf074499a --- .../android/content/res/AssetManager.java | 4 +- .../android/content/res/ResourcesImpl.java | 2 +- core/java/android/content/res/TypedArray.java | 52 +++++++------- core/jni/android_util_AssetManager.cpp | 70 +++---------------- libs/androidfw/AttributeResolution.cpp | 13 ++-- .../include/androidfw/AttributeResolution.h | 6 +- .../tests/AttributeResolution_test.cpp | 6 +- .../android/content/res/BridgeTypedArray.java | 2 +- 8 files changed, 53 insertions(+), 102 deletions(-) diff --git a/core/java/android/content/res/AssetManager.java b/core/java/android/content/res/AssetManager.java index b0d0d798289ba..db24ffef99cfe 100644 --- a/core/java/android/content/res/AssetManager.java +++ b/core/java/android/content/res/AssetManager.java @@ -815,9 +815,9 @@ public final class AssetManager implements AutoCloseable { /*package*/ static final int STYLE_DENSITY = 5; @FastNative - /*package*/ native static final boolean applyStyle(long theme, + /*package*/ native static final void applyStyle(long theme, int defStyleAttr, int defStyleRes, long xmlParser, - int[] inAttrs, int[] outValues, int[] outIndices); + int[] inAttrs, int length, long outValuesAddress, long outIndicesAddress); @FastNative /*package*/ native static final boolean resolveAttrs(long theme, int defStyleAttr, int defStyleRes, int[] inValues, diff --git a/core/java/android/content/res/ResourcesImpl.java b/core/java/android/content/res/ResourcesImpl.java index c46fe29d07a5b..eb010e46ea910 100644 --- a/core/java/android/content/res/ResourcesImpl.java +++ b/core/java/android/content/res/ResourcesImpl.java @@ -1126,7 +1126,7 @@ public class ResourcesImpl { final XmlBlock.Parser parser = (XmlBlock.Parser) set; AssetManager.applyStyle(mTheme, defStyleAttr, defStyleRes, parser != null ? parser.mParseState : 0, - attrs, array.mData, array.mIndices); + attrs, attrs.length, array.mDataAddress, array.mIndicesAddress); array.mTheme = wrapper; array.mXml = parser; diff --git a/core/java/android/content/res/TypedArray.java b/core/java/android/content/res/TypedArray.java index 1e44a5ccafc93..ca95ce19fa9c9 100644 --- a/core/java/android/content/res/TypedArray.java +++ b/core/java/android/content/res/TypedArray.java @@ -30,6 +30,8 @@ import android.util.TypedValue; import com.android.internal.util.XmlUtils; +import dalvik.system.VMRuntime; + import java.util.Arrays; /** @@ -44,28 +46,17 @@ import java.util.Arrays; public class TypedArray { static TypedArray obtain(Resources res, int len) { - final TypedArray attrs = res.mTypedArrayPool.acquire(); - if (attrs != null) { - attrs.mLength = len; - attrs.mRecycled = false; - - // Reset the assets, which may have changed due to configuration changes - // or further resource loading. - attrs.mAssets = res.getAssets(); - - final int fullLen = len * AssetManager.STYLE_NUM_ENTRIES; - if (attrs.mData.length >= fullLen) { - return attrs; - } - - attrs.mData = new int[fullLen]; - attrs.mIndices = new int[1 + len]; - return attrs; + TypedArray attrs = res.mTypedArrayPool.acquire(); + if (attrs == null) { + attrs = new TypedArray(res); } - return new TypedArray(res, - new int[len*AssetManager.STYLE_NUM_ENTRIES], - new int[1+len], len); + attrs.mRecycled = false; + // Reset the assets, which may have changed due to configuration changes + // or further resource loading. + attrs.mAssets = res.getAssets(); + attrs.resize(len); + return attrs; } private final Resources mResources; @@ -77,10 +68,25 @@ public class TypedArray { /*package*/ XmlBlock.Parser mXml; /*package*/ Resources.Theme mTheme; /*package*/ int[] mData; + /*package*/ long mDataAddress; /*package*/ int[] mIndices; + /*package*/ long mIndicesAddress; /*package*/ int mLength; /*package*/ TypedValue mValue = new TypedValue(); + private void resize(int len) { + mLength = len; + final int dataLen = len * AssetManager.STYLE_NUM_ENTRIES; + final int indicesLen = len + 1; + final VMRuntime runtime = VMRuntime.getRuntime(); + if (mData == null || mData.length < dataLen) { + mData = (int[]) runtime.newNonMovableArray(int.class, dataLen); + mDataAddress = runtime.addressOf(mData); + mIndices = (int[]) runtime.newNonMovableArray(int.class, indicesLen); + mIndicesAddress = runtime.addressOf(mIndices); + } + } + /** * Returns the number of values in this array. * @@ -1217,13 +1223,11 @@ public class TypedArray { return mAssets.getPooledStringForCookie(cookie, data[index+AssetManager.STYLE_DATA]); } - /*package*/ TypedArray(Resources resources, int[] data, int[] indices, int len) { + /** @hide */ + protected TypedArray(Resources resources) { mResources = resources; mMetrics = mResources.getDisplayMetrics(); mAssets = mResources.getAssets(); - mData = data; - mIndices = indices; - mLength = len; } @Override diff --git a/core/jni/android_util_AssetManager.cpp b/core/jni/android_util_AssetManager.cpp index d70fbb9d3915a..d382f245e0d78 100644 --- a/core/jni/android_util_AssetManager.cpp +++ b/core/jni/android_util_AssetManager.cpp @@ -1179,67 +1179,17 @@ static jboolean android_content_AssetManager_resolveAttrs(JNIEnv* env, jobject c return result ? JNI_TRUE : JNI_FALSE; } -static jboolean android_content_AssetManager_applyStyle(JNIEnv* env, jobject clazz, - jlong themeToken, - jint defStyleAttr, - jint defStyleRes, - jlong xmlParserToken, - jintArray attrs, - jintArray outValues, - jintArray outIndices) -{ - if (themeToken == 0) { - jniThrowNullPointerException(env, "theme token"); - return JNI_FALSE; - } - if (attrs == NULL) { - jniThrowNullPointerException(env, "attrs"); - return JNI_FALSE; - } - if (outValues == NULL) { - jniThrowNullPointerException(env, "out values"); - return JNI_FALSE; - } - - const jsize NI = env->GetArrayLength(attrs); - const jsize NV = env->GetArrayLength(outValues); - if (NV < (NI*STYLE_NUM_ENTRIES)) { - jniThrowException(env, "java/lang/IndexOutOfBoundsException", "out values too small"); - return JNI_FALSE; - } - - jint* src = (jint*)env->GetPrimitiveArrayCritical(attrs, 0); - if (src == NULL) { - return JNI_FALSE; - } - - jint* baseDest = (jint*)env->GetPrimitiveArrayCritical(outValues, 0); - if (baseDest == NULL) { - env->ReleasePrimitiveArrayCritical(attrs, src, 0); - return JNI_FALSE; - } - - jint* indices = NULL; - if (outIndices != NULL) { - if (env->GetArrayLength(outIndices) > NI) { - indices = (jint*)env->GetPrimitiveArrayCritical(outIndices, 0); - } - } - +static void android_content_AssetManager_applyStyle(JNIEnv* env, jobject, jlong themeToken, + jint defStyleAttr, jint defStyleRes, jlong xmlParserToken, jintArray attrsObj, jint length, + jlong outValuesAddress, jlong outIndicesAddress) { + jint* attrs = env->GetIntArrayElements(attrsObj, 0); ResTable::Theme* theme = reinterpret_cast(themeToken); ResXMLParser* xmlParser = reinterpret_cast(xmlParserToken); - bool result = ApplyStyle(theme, xmlParser, - defStyleAttr, defStyleRes, - (uint32_t*) src, NI, - (uint32_t*) baseDest, - (uint32_t*) indices); - - if (indices != NULL) { - env->ReleasePrimitiveArrayCritical(outIndices, indices, 0); - } - env->ReleasePrimitiveArrayCritical(outValues, baseDest, 0); - env->ReleasePrimitiveArrayCritical(attrs, src, 0); - return result ? JNI_TRUE : JNI_FALSE; + uint32_t* outValues = reinterpret_cast(static_cast(outValuesAddress)); + uint32_t* outIndices = reinterpret_cast(static_cast(outIndicesAddress)); + ApplyStyle(theme, xmlParser, defStyleAttr, defStyleRes, + reinterpret_cast(attrs), length, outValues, outIndices); + env->ReleaseIntArrayElements(attrsObj, attrs, JNI_ABORT); } static jboolean android_content_AssetManager_retrieveAttributes(JNIEnv* env, jobject clazz, @@ -1795,7 +1745,7 @@ static const JNINativeMethod gAssetManagerMethods[] = { { "dumpTheme", "(JILjava/lang/String;Ljava/lang/String;)V", (void*) android_content_AssetManager_dumpTheme }, // @FastNative - { "applyStyle","(JIIJ[I[I[I)Z", + { "applyStyle","(JIIJ[IIJJ)V", (void*) android_content_AssetManager_applyStyle }, // @FastNative { "resolveAttrs","(JII[I[I[I[I)Z", diff --git a/libs/androidfw/AttributeResolution.cpp b/libs/androidfw/AttributeResolution.cpp index 00f7a42f10e47..d71fc39802aab 100644 --- a/libs/androidfw/AttributeResolution.cpp +++ b/libs/androidfw/AttributeResolution.cpp @@ -193,9 +193,9 @@ bool ResolveAttrs(ResTable::Theme* theme, uint32_t def_style_attr, uint32_t def_ return true; } -bool ApplyStyle(ResTable::Theme* theme, ResXMLParser* xml_parser, uint32_t def_style_attr, - uint32_t def_style_res, uint32_t* attrs, size_t attrs_length, uint32_t* out_values, - uint32_t* out_indices) { +void ApplyStyle(ResTable::Theme* theme, ResXMLParser* xml_parser, uint32_t def_style_attr, + uint32_t def_style_res, const uint32_t* attrs, size_t attrs_length, + uint32_t* out_values, uint32_t* out_indices) { if (kDebugStyles) { ALOGI("APPLY STYLE: theme=0x%p defStyleAttr=0x%x defStyleRes=0x%x xml=0x%p", theme, def_style_attr, def_style_res, xml_parser); @@ -376,7 +376,7 @@ bool ApplyStyle(ResTable::Theme* theme, ResXMLParser* xml_parser, uint32_t def_s out_values[STYLE_CHANGING_CONFIGURATIONS] = type_set_flags; out_values[STYLE_DENSITY] = config.density; - if (out_indices != NULL && value.dataType != Res_value::TYPE_NULL) { + if (value.dataType != Res_value::TYPE_NULL) { indices_idx++; out_indices[indices_idx] = ii; } @@ -386,10 +386,7 @@ bool ApplyStyle(ResTable::Theme* theme, ResXMLParser* xml_parser, uint32_t def_s res.unlock(); - if (out_indices != NULL) { - out_indices[0] = indices_idx; - } - return true; + out_indices[0] = indices_idx; } bool RetrieveAttributes(const ResTable* res, ResXMLParser* xml_parser, uint32_t* attrs, diff --git a/libs/androidfw/include/androidfw/AttributeResolution.h b/libs/androidfw/include/androidfw/AttributeResolution.h index 3ed8bced22ef8..8d5ff461444ed 100644 --- a/libs/androidfw/include/androidfw/AttributeResolution.h +++ b/libs/androidfw/include/androidfw/AttributeResolution.h @@ -44,9 +44,9 @@ bool ResolveAttrs(ResTable::Theme* theme, uint32_t def_style_attr, uint32_t def_ uint32_t* src_values, size_t src_values_length, uint32_t* attrs, size_t attrs_length, uint32_t* out_values, uint32_t* out_indices); -bool ApplyStyle(ResTable::Theme* theme, ResXMLParser* xml_parser, uint32_t def_style_attr, - uint32_t def_style_res, uint32_t* attrs, size_t attrs_length, uint32_t* out_values, - uint32_t* out_indices); +void ApplyStyle(ResTable::Theme* theme, ResXMLParser* xml_parser, uint32_t def_style_attr, + uint32_t def_style_res, const uint32_t* attrs, size_t attrs_length, + uint32_t* out_values, uint32_t* out_indices); bool RetrieveAttributes(const ResTable* res, ResXMLParser* xml_parser, uint32_t* attrs, size_t attrs_length, uint32_t* out_values, uint32_t* out_indices); diff --git a/libs/androidfw/tests/AttributeResolution_test.cpp b/libs/androidfw/tests/AttributeResolution_test.cpp index d6d7890882a81..3aac17838e2a7 100644 --- a/libs/androidfw/tests/AttributeResolution_test.cpp +++ b/libs/androidfw/tests/AttributeResolution_test.cpp @@ -161,9 +161,9 @@ TEST_F(AttributeResolutionXmlTest, ThemeAndXmlParser) { std::vector values; values.resize(arraysize(attrs) * 6); - ASSERT_TRUE(ApplyStyle(&theme, &xml_parser_, 0 /*def_style_attr*/, - 0 /*def_style_res*/, attrs, arraysize(attrs), - values.data(), nullptr /*out_indices*/)); + ApplyStyle(&theme, &xml_parser_, 0 /*def_style_attr*/, + 0 /*def_style_res*/, attrs, arraysize(attrs), + values.data(), nullptr /*out_indices*/); const uint32_t public_flag = ResTable_typeSpec::SPEC_PUBLIC; diff --git a/tools/layoutlib/bridge/src/android/content/res/BridgeTypedArray.java b/tools/layoutlib/bridge/src/android/content/res/BridgeTypedArray.java index 9da65a60a3191..35cf9038f9ae1 100644 --- a/tools/layoutlib/bridge/src/android/content/res/BridgeTypedArray.java +++ b/tools/layoutlib/bridge/src/android/content/res/BridgeTypedArray.java @@ -80,7 +80,7 @@ public final class BridgeTypedArray extends TypedArray { public BridgeTypedArray(Resources resources, BridgeContext context, int len, boolean platformFile) { - super(resources, null, null, 0); + super(resources); mBridgeResources = resources; mContext = context; mPlatformFile = platformFile;