From 7891805d22e3fdc821961ff0ccc5c450c3d625c8 Mon Sep 17 00:00:00 2001 From: Kathy Gray Date: Wed, 17 Jan 2018 10:35:18 -0800 Subject: [PATCH] move native accesses of map into two accesses instead of per-element Differential Revision: D6650192 fbshipit-source-id: 0eb0719fd3b7813add33a3d4fe3fb70e6f375fd7 --- .../react/bridge/ReadableNativeMap.java | 235 +++++++++++++++--- .../main/jni/react/jni/ReadableNativeMap.cpp | 91 ++++++- .../main/jni/react/jni/ReadableNativeMap.h | 5 + 3 files changed, 286 insertions(+), 45 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/ReadableNativeMap.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/ReadableNativeMap.java index b7b73b34b..ea1d04d07 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/ReadableNativeMap.java +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/ReadableNativeMap.java @@ -14,6 +14,9 @@ import com.facebook.proguard.annotations.DoNotStrip; import java.util.HashMap; +import com.facebook.infer.annotation.Assertions; +import javax.annotation.Nullable; + /** * Implementation of a read-only map in native memory. This will generally be constructed and filled * in native code so you shouldn't construct one yourself. @@ -28,24 +31,177 @@ public class ReadableNativeMap extends NativeMap implements ReadableMap { super(hybridData); } + private @Nullable String[] mKeys; + private @Nullable HashMap mLocalMap; + private @Nullable HashMap mLocalTypeMap; + private static boolean mUseNativeAccessor; + private static int mJniCallCounter; + public static void setUseNativeAccessor(boolean useNativeAccessor) { + mUseNativeAccessor = useNativeAccessor; + } + public static int getJNIPassCounter() { + return mJniCallCounter; + } + + private HashMap getLocalMap() { + // Fast return for the common case + if (mLocalMap != null) { + return mLocalMap; + } + // Check and when necessary get keys atomicaly + synchronized (this) { + if (mKeys == null) { + mKeys = Assertions.assertNotNull(importKeys()); + mJniCallCounter++; + } + if (mLocalMap == null) { + Object[] values = Assertions.assertNotNull(importValues()); + mJniCallCounter++; + mLocalMap = new HashMap<>(); + for(int i = 0; i< mKeys.length; i++) { + mLocalMap.put(mKeys[i], values[i]); + } + } + } + return mLocalMap; + } + private native String[] importKeys(); + private native Object[] importValues(); + + private HashMap getLocalTypeMap() { + // Fast and non-blocking return for common case + if (mLocalTypeMap != null) { + return mLocalTypeMap; + } + // Check and when necessary get keys + synchronized (this) { + if (mKeys == null) { + mKeys = Assertions.assertNotNull(importKeys()); + mJniCallCounter++; + } + // check that no other thread has already updated + if (mLocalTypeMap == null) { + Object[] types = Assertions.assertNotNull(importTypes()); + mJniCallCounter++; + mLocalTypeMap = new HashMap<>(); + for(int i = 0; i< mKeys.length; i++) { + mLocalTypeMap.put(mKeys[i], (ReadableType) types[i]); + } + } + } + return mLocalTypeMap; + } + private native Object[] importTypes(); + @Override - public native boolean hasKey(String name); + public boolean hasKey(String name) { + if (mUseNativeAccessor) { + mJniCallCounter++; + return hasKeyNative(name); + } + return getLocalMap().containsKey(name); + } + private native boolean hasKeyNative(String name); + @Override - public native boolean isNull(String name); + public boolean isNull(String name) { + if (mUseNativeAccessor) { + mJniCallCounter++; + return isNullNative(name); + } + if (getLocalMap().containsKey(name)) { + return getLocalMap().get(name) == null; + } + throw new NoSuchKeyException(name); + } + private native boolean isNullNative(String name); + + private Object getValue(String name) { + if (hasKey(name) && !(isNull(name))) { + return Assertions.assertNotNull(getLocalMap().get(name)); + } + throw new NoSuchKeyException(name); + } + private @Nullable Object getNullableValue(String name) { + if (hasKey(name)) { + return getLocalMap().get(name); + } + throw new NoSuchKeyException(name); + } + @Override - public native boolean getBoolean(String name); + public boolean getBoolean(String name) { + if (mUseNativeAccessor) { + mJniCallCounter++; + return getBooleanNative(name); + } + return ((Boolean) getValue(name)).booleanValue(); + } + private native boolean getBooleanNative(String name); + @Override - public native double getDouble(String name); + public double getDouble(String name) { + if (mUseNativeAccessor) { + mJniCallCounter++; + return getDoubleNative(name); + } + return ((Double) getValue(name)).doubleValue(); + } + private native double getDoubleNative(String name); + @Override - public native int getInt(String name); + public int getInt(String name) { + if (mUseNativeAccessor) { + mJniCallCounter++; + return getIntNative(name); + } + // All numbers coming out of native are doubles, so cast here then truncate + return ((Double) getValue(name)).intValue(); + } + private native int getIntNative(String name); + @Override - public native String getString(String name); + public @Nullable String getString(String name) { + if (mUseNativeAccessor) { + mJniCallCounter++; + return getStringNative(name); + } + return (String) getNullableValue(name); + } + private native String getStringNative(String name); + @Override - public native ReadableNativeArray getArray(String name); + public @Nullable ReadableArray getArray(String name) { + if (mUseNativeAccessor) { + mJniCallCounter++; + return getArrayNative(name); + } + return (ReadableArray) getNullableValue(name); + } + private native ReadableNativeArray getArrayNative(String name); + @Override - public native ReadableNativeMap getMap(String name); + public @Nullable ReadableNativeMap getMap(String name) { + if (mUseNativeAccessor) { + mJniCallCounter++; + return getMapNative(name); + } + return (ReadableNativeMap) getNullableValue(name); + } + private native ReadableNativeMap getMapNative(String name); + @Override - public native ReadableType getType(String name); + public ReadableType getType(String name) { + if (mUseNativeAccessor) { + mJniCallCounter++; + return getTypeNative(name); + } + if (getLocalTypeMap().containsKey(name)) { + return Assertions.assertNotNull(getLocalTypeMap().get(name)); + } + throw new NoSuchKeyException(name); + } + private native ReadableType getTypeNative(String name); @Override public Dynamic getDynamic(String name) { @@ -59,35 +215,42 @@ public class ReadableNativeMap extends NativeMap implements ReadableMap { @Override public HashMap toHashMap() { - ReadableMapKeySetIterator iterator = keySetIterator(); - HashMap hashMap = new HashMap<>(); + if (mUseNativeAccessor) { + ReadableMapKeySetIterator iterator = keySetIterator(); + HashMap hashMap = new HashMap<>(); - while (iterator.hasNextKey()) { - String key = iterator.nextKey(); - switch (getType(key)) { - case Null: - hashMap.put(key, null); - break; - case Boolean: - hashMap.put(key, getBoolean(key)); - break; - case Number: - hashMap.put(key, getDouble(key)); - break; - case String: - hashMap.put(key, getString(key)); - break; - case Map: - hashMap.put(key, getMap(key).toHashMap()); - break; - case Array: - hashMap.put(key, getArray(key).toArrayList()); - break; - default: - throw new IllegalArgumentException("Could not convert object with key: " + key + "."); - } + while (iterator.hasNextKey()) { + // increment for hasNextKey call + mJniCallCounter++; + String key = iterator.nextKey(); + // increment for nextKey call + mJniCallCounter++; + switch (getType(key)) { + case Null: + hashMap.put(key, null); + break; + case Boolean: + hashMap.put(key, getBoolean(key)); + break; + case Number: + hashMap.put(key, getDouble(key)); + break; + case String: + hashMap.put(key, getString(key)); + break; + case Map: + hashMap.put(key, Assertions.assertNotNull(getMap(key)).toHashMap()); + break; + case Array: + hashMap.put(key, Assertions.assertNotNull(getArray(key)).toArrayList()); + break; + default: + throw new IllegalArgumentException("Could not convert object with key: " + key + "."); + } + } + return hashMap; } - return hashMap; + return getLocalMap(); } /** diff --git a/ReactAndroid/src/main/jni/react/jni/ReadableNativeMap.cpp b/ReactAndroid/src/main/jni/react/jni/ReadableNativeMap.cpp index b057f7fe1..a580fd62b 100644 --- a/ReactAndroid/src/main/jni/react/jni/ReadableNativeMap.cpp +++ b/ReactAndroid/src/main/jni/react/jni/ReadableNativeMap.cpp @@ -17,6 +17,76 @@ void ReadableNativeMap::mapException(const std::exception& ex) { } } +local_ref> ReadableNativeMap::importKeys() { + auto pairs = map_.items(); + keys_ = folly::dynamic::array(); + for (auto &pair : pairs) { + keys_.value().push_back(pair.first.asString()); + } + jint size = keys_.value().size(); + auto jarray = JArrayClass::newArray(size); + for (jint i = 0; i < size; i++) { + jarray->setElement(i, make_jstring(keys_.value()[i].getString().c_str()).release()); + } + return jarray; +} + +local_ref> ReadableNativeMap::importValues() { + jint size = keys_.value().size(); + auto jarray = JArrayClass::newArray(size); + for (jint i = 0; i < size; i++) { + std::string key = keys_.value()[i].getString().c_str(); + const auto element = map_.at(key); + switch(element.type()) { + case folly::dynamic::Type::NULLT: { + jarray->setElement(i, nullptr); + break; + } + case folly::dynamic::Type::BOOL: { + jarray-> + setElement(i, + JBoolean::valueOf(ReadableNativeMap::getBooleanKey(key)).release()); + break; + } + case folly::dynamic::Type::INT64: + case folly::dynamic::Type::DOUBLE: { + jarray->setElement(i, + JDouble::valueOf(ReadableNativeMap::getDoubleKey(key)).release()); + break; + } + case folly::dynamic::Type::STRING: { + jarray-> + setElement(i, + ReadableNativeMap::getStringKey(key).release()); + break; + } + case folly::dynamic::Type::OBJECT: { + jarray->setElement(i,ReadableNativeMap::getMapKey(key).release()); + break; + } + case folly::dynamic::Type::ARRAY: { + jarray->setElement(i,ReadableNativeMap::getArrayKey(key).release()); + break; + } + default: { + jarray->setElement(i,nullptr); + break; + } + } + } + return jarray; +} + +local_ref> ReadableNativeMap::importTypes() { + jint size = keys_.value().size(); + auto jarray = JArrayClass::newArray(size); + for (jint i = 0; i < size; i++) { + std::string key = keys_.value()[i].getString().c_str(); + jarray->setElement(i, ReadableNativeMap::getValueType(key).release()); + } + return jarray; +} + bool ReadableNativeMap::hasKey(const std::string& key) { return map_.find(key) != map_.items().end(); } @@ -99,15 +169,18 @@ local_ref ReadableNativeMap::createWithContent void ReadableNativeMap::registerNatives() { registerHybrid({ - makeNativeMethod("hasKey", ReadableNativeMap::hasKey), - makeNativeMethod("isNull", ReadableNativeMap::isNull), - makeNativeMethod("getBoolean", ReadableNativeMap::getBooleanKey), - makeNativeMethod("getDouble", ReadableNativeMap::getDoubleKey), - makeNativeMethod("getInt", ReadableNativeMap::getIntKey), - makeNativeMethod("getString", ReadableNativeMap::getStringKey), - makeNativeMethod("getArray", ReadableNativeMap::getArrayKey), - makeNativeMethod("getMap", ReadableNativeMap::getMapKey), - makeNativeMethod("getType", ReadableNativeMap::getValueType), + makeNativeMethod("importKeys", ReadableNativeMap::importKeys), + makeNativeMethod("importValues", ReadableNativeMap::importValues), + makeNativeMethod("importTypes", ReadableNativeMap::importTypes), + makeNativeMethod("hasKeyNative", ReadableNativeMap::hasKey), + makeNativeMethod("isNullNative", ReadableNativeMap::isNull), + makeNativeMethod("getBooleanNative", ReadableNativeMap::getBooleanKey), + makeNativeMethod("getDoubleNative", ReadableNativeMap::getDoubleKey), + makeNativeMethod("getIntNative", ReadableNativeMap::getIntKey), + makeNativeMethod("getStringNative", ReadableNativeMap::getStringKey), + makeNativeMethod("getArrayNative", ReadableNativeMap::getArrayKey), + makeNativeMethod("getMapNative", ReadableNativeMap::getMapKey), + makeNativeMethod("getTypeNative", ReadableNativeMap::getValueType), }); } diff --git a/ReactAndroid/src/main/jni/react/jni/ReadableNativeMap.h b/ReactAndroid/src/main/jni/react/jni/ReadableNativeMap.h index 92f36c37a..de06d6975 100644 --- a/ReactAndroid/src/main/jni/react/jni/ReadableNativeMap.h +++ b/ReactAndroid/src/main/jni/react/jni/ReadableNativeMap.h @@ -5,6 +5,7 @@ #include #include #include +#include #include "NativeCommon.h" #include "NativeMap.h" @@ -18,6 +19,9 @@ struct WritableNativeMap; struct ReadableNativeMap : jni::HybridClass { static auto constexpr kJavaDescriptor = "Lcom/facebook/react/bridge/ReadableNativeMap;"; + jni::local_ref> importKeys(); + jni::local_ref> importValues(); + jni::local_ref> importTypes(); bool hasKey(const std::string& key); const folly::dynamic& getMapValue(const std::string& key); bool isNull(const std::string& key); @@ -28,6 +32,7 @@ struct ReadableNativeMap : jni::HybridClass { jni::local_ref getArrayKey(const std::string& key); jni::local_ref getMapKey(const std::string& key); jni::local_ref getValueType(const std::string& key); + folly::Optional keys_; static jni::local_ref createWithContents(folly::dynamic&& map); static void mapException(const std::exception& ex);