Bridge performance: Using JSON-Strings is faster than WritableMap/WritableArray

[fab1an]

Coming from this issue: #8780

I've put together a benchmark calling the bridge and receiving various objects. It compares:

1. call and receive an object using WritableMap/WritableArray.
2. call and receive the same object by using the gson library to serialise it to a String, then deserialise it using JSON.parse.

The benchmark shows that using Json strings is faster than using WritableMap/WritableArray, especially with arrays.

Source: https://github.com/fab1an/react-native-test/tree/testArguments

Updated results after improvements

The results:

• Simple Object: GSON is 3% faster

test_obj_simple_rn         x 493 ops/sec ±1.83% (133 runs sampled) 
test_obj_simple_json       x 507 ops/sec ±1.56% (135 runs sampled) 

• More complex, nested object: GSON is 51% faster

test_obj_complex_rn        x 270 ops/sec ±1.96% (134 runs sampled) 
test_obj_complex_json      x 408 ops/sec ±1.67% (134 runs sampled) 

• 10-element array of integers: equal

test_array_num_small_rn         x 500 ops/sec ±1.54% (134 runs sampled) 
test_array_num_small_json       x 501 ops/sec ±1.84% (133 runs sampled) 

• 1000-element array of integers: GSON is 64% faster

test_array_num_large_rn         x 76.41 ops/sec ±1.54% (135 runs sampled) 
test_array_num_large_json       x 125 ops/sec ±1.61% (133 runs sampled) 

• 10-element array of strings: GSON is 5% faster

test_array_str_small_rn         x 469 ops/sec ±1.79% (133 runs sampled) 
test_array_str_small_json       x 490 ops/sec ±1.78% (133 runs sampled) 

• 1000-element array of strings: GSON is 90% faster

test_array_str_large_rn         x 44.78 ops/sec ±2.03% (134 runs sampled) 
test_array_str_large_json       x 85.04 ops/sec ±1.40% (134 runs sampled) 

[ide]

The benchmarks look good, it'd be good to know where the performance improvement is coming from. I've passed this on to people who work at facebook since they have tools for finding regressions.

[fab1an]

Thanks. The problem might lie with WritableArray, since the complex object contains an array as well.

[mkonicek]

Is this for the old Java bridge or the new C++ bridge that's going to replace the old bridge both on iOS and Android?

cc @mhorowitz who works on the C++ bridge - Marc has the C++ bridge been rolled out in open source and the Java code deleted? I still see Java code here: https://github.com/facebook/react-native/tree/master/ReactAndroid/src/main/java/com/facebook/react/bridge

[mhorowitz]

Yes, the C++ bridge is the only bridge in OSS for Android. The old Android bridge is deprecated and gone. The directory @mkonicek linked above is the part of the bridge code which was shared between the old and new bridges.

The benchmarks are interesting. It does seem a bit unfair to say that gson is faster than WritableMap/Array, since it looks like the winner depends on the workload. I don't have a lot of intuition about which workload is more common. I don't think it's common to be moving arrays of thousands of strings around, at least in our app.

Another complicating factor here is that we have an iOS bridge under development (available soon, I hope) which shares the C++ code in https://github.com/facebook/react-native/tree/master/ReactCommon/cxxreact on iOS and Android, so any performance improvements need to be considered on both platforms.

[fab1an]

I've updated the benchmarks to write more efficient JSON.

Now Gson is faster in every aspect compared to react-native, except in one where they tie.

Some operations are almost twice as fast! Please have a look into this…

[fab1an]

@ide @mkonicek Can anyone give me a short hint on what to do here? Should I switch all my production code to using Gson?

[ohtangza]

Any updates on this issue? We are loading wave files to JS code to do some signal processing (very small chunks) and it takes more than 1000ms for sending 500kb file. If I just serialize it to Base64 string, it only takes around 100ms. This is only specific to Android.

[fab1an]

@ohtangza: For what it's worth, I use string-serialization in production. RN engineers think otherwise, but I consider their bridge-memory-management on Android flawed.

