xmtp / xmtp-react-native

A package you can use to build with XMTP in a React Native or Expo app.
MIT License
41 stars 19 forks source link

Performance: the react-native SDK seems slower than the Webview with JS SDK #33

Closed nmalzieu closed 1 year ago

nmalzieu commented 1 year ago

Describe the bug

Webview side

How it works: I pass the XMTP Key to a Webview that contains the JS SDK and I instantiate a client using

const client = await Client.create(null, {
  privateKeyOverride: Buffer.from(JSON.parse(data.keys)),
  env: "dev",
});

Then I do

const now = new Date().getTime();
const conversations = await client.conversations.list();
const after = new Date().getTime();
console.log(
  `Listing ${conversations.length} conversations took ${(after - now) / 1000} seconds`
);
const now2 = new Date().getTime();
const conversations2 = await client.conversations.list();
const after2 = new Date().getTime();
console.log(
  `Re-listing ${conversations2.length} conversations took ${(after2 - now2) / 1000} seconds`
);

I get "Listing 240 conversations took 2.986 seconds" then "Re-listing 240 conversations took 0.76 seconds", which shows that there is some caching happening since the second call is so much faster!

React Native side

I get "Listing 236 conversations took 5.781 seconds" then "Re-listing 236 conversations took 5.923 seconds"

Expected behavior

I would expect the RN SDK to be at least as fast as the JS SDK in a webview I would expect both SDKs to return the same # of conversations I would expect the RN SDK to have some caching as well

Steps to reproduce the bug

No response

michaelx11 commented 1 year ago

Hi @nmalzieu, thanks for reporting this! We have work in the pipeline to improve performance as we move beyond pre-preview.

Your performance numbers don't align with some of our internal benchmarks. I have a couple questions to help us drill down to the core performance issue.

Note: as you noticed, the new RN SDK is powered by https://github.com/xmtp/xmtp-ios which does not return self-conversations.

Thanks for your input! As you mentioned we also believe the react-native SDK should outperform webview, but we have some optimization work to do.

nmalzieu commented 1 year ago

Hi @michaelx11 , thanks for your answer!

console.log("creating RN client...");
const client = await XMTP.Client.create(signer, "dev");
console.log("RN client created!!", client.address);
const now = new Date().getTime();
const conversations = await client.conversations.list();
const after = new Date().getTime();
console.log(
`Listing ${conversations.length} took ${(after - now) / 1000} seconds`
);
const now2 = new Date().getTime();
const conversations2 = await client.conversations.list();
const after2 = new Date().getTime();
console.log(
`Listing ${conversations2.length} took ${(after2 - now2) / 1000} seconds`
);

Conclusions:

I have multiple ConversationV1 which I know is not the case for everyone, maybe that's why it doesn't align with your benchmarks?

nplasterer commented 1 year ago

Thanks @nmalzieu for the explanation. I just published a new package that has a few performance improvements. On iOS we point to our latest cocopod which we've confirmed sped up conversation list by almost double. And on android I was able to fix the conversation list only showing the last 100 convos to now showing all. Here are the performance benchmarks I'm seeing on the latest package on first load

Android: Loaded 2001 conversations in 28321ms (28.321s) iOS: Loaded 2001 conversations in 10157ms (10.157s)

Be curious to know if you see any improvements on the latest package?

There's definitely more improvements here first being hunting down what's causing the near triple load time on android and second being caching so that the second load isn't as long.

nmalzieu commented 1 year ago

Hey @nplasterer thanks!

iOS: Listing 236 conversations took 1.582 seconds Android (still an old, slow device): Listing 236 conversations took 149.789 seconds

I will test on a newer Android device soon but I can confirm the new SDK show all convos on android and a big perf improvement on iOS, with still no caching

Keep me posted on further improvements as well!

nplasterer commented 1 year ago

@nmalzieu Just wanted to give you an update on the performance work here. I was able to hunt down the slowness on android to these cryptography lines of code (that get called 3 times per conversation) https://github.com/xmtp/xmtp-android/blob/main/library/src/main/java/org/xmtp/android/library/messages/PrivateKeyBundleV2.kt#L47-L51. I've tried some experimental things to speed it up with no success so we may have to go the rust route like iOS is doing. https://github.com/xmtp/libxmtp/blob/main/xmtp_crypto/src/k256_helper.rs#L14:L14. We've got some work moving along on the rust bindings for kotlin currently and I should hopefully know more by Monday.

nmalzieu commented 1 year ago

Hey @nplasterer , thanks, tell me when you need me to do more tests! Are you saying that this performance issue has nothing to do with React Native, it's in the Kotlin SDK (which we are not using in Converse since we're still using the JS SDK in the Webview)

nplasterer commented 1 year ago

@nmalzieu Correct the performance issue we were seeing on the android side was actually coming from the KotlinSDK. The React Native SDK is mostly a thin wrapper around the Kotlin and Swift SDKs. So the performance fixes are down a few levels. I just pushed up a major performance boost for cold loads on Android shaving off about 1/3 of the time for 2001 conversations. Wondering if you can give it another spin to see if Android has improved for you? I'm hoping that this current version of the React Native SDK should be faster or similar to the webview for iOS and Android on cold loads. Please let me know if that's not the case? As for cached loads @dmccartney is working on that right now.

nmalzieu commented 1 year ago

Hey @nplasterer I just tested again with an account with 1290 convos (in production), on a newer android device!

With the RN SDK:

First load : listing 1290 conversations took 15 seconds Second load: listing 1290 conversations took 12 seconds (=> I guess this is not caching? In webview the gap is a lot bigger)

Same account but in a webview:

First load: 15.679 seconds for 1300 conversations Second load: 3.671 seconds for 1300 conversations => now that's caching :)

=> So I get the same perf in webview & RN SDK, not better though!

Also, I wanted to compare with the same account on iOS in production, but I can't make the RN SDK work with production env. See https://github.com/xmtp/xmtp-react-native/issues/51

nplasterer commented 1 year ago

Going to close this out as we've improved cold load and cached load times substantially. Please let me know if you are still seeing issues here:

On iOS (first load and cached load)
 LOG  Loaded 2002 conversations in 8924ms (8.9s)
 LOG  Loaded 2002 conversations in 741ms (0.7s)
On Android (first load and cached load)
 LOG  Loaded 2002 conversations in 17053ms (17s)
 LOG  Loaded 2002 conversations in 1395ms (1.3s)