Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
nmalzieu opened this issue May 17, 2023 · 9 comments
Closed
Assignees
Labels
bug Something isn't working

Comments

@nmalzieu
Copy link
Contributor

nmalzieu commented May 17, 2023

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"

  • I get 236 conversations, not 240 anymore
    • I suspect it doesn't return ConversationV1 or it doesn't return conversations with myself (I have a few)
  • The first call takes LONGER than the JS SDK in a Webview
  • The second call takes the same time as the first call (no caching?)

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

@nmalzieu nmalzieu added the bug Something isn't working label May 17, 2023
@michaelx11
Copy link
Contributor

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.

  • What address are you testing with?
  • Which device or simulator are you using? Which platform?
  • Can you share the code you're using to bootstrap the new RN SDK? Instantiate the XMTP.Client etc
  • What performance numbers do you get if you switch platforms?

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
Copy link
Contributor Author

nmalzieu commented May 19, 2023

Hi @michaelx11 , thanks for your answer!

  • My tests are on env dev, and my address is 0x2376e9C7C604D1827bA9aCb1293Dc8b4DA2f0DB3
  • I'm testing on my iPhone 12, iOS 16, and an old Android, Samsung Galaxy A5, Android 8 ( it's globally very slow but we can still compare the two SDKs)
  • I'm at home with great connectivity
  • I'm instantiating the RN SDK using an ethers Signer that I got using the @walletconnect/react-native-dapp lib:
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`
);

  • With the JS SDK inside a webview, on my iPhone 12: first load 240 conversations 2.899 seconds, second load 240 conversations 0.5 seconds

  • With the RN SDK, on my iPhone 12: first load 236 conversations 4.336 seconds, second load 236 conversations 4.256 seconds ==> ~ takes 2x the time, and no caching

  • With the JS SDK inside a webview, on my Samsung first load 240 conversations 52.543 seconds, second load 240 conversations 1.06 seconds

  • With the RN SDK, on my Samsung first load 103 conversations (no idea why it's not all my conversations!), 89.585 seconds (so still slower than the webview), second load 103 conversations, 88.932 seconds

Conclusions:

  • for me, the RN SDK is slower than the JS SDK in a webview
  • the RN SDK does not implement caching (second call to conversations.list() takes roughly the same time)
  • the RN SDK in Android does not return enough conversations 103

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
Copy link
Contributor

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
Copy link
Contributor Author

nmalzieu commented May 20, 2023

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
Copy link
Contributor

@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.

richardhuaaa added a commit to xmtp/libxmtp that referenced this issue May 30, 2023
We need to expose the Rust-backed Diffie-Hellman operation to unblock performance improvements in Android, context [here](xmtp/xmtp-react-native#33 (comment)). To keep binary size and build-times low, as well as to avoid significant time overhauling `xmtp_swift`, I've made a separate crate called `xmtp_dh` that only contains the single `diffie_hellman_k256` API that is needed for now.

There are three pieces here:
- The `xmtp_dh` crate. The `README.md` describes how to integrate it into an Android app.
- An example Android app (`xmtp_dh/examples`). Used to run an end-to-end test.
- The `uniffi_bindgen_generator` crate. A boilerplate binary crate used to [generate the platform-specific bindings](https://mozilla.github.io/uniffi-rs/tutorial/foreign_language_bindings.html#multi-crate-workspaces) in the other crates. Will be used for `bindings_ffi` soon as well.

This review is not as big as it looks - **you can ignore everything in the Android example app under `xmtp_dh/examples` except for `MainActivity.kt`**.

Credit to @michaelx11 - I reused a lot of the structure from his vmac prototype here.
@nmalzieu
Copy link
Contributor Author

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
Copy link
Contributor

nplasterer commented Jun 1, 2023

@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
Copy link
Contributor Author

nmalzieu commented Jun 6, 2023

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 #51

@nplasterer
Copy link
Contributor

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)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants