Optimised connection params building#852
Conversation
🦋 Changeset detectedLatest commit: cb29c3c The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
📝 WalkthroughWalkthroughRefactors connection query parameter assembly in SignalClient: replaces list-based building with a StringBuilder and helper Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧹 Recent nitpick comments
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧬 Code graph analysis (1)livekit-android-sdk/src/main/java/io/livekit/android/room/SignalClient.kt (1)
🔇 Additional comments (2)
✏️ Tip: You can disable this entire section by setting Comment |
|
Hmm, it doesn't really make sense that the string concatenation here would account for such large slowdown; 150ms is a very long time for just adding 10 or so strings together. There's probably a separate underlying issue here. Nonetheless, it's still a good optimization to have here though. Thanks for the PR! |
|
I was benchmarking in the sample app in a debug environment so that could add to the reason of slow execution but as you said, nevertheless it's a good optimisation because for each query param we are concating twice so for about 10 parameters that's 20 concatenations plus variable assignments due to the nature of |
I noticed creation of
wsUrlStringtakes over 100ms, sometimes taking close to 150ms on my test device(Oneplus Nord CE 2 Lite). The function just builds a string of query parameters but does it inefficiently by using string concatenation. I replaced it withStringBuilderand prepared the string as we go. This brought down the execution time down to ~5ms or lesser.Summary by CodeRabbit
Refactor
Chores
✏️ Tip: You can customize this high-level summary in your review settings.