Skip to content

Commit 7891805

Browse files
Kathy Grayfacebook-github-bot
Kathy Gray
authored and
facebook-github-bot
committed
move native accesses of map into two accesses instead of per-element
Differential Revision: D6650192 fbshipit-source-id: 0eb0719fd3b7813add33a3d4fe3fb70e6f375fd7
1 parent 5649aed commit 7891805

File tree

3 files changed

+287
-46
lines changed

3 files changed

+287
-46
lines changed

Diff for: ‎ReactAndroid/src/main/java/com/facebook/react/bridge/ReadableNativeMap.java

+200-37
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,9 @@
1414

1515
import java.util.HashMap;
1616

17+
import com.facebook.infer.annotation.Assertions;
18+
import javax.annotation.Nullable;
19+
1720
/**
1821
* Implementation of a read-only map in native memory. This will generally be constructed and filled
1922
* in native code so you shouldn't construct one yourself.
@@ -28,24 +31,177 @@ protected ReadableNativeMap(HybridData hybridData) {
2831
super(hybridData);
2932
}
3033

34+
private @Nullable String[] mKeys;
35+
private @Nullable HashMap<String,Object> mLocalMap;
36+
private @Nullable HashMap<String,ReadableType> mLocalTypeMap;
37+
private static boolean mUseNativeAccessor;
38+
private static int mJniCallCounter;
39+
public static void setUseNativeAccessor(boolean useNativeAccessor) {
40+
mUseNativeAccessor = useNativeAccessor;
41+
}
42+
public static int getJNIPassCounter() {
43+
return mJniCallCounter;
44+
}
45+
46+
private HashMap<String,Object> getLocalMap() {
47+
// Fast return for the common case
48+
if (mLocalMap != null) {
49+
return mLocalMap;
50+
}
51+
// Check and when necessary get keys atomicaly
52+
synchronized (this) {
53+
if (mKeys == null) {
54+
mKeys = Assertions.assertNotNull(importKeys());
55+
mJniCallCounter++;
56+
}
57+
if (mLocalMap == null) {
58+
Object[] values = Assertions.assertNotNull(importValues());
59+
mJniCallCounter++;
60+
mLocalMap = new HashMap<>();
61+
for(int i = 0; i< mKeys.length; i++) {
62+
mLocalMap.put(mKeys[i], values[i]);
63+
}
64+
}
65+
}
66+
return mLocalMap;
67+
}
68+
private native String[] importKeys();
69+
private native Object[] importValues();
70+
71+
private HashMap<String,ReadableType> getLocalTypeMap() {
72+
// Fast and non-blocking return for common case
73+
if (mLocalTypeMap != null) {
74+
return mLocalTypeMap;
75+
}
76+
// Check and when necessary get keys
77+
synchronized (this) {
78+
if (mKeys == null) {
79+
mKeys = Assertions.assertNotNull(importKeys());
80+
mJniCallCounter++;
81+
}
82+
// check that no other thread has already updated
83+
if (mLocalTypeMap == null) {
84+
Object[] types = Assertions.assertNotNull(importTypes());
85+
mJniCallCounter++;
86+
mLocalTypeMap = new HashMap<>();
87+
for(int i = 0; i< mKeys.length; i++) {
88+
mLocalTypeMap.put(mKeys[i], (ReadableType) types[i]);
89+
}
90+
}
91+
}
92+
return mLocalTypeMap;
93+
}
94+
private native Object[] importTypes();
95+
3196
@Override
32-
public native boolean hasKey(String name);
97+
public boolean hasKey(String name) {
98+
if (mUseNativeAccessor) {
99+
mJniCallCounter++;
100+
return hasKeyNative(name);
101+
}
102+
return getLocalMap().containsKey(name);
103+
}
104+
private native boolean hasKeyNative(String name);
105+
33106
@Override
34-
public native boolean isNull(String name);
107+
public boolean isNull(String name) {
108+
if (mUseNativeAccessor) {
109+
mJniCallCounter++;
110+
return isNullNative(name);
111+
}
112+
if (getLocalMap().containsKey(name)) {
113+
return getLocalMap().get(name) == null;
114+
}
115+
throw new NoSuchKeyException(name);
116+
}
117+
private native boolean isNullNative(String name);
118+
119+
private Object getValue(String name) {
120+
if (hasKey(name) && !(isNull(name))) {
121+
return Assertions.assertNotNull(getLocalMap().get(name));
122+
}
123+
throw new NoSuchKeyException(name);
124+
}
125+
private @Nullable Object getNullableValue(String name) {
126+
if (hasKey(name)) {
127+
return getLocalMap().get(name);
128+
}
129+
throw new NoSuchKeyException(name);
130+
}
131+
35132
@Override
36-
public native boolean getBoolean(String name);
133+
public boolean getBoolean(String name) {
134+
if (mUseNativeAccessor) {
135+
mJniCallCounter++;
136+
return getBooleanNative(name);
137+
}
138+
return ((Boolean) getValue(name)).booleanValue();
139+
}
140+
private native boolean getBooleanNative(String name);
141+
37142
@Override
38-
public native double getDouble(String name);
143+
public double getDouble(String name) {
144+
if (mUseNativeAccessor) {
145+
mJniCallCounter++;
146+
return getDoubleNative(name);
147+
}
148+
return ((Double) getValue(name)).doubleValue();
149+
}
150+
private native double getDoubleNative(String name);
151+
39152
@Override
40-
public native int getInt(String name);
153+
public int getInt(String name) {
154+
if (mUseNativeAccessor) {
155+
mJniCallCounter++;
156+
return getIntNative(name);
157+
}
158+
// All numbers coming out of native are doubles, so cast here then truncate
159+
return ((Double) getValue(name)).intValue();
160+
}
161+
private native int getIntNative(String name);
162+
41163
@Override
42-
public native String getString(String name);
164+
public @Nullable String getString(String name) {
165+
if (mUseNativeAccessor) {
166+
mJniCallCounter++;
167+
return getStringNative(name);
168+
}
169+
return (String) getNullableValue(name);
170+
}
171+
private native String getStringNative(String name);
172+
43173
@Override
44-
public native ReadableNativeArray getArray(String name);
174+
public @Nullable ReadableArray getArray(String name) {
175+
if (mUseNativeAccessor) {
176+
mJniCallCounter++;
177+
return getArrayNative(name);
178+
}
179+
return (ReadableArray) getNullableValue(name);
180+
}
181+
private native ReadableNativeArray getArrayNative(String name);
182+
45183
@Override
46-
public native ReadableNativeMap getMap(String name);
184+
public @Nullable ReadableNativeMap getMap(String name) {
185+
if (mUseNativeAccessor) {
186+
mJniCallCounter++;
187+
return getMapNative(name);
188+
}
189+
return (ReadableNativeMap) getNullableValue(name);
190+
}
191+
private native ReadableNativeMap getMapNative(String name);
192+
47193
@Override
48-
public native ReadableType getType(String name);
194+
public ReadableType getType(String name) {
195+
if (mUseNativeAccessor) {
196+
mJniCallCounter++;
197+
return getTypeNative(name);
198+
}
199+
if (getLocalTypeMap().containsKey(name)) {
200+
return Assertions.assertNotNull(getLocalTypeMap().get(name));
201+
}
202+
throw new NoSuchKeyException(name);
203+
}
204+
private native ReadableType getTypeNative(String name);
49205

50206
@Override
51207
public Dynamic getDynamic(String name) {
@@ -59,35 +215,42 @@ public ReadableMapKeySetIterator keySetIterator() {
59215

60216
@Override
61217
public HashMap<String, Object> toHashMap() {
62-
ReadableMapKeySetIterator iterator = keySetIterator();
63-
HashMap<String, Object> hashMap = new HashMap<>();
64-
65-
while (iterator.hasNextKey()) {
66-
String key = iterator.nextKey();
67-
switch (getType(key)) {
68-
case Null:
69-
hashMap.put(key, null);
70-
break;
71-
case Boolean:
72-
hashMap.put(key, getBoolean(key));
73-
break;
74-
case Number:
75-
hashMap.put(key, getDouble(key));
76-
break;
77-
case String:
78-
hashMap.put(key, getString(key));
79-
break;
80-
case Map:
81-
hashMap.put(key, getMap(key).toHashMap());
82-
break;
83-
case Array:
84-
hashMap.put(key, getArray(key).toArrayList());
85-
break;
86-
default:
87-
throw new IllegalArgumentException("Could not convert object with key: " + key + ".");
88-
}
218+
if (mUseNativeAccessor) {
219+
ReadableMapKeySetIterator iterator = keySetIterator();
220+
HashMap<String, Object> hashMap = new HashMap<>();
221+
222+
while (iterator.hasNextKey()) {
223+
// increment for hasNextKey call
224+
mJniCallCounter++;
225+
String key = iterator.nextKey();
226+
// increment for nextKey call
227+
mJniCallCounter++;
228+
switch (getType(key)) {
229+
case Null:
230+
hashMap.put(key, null);
231+
break;
232+
case Boolean:
233+
hashMap.put(key, getBoolean(key));
234+
break;
235+
case Number:
236+
hashMap.put(key, getDouble(key));
237+
break;
238+
case String:
239+
hashMap.put(key, getString(key));
240+
break;
241+
case Map:
242+
hashMap.put(key, Assertions.assertNotNull(getMap(key)).toHashMap());
243+
break;
244+
case Array:
245+
hashMap.put(key, Assertions.assertNotNull(getArray(key)).toArrayList());
246+
break;
247+
default:
248+
throw new IllegalArgumentException("Could not convert object with key: " + key + ".");
249+
}
250+
}
251+
return hashMap;
89252
}
90-
return hashMap;
253+
return getLocalMap();
91254
}
92255

93256
/**

Diff for: ‎ReactAndroid/src/main/jni/react/jni/ReadableNativeMap.cpp

+82-9
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,76 @@ void ReadableNativeMap::mapException(const std::exception& ex) {
1717
}
1818
}
1919

20+
local_ref<JArrayClass<jstring>> ReadableNativeMap::importKeys() {
21+
auto pairs = map_.items();
22+
keys_ = folly::dynamic::array();
23+
for (auto &pair : pairs) {
24+
keys_.value().push_back(pair.first.asString());
25+
}
26+
jint size = keys_.value().size();
27+
auto jarray = JArrayClass<jstring>::newArray(size);
28+
for (jint i = 0; i < size; i++) {
29+
jarray->setElement(i, make_jstring(keys_.value()[i].getString().c_str()).release());
30+
}
31+
return jarray;
32+
}
33+
34+
local_ref<JArrayClass<jobject>> ReadableNativeMap::importValues() {
35+
jint size = keys_.value().size();
36+
auto jarray = JArrayClass<jobject>::newArray(size);
37+
for (jint i = 0; i < size; i++) {
38+
std::string key = keys_.value()[i].getString().c_str();
39+
const auto element = map_.at(key);
40+
switch(element.type()) {
41+
case folly::dynamic::Type::NULLT: {
42+
jarray->setElement(i, nullptr);
43+
break;
44+
}
45+
case folly::dynamic::Type::BOOL: {
46+
jarray->
47+
setElement(i,
48+
JBoolean::valueOf(ReadableNativeMap::getBooleanKey(key)).release());
49+
break;
50+
}
51+
case folly::dynamic::Type::INT64:
52+
case folly::dynamic::Type::DOUBLE: {
53+
jarray->setElement(i,
54+
JDouble::valueOf(ReadableNativeMap::getDoubleKey(key)).release());
55+
break;
56+
}
57+
case folly::dynamic::Type::STRING: {
58+
jarray->
59+
setElement(i,
60+
ReadableNativeMap::getStringKey(key).release());
61+
break;
62+
}
63+
case folly::dynamic::Type::OBJECT: {
64+
jarray->setElement(i,ReadableNativeMap::getMapKey(key).release());
65+
break;
66+
}
67+
case folly::dynamic::Type::ARRAY: {
68+
jarray->setElement(i,ReadableNativeMap::getArrayKey(key).release());
69+
break;
70+
}
71+
default: {
72+
jarray->setElement(i,nullptr);
73+
break;
74+
}
75+
}
76+
}
77+
return jarray;
78+
}
79+
80+
local_ref<JArrayClass<jobject>> ReadableNativeMap::importTypes() {
81+
jint size = keys_.value().size();
82+
auto jarray = JArrayClass<jobject>::newArray(size);
83+
for (jint i = 0; i < size; i++) {
84+
std::string key = keys_.value()[i].getString().c_str();
85+
jarray->setElement(i, ReadableNativeMap::getValueType(key).release());
86+
}
87+
return jarray;
88+
}
89+
2090
bool ReadableNativeMap::hasKey(const std::string& key) {
2191
return map_.find(key) != map_.items().end();
2292
}
@@ -99,15 +169,18 @@ local_ref<ReadableNativeMap::jhybridobject> ReadableNativeMap::createWithContent
99169

100170
void ReadableNativeMap::registerNatives() {
101171
registerHybrid({
102-
makeNativeMethod("hasKey", ReadableNativeMap::hasKey),
103-
makeNativeMethod("isNull", ReadableNativeMap::isNull),
104-
makeNativeMethod("getBoolean", ReadableNativeMap::getBooleanKey),
105-
makeNativeMethod("getDouble", ReadableNativeMap::getDoubleKey),
106-
makeNativeMethod("getInt", ReadableNativeMap::getIntKey),
107-
makeNativeMethod("getString", ReadableNativeMap::getStringKey),
108-
makeNativeMethod("getArray", ReadableNativeMap::getArrayKey),
109-
makeNativeMethod("getMap", ReadableNativeMap::getMapKey),
110-
makeNativeMethod("getType", ReadableNativeMap::getValueType),
172+
makeNativeMethod("importKeys", ReadableNativeMap::importKeys),
173+
makeNativeMethod("importValues", ReadableNativeMap::importValues),
174+
makeNativeMethod("importTypes", ReadableNativeMap::importTypes),
175+
makeNativeMethod("hasKeyNative", ReadableNativeMap::hasKey),
176+
makeNativeMethod("isNullNative", ReadableNativeMap::isNull),
177+
makeNativeMethod("getBooleanNative", ReadableNativeMap::getBooleanKey),
178+
makeNativeMethod("getDoubleNative", ReadableNativeMap::getDoubleKey),
179+
makeNativeMethod("getIntNative", ReadableNativeMap::getIntKey),
180+
makeNativeMethod("getStringNative", ReadableNativeMap::getStringKey),
181+
makeNativeMethod("getArrayNative", ReadableNativeMap::getArrayKey),
182+
makeNativeMethod("getMapNative", ReadableNativeMap::getMapKey),
183+
makeNativeMethod("getTypeNative", ReadableNativeMap::getValueType),
111184
});
112185
}
113186

Diff for: ‎ReactAndroid/src/main/jni/react/jni/ReadableNativeMap.h

+5
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
#include <fb/fbjni.h>
66
#include <folly/dynamic.h>
77
#include <folly/json.h>
8+
#include <folly/Optional.h>
89

910
#include "NativeCommon.h"
1011
#include "NativeMap.h"
@@ -18,6 +19,9 @@ struct WritableNativeMap;
1819
struct ReadableNativeMap : jni::HybridClass<ReadableNativeMap, NativeMap> {
1920
static auto constexpr kJavaDescriptor = "Lcom/facebook/react/bridge/ReadableNativeMap;";
2021

22+
jni::local_ref<jni::JArrayClass<jstring>> importKeys();
23+
jni::local_ref<jni::JArrayClass<jobject>> importValues();
24+
jni::local_ref<jni::JArrayClass<jobject>> importTypes();
2125
bool hasKey(const std::string& key);
2226
const folly::dynamic& getMapValue(const std::string& key);
2327
bool isNull(const std::string& key);
@@ -28,6 +32,7 @@ struct ReadableNativeMap : jni::HybridClass<ReadableNativeMap, NativeMap> {
2832
jni::local_ref<ReadableNativeArray::jhybridobject> getArrayKey(const std::string& key);
2933
jni::local_ref<jhybridobject> getMapKey(const std::string& key);
3034
jni::local_ref<ReadableType> getValueType(const std::string& key);
35+
folly::Optional<folly::dynamic> keys_;
3136
static jni::local_ref<jhybridobject> createWithContents(folly::dynamic&& map);
3237

3338
static void mapException(const std::exception& ex);

0 commit comments

Comments
 (0)