I guess this issue will be probably closed because of time passing.

If you want to help with that, you could post graphs and performance comparison of your usecase to show the slowdown.

[hramos]

Hi there! This issue is being closed because it has been inactive for a while. Maybe the issue has been fixed in a recent release, or perhaps it is not affecting a lot of people. Either way, we're automatically closing issues after a period of inactivity. Please do not take it personally!

If you think this issue should definitely remain open, please let us know. The following information is helpful when it comes to determining if the issue should be re-opened:

• Does the issue still reproduce on the latest release candidate? Post a comment with the version you tested.
• If so, is there any information missing from the bug report? Post a comment with all the information required by the issue template.
• Is there a pull request that addresses this issue? Post a comment with the PR number so we can follow up.

If you would like to work on a patch to fix the issue, contributions are very welcome! Read through the contribution guide, and feel free to hop into #react-native if you need help planning your contribution.

[fab1an]

Did anyone even bother to run my benchmarks?
I don't think I'll waste time contributing to react-native again.

[hramos]

I'm sorry you feel that way. With no recent activity on the issue, and due to the high number of issues still open in the repo, I've decided to icebox stale issues, especially those that don't deal with a particular, reproducible bug. If you feel strongly that this should remain open, let me know. I'd like to know what specific next steps you had in mind for resolving the issue. Perhaps there's a better forum for this type of discussion elsewhere.

[fab1an]

What is a reproducible issue if not this one, where I added actual benchmarking code you can run?

The next steps would be run the benchmarks and decide whether to do something about that slowdown or ignore it.

Then you can choose whether to take time to write up a reason for the choice to ignore it or just close the issue without comment.

[ohtangza]

@hramos I also agree on @fab1an. I think this issue is still valid because WritableArray is abnormally slow in Android (compared to iOS) when we tested with RN 0.43. I think this issue is very clear and critical for high-performant app. We are currently streaming the binary data from microphone to JS side for simple amplitude extraction, and Android code is way too slow.

[hramos]

Closing the issue does not undo the work you've already put in here. I am not strongly against re-opening the issue, but I do want to make sure there is a clear actionable step to resolve the issue before doing so. It seems like there was some initial interest in the benchmarks here, but from what I can gather nothing actionable has yet come out of it. Until then, I suggest using a forum better suited to long running discussions, such as https://discuss.reactjs.org/

[ohtangza]

@hramos Oh, it seems that there is little misunderstanding between us. I considered this as a bug-like issue not a general performance issue because sending simple WritableArray representation of native array should be faster than a serialized string of the native array.

So, does it mean that we'd better send an object from native code (Obj-C or Java) to send using serialization rather than WritableArray/WritableObject? It's little difficult for me to understand this is simply closed like this without further investigation.

Let me know if I understand something wrong. (I'm a big fan of RN, btw!)

[hramos]

I'll go ahead and re-open. Going forward I'll follow this policy: if a stale issue gets closed but more than one person comes forward and makes a case for it to remain open, I'll default to re-opening. Note that simply stating "it is still an issue" may not meet this bar - I specifically want to handle cases like this particular issue where people feel invested in the work put into the issue so far.

You may notice I've gone back and updated the comments that my script left, so that we're no longer redirecting stale issues to Canny.

[fab1an]

@hramos That sounds like a good policy. Additionally I'd focus on open issues with developers who are willing to put time into producing additional info or working test-code like me. ;-)

I'm a fan of RN as well.

[fmnxl]

This would be very helpful for loading a relatively large amount of data from a database. At the moment loading ~200kb worth of data takes 1-2 seconds on Android phones for us (https://github.com/andpor/react-native-sqlite-storage) and it's really affecting our startup time.

[kesha-antonov]

+1. Nice job on this.

[ohtangza]

@freemanon What our team tried is that getting sensor data stream from native side, which the data does not exist when initializing the loading. For the pre-stored data, like you said, a database could be helpful.

[fab1an]

Closing this, because I don't think it will ever get fixed. Have a nice good new year.