Performing fragment transactions is noticably slower than when performing them manually

Zhuinden / simple-stack

[ACTIVE] Simple Stack, a backstack library / navigation framework for simpler navigation and state management (for fragments, views, or whatevers).

Apache License 2.0

1.36k stars 76 forks source link

Performing fragment transactions is noticably slower than when performing them manually #233

Closed topnax closed 4 years ago

topnax commented 4 years ago

Imagine this scenario:

you launch your app, display a login screen, so far so good
your app then goes into the background and listens for incoming events (may it be intents or HTTP requests)
an event comes -> onResume is called -> the event contains login credentials
once we receive such event we immediately set the stack history, displaying a loading fragment (in onResume())

Expected behaviour:

when the app comes from the background we do not see the login screen, but the loading screen

Reality:

for a brief moment the login screen is shown

Using just fragment transactions, the login screen is not visible while being moved to the foreground (commitNow() is used). Any ideas how to improve this? I guess this can be fixed in the DefaultFragmentStateChanger class.

Thank you very much for the rest of the amazing library

topnax commented 4 years ago

Might be fixed by #234

topnax commented 4 years ago

I can confirm that disabling fragment transactions animations fixes this issue.

Zhuinden commented 4 years ago

Hrmm, I'm not sure why the fragment animation makes that happen (fragment animations always live their own life to some degree ☹️) , but as outlined in https://github.com/Zhuinden/simple-stack/issues/234#issuecomment-674445940 the animations are customizable (through override).

The reason why fragmentTransaction.commitAllowingStateLoss() is used, is because fragmentTransaction.commitNow*() gets "confused" when DialogFragments are involved. But as fragmentTransaction.commitAllowingStateLoss() is used, supportFragmentManager.executePendingTransactions() is used if there would be overlapping fragment transactions. 🤔

So anything that is not commitAllowingStateLoss() in this pattern hasn't been as stable, which is why DefaultFragmentStateChanger doesn't expose a way to change it at the moment. The delayed execution of FragmentTransactions can be overridden through via supportFragmentManager.executePendingTransactions() if truly needed. 🤔

Zhuinden commented 4 years ago

I truly do wonder where the slowness comes from. I haven't experienced anything of the sort, and it's been a while... I might need a sample but fragments do have their own way of doing things.

Zhuinden commented 4 years ago

Theoretically, animations shouldn't slow down the views, but it's true that due to the default behavior of attach/detach, it can be slow (just like replace.addToBackStack, as this is the only way to reliably trigger the right lifecycle events in Fragments atm).

commitAllowingStateLoss() is used as it is the only way to safely trigger fragment transactions when DialogFragments are involved, commitNow() breaks that.

Unfortunately, with this amount of info, all I can say is "I recommend checking for the inflation time with a profiler, and if that is the culprit, maybe consider using show/hide like https://github.com/Zhuinden/simple-stack/issues/235#issuecomment-679924722"