firebase / quickstart-unity

Firebase Quickstart Samples for Unity
https://firebase.google.com/games
Apache License 2.0
820 stars 424 forks source link

Performance issue in CharVector.CopyTo when calling RemoteConfig.GetValue #743

Open jjfmarket opened 3 years ago

jjfmarket commented 3 years ago

Please fill in the following fields:

Unity editor version: 2018.4 Firebase Unity SDK version: Firebase Remote Config Version 6.15.2 Source you installed the SDK (.unitypackage or Unity Package Manager): Unity Package Manager Firebase plugins in use (Auth, Database, etc.): RemoteConfig Additional SDKs you are using (Facebook, AdMob, etc.): Platform you are using the Unity editor on (Mac, Windows, or Linux): All Platform you are targeting (iOS, Android, and/or desktop): iOS, Android Scripting Runtime (Mono, and/or IL2CPP): IL2CPP

Please describe the issue here:

(Please list the full steps to reproduce the issue. Include device logs, Unity logs, and stack traces if available.)

  1. Setup the firebase quickstart for unity remote config testapp
  2. Add a very long string as a Remote config value with the key 'really_big_string'
  3. Fetch the remote config with FirebaseRemoteConfig.FetchAsync
  4. Call FirebaseRemoteConfig.GetValue("really_big_string");

Note that the amount of time the FirebaseRemoteConfig.GetValue takes scales linearly with the number of bytes in the string. If you step into the GetValue call you can observe the CharVector.CopyTo calls Array.Copy for every byte in the string, rather than doing a single Array.Copy for the entire string.

For larger strings this ends up taking quite a bit of time, and can cause a framerate loss.

Please answer the following, if applicable:

Have you been able to reproduce this issue with just the Firebase Unity quickstarts (this GitHub project)? Yes

What's the issue repro rate? (eg 100%, 1/5 etc) 100%

DellaBitta commented 3 years ago

Hi @jjfmarket,

It seems that on desktop implementations there is indeed a for loop copying a character at a time, but we call into the Android and iOS implementations to faciliate the GetData requests on those platforms. Could you please confirm which platforms (Desktop or Phone SDKs) that you're experiencing this performance hit so we can better understand the scope? Is it only desktop or the phones as well?

Thanks!

jjfmarket commented 3 years ago

Hey @DellaBitta,

We were seeing a hitch in our iOS/Android apps, which is what prompted the investigation that discovered this issue. I haven't stepped though the compiled device code to confirm that it's running through that same CharVectory.CopyTo code path, but I can confirm that we were experiencing the performance problem on phones.

DellaBitta commented 3 years ago

Ok, thank you. I'll track this as an internal ticket for investigation.

jjfmarket commented 3 years ago

Great, Thanks a bunch. Keep me posted 😁😁

Usslessvoid commented 1 month ago

Same here. Its repeats on Android. Array.SetValue called 7000+ times. How to fix this?