andpor / react-native-sqlite-storage

Full featured SQLite3 Native Plugin for React Native (Android and iOS)
MIT License
2.76k stars 520 forks source link

Question: backgroundExecuteSqlBatch - Android Implementation #133

Closed madhu314 closed 7 years ago

madhu314 commented 7 years ago

In Android it seems that both backgroundExecuteSqlBatch & executeSqlBatch are equivalent with same case fallback

case executeSqlBatch: case backgroundExecuteSqlBatch:

What is the reason for not doing so?

dryganets commented 7 years ago

On android all queries executed on dedicated thread. On iOS you could execute query right on the react thread but I wouldn't do it if I were you as it will cause performance problems

itinance commented 7 years ago

On iOS you could execute query right on the react thread but I wouldn't do it if I were you as it will cause performance problems

@dryganets do you have an own implementation on iOS using dedicated threads for executing queries?

dryganets commented 7 years ago

@itinance, no I just use executeBackground only on js level. on iOS it works on background thread.

dryganets commented 7 years ago

As for our implementation, I have android and typescript done already. According to my measurements, we got 4 times better throughput. It's a plugin for https://github.com/Microsoft/NoSQLProvider

We have a multithreaded model for work with the database now and capability to finish transactions as soon as js layer knows it's the last query in the transaction. https://github.com/Microsoft/NoSQLProvider/pull/24

It would take a time to open source the library but we definitely will do it as soon as iOS part will be completed. It's not a high priority to finish iOS now though.

andpor commented 7 years ago

@dryganets - 4x better throughput is very nice. Anything you can share or suggest for enhancement to this library to make it perform better?

dryganets commented 7 years ago

I did the changes in the following order:

Turns out that it's faster to parse the string with JSON.parse on js layer rather than use WritableNativeArray/Map. Most of the overhead is in passing JNI boundary too many times I believe.

Microbenchmark ran on the desktop (Macbook pro 2.5Ghz Corei7, 16GM ram DDR3 1600Mhz) https://microbenchmarks.appspot.com/runs/a9f21001-922d-4619-9d89-d60daeeb3ae4

I have an end to end test which shows that my version is faster in terms of total roundtrip time on android. I'll share it as I get a minute.

Also, we have a configurable max of concurrent transactions. So if your app has more reads than writes it could be beneficial to use a value different from 1.

dryganets commented 7 years ago

https://github.com/dryganets/BridgeLatencyTest this is the simple benchmark of WritableNativeMap/JSON and Jackson implementations.

You could play with the array size and datasets for the test to figure out what is suits your needs.

madhu314 commented 7 years ago

We have also run into performance issues on android. Since there is only one thread involved in entire db operations on the native side, our database reads suffered.

Also as @dryganets pointed out, there is a huge amount of time spent in converting json back forth from RN's Writable and Readable instances. In fact, for queries that return lot of db rows almost about 80% of time is spent in conversion and only 20% time is taken for actual sql query.

So finally we ended up implementing our own wrapper around android's sqlite provider with priority scheduling of writes vs. reads. We have also completely eliminated json conversion. We directly write into RN's writable instances.

andpor commented 7 years ago

@madhu314 - do you want to contribute any of the enhancements to this module? PR's are welcome..

dryganets commented 7 years ago

@andpor Once I get a free moment I'll add string serialization to the module. Though it will break compatibility with Cordova plugin.

andpor commented 7 years ago

@dryganets - 4x performance gain takes precedence over compatibility :)

fmnxl commented 7 years ago

I tried implementing @dryganets's suggestion above, but it seems that this method doesn't scale well with increasing data size. We have to load ~200kb worth of data from the database in one query, and although serializing with Jackson is fast, transferring the very long string across the bridge seems to be 4x worse than the WritableMap implementation 😞

dryganets commented 7 years ago

@freemanon is 200kb a JSON string size in symbols? older versions of react-native used to have a folly with pretty bad performance bug: https://github.com/facebook/folly/issues/477 The version used in react-native now have it fixed. Maybe you are affected by this bug?

The main performance problem of react-native IMHO in string conversions and related data copy. Every string you pass from UTF-16 (Java) got converted to UTF-8 for folly library and later on converted back to UTF-16 in order to pass it to JavaScriptCore.

All javascript calls to native got converted from UTF-16 (JavaScriptCore) to UTF-8 (folly) and later back to UTF-16 (java)

fmnxl commented 7 years ago

Thanks for the tip :) We're using RN 0.44 but ugrading to 0.46 in the next week or two. If that's the case, a quick fix would be to send the rows one by one using events rather than a callback to cheat on the O(n^2) time complexity.

dryganets commented 7 years ago

@freemanon You are not affected by that issue.

In my experience the large the string the better improvement from using plain text. I'll try to share my test suite on GitHub this weekend.

you could try it on your data.

I measured total bridge time for the same data set with different serialization types.