Skip to content
This repository has been archived by the owner on Mar 15, 2022. It is now read-only.

Improve overall app performance #62

Open
Skrilltrax opened this issue Jul 3, 2020 · 23 comments
Open

Improve overall app performance #62

Skrilltrax opened this issue Jul 3, 2020 · 23 comments
Labels
enhancement New feature or request

Comments

@Skrilltrax
Copy link
Contributor

Please only report your issue here, if all points below are true

  • I installed the App from vanced.app, this github repository or the Vanced Discord server
  • I am using the latest version
  • This is an issue in the Vanced Manager app (NOT Youtube Vanced)
  • This issue keeps re-occurring every time I try
  • For MIUI users: I disabled MIUI optimisation
  • For root users: I disabled apk Signature Verification

Phone Specifications:

  • Brand: Xiaomi
  • Operating System: PixelRom
  • Android Version: Android 10
  • Vanced Manager Version: -

Please describe the problem you are having in as much detail as possible:
Hey guys, I just installed the app to try and test it but the app startup time was huge on my phone. I'm probably thinking this is due to a lot of fragments that are being loaded during app startup. I've forked the app and found some changes that can improve the app performance a bit.

Steps to reproduce:
Generally navigate around the app. Also Logcat was spamming with skipped frames message.

Further details:

@Skrilltrax
Copy link
Contributor Author

I'll definitely try and make a PR to the app if you guys are accepting contributions.

@KevinX8
Copy link
Contributor

KevinX8 commented Jul 3, 2020

We do accept PRs, preferably to Dev as that's the working branch, master is for release ready code

@X1nto
Copy link
Member

X1nto commented Jul 3, 2020

Feel free to contribute, but please try to open PR on dev branch instead of master

@Skrilltrax
Copy link
Contributor Author

Sure 👍

@X1nto X1nto closed this as completed Jul 4, 2020
@Skrilltrax
Copy link
Contributor Author

Hey, do you mind if we keep this open? The data binding PR does not solve the issue. I will be contributing more to help reduce the overall startup time.

@Vendicated Vendicated reopened this Jul 4, 2020
@X1nto
Copy link
Member

X1nto commented Jul 4, 2020

Well, sure. I don't really think that app loads all fragments on start, but I can't deny it either because I don't know how navigation libs work. Feel free to make a PR whenever you find a way to improve app

@Skrilltrax
Copy link
Contributor Author

@X1nto Yeah fragments may not be an issue at all but the app is still doing a lot of processing at startup. I'll try and look at other parts that may be causing an issue. Also, stuff like switching between root and non-root mode increases the load on the main thread, I think some of that work can be pushed over to the background thread for a better UX.

@X1nto
Copy link
Member

X1nto commented Jul 5, 2020

Hmm, might be issue with loading data from the server, but as far as I know getjson library uses AsyncTask to load data, so it should be fine. Switching from root and nonroot just recreates activity with overridePendingTransition, so the issue should be with home fragment I guess, or it's viewmodel

@X1nto X1nto added the enhancement New feature or request label Jul 5, 2020
@ghost
Copy link

ghost commented Jul 5, 2020

@X1nto Have you add a check to avoid multiple downloads of the same package (vanced or microg), after pressing the download button again?

@X1nto
Copy link
Member

X1nto commented Jul 5, 2020

@X1nto Have you add a check to avoid multiple downloads of the same package (vanced or microg), after pressing the download button again?

No, it's easier to redownload because sometimes downloaded package may be corrupted and to avoid this we redownload again. I don't see any issues here because 1. Wifi is unlimited 2. Who will download using mobile data?

@ghost
Copy link

ghost commented Jul 5, 2020

@X1nto

However I think that I have understand why the app stuck on the splashscreen at startup, and concerns the optimizations mentioned by skrilltrax: with slow network there are difficulties in contacting the server, and after a while the app forcely closed.

If will be happen again I will try to make a video.

@X1nto
Copy link
Member

X1nto commented Jul 5, 2020

@X1nto

However I think I understand why the app stuck on the splashscreen at startup, and concerns the optimizations mentioned by skrilltrax: with slow network there are difficulties in contacting the server, and after a while the app forcely closed.

If will be happen again I will try to make a video.

I don't see a reason why slow internet would slow down app loading speed. You don't need a good internet to load data from server, you need at least 10kbps and I'm pretty sure you have much better connection than this

@ghost
Copy link

ghost commented Jul 5, 2020

@X1nto

This is an example (this time there were no forced closings). 1 minute of stuck:

https://streamable.com/owgw9e

@ghost
Copy link

ghost commented Jul 5, 2020

@X1nto

To simplify your tests set your cellular data to edge (2g), open app and wait three seconds before totally disable cellular data. With this approach you can also trigger a FC 😁

https://streamable.com/4o7sln

@X1nto
Copy link
Member

X1nto commented Jul 5, 2020

@X1nto

To simplify your tests set your cellular data to edge (2g), open app and wait three seconds before totally disable cellular data. With this approach you can also trigger a FC 😁

https://streamable.com/4o7sln

That's not something I can fix, here's a explanation:
When you start app, it's on splash screen until app loads all the required data for it to show layout, when you have a slow connection, it takes a very long time to load data, Android sees that application shows a blank splash screen for more than 10 seconds and just kills app.
Solution: upgrade your network speed

@ghost
Copy link

ghost commented Jul 5, 2020

@X1nto
To simplify your tests set your cellular data to edge (2g), open app and wait three seconds before totally disable cellular data. With this approach you can also trigger a FC grin
https://streamable.com/4o7sln

That's not something I can fix, here's a explanation:
When you start app, it's on splash screen until app loads all the required data for it to show layout, when you have a slow connection, it takes a very long time to load data, Android sees that application shows a blank splash screen for more than 10 seconds and just kills app.
Solution: upgrade your network speed

Maybe you can implement a minimum timelapse (3 seconds?), before showing a window of download failed due to slow connection, and maybe add another one when the connection is totally lost (to avoid crash).

@Skrilltrax
Copy link
Contributor Author

@X1nto I think that can be fixed. The only data that needs to fetched from the server are changelogs. That can be lazily fetched whenever the user requests it. This will help in implementing a No Internet state in the app and will reduce the number of crashes that people experience.

@X1nto
Copy link
Member

X1nto commented Jul 5, 2020

@X1nto I think that can be fixed. The only data that needs to fetched from the server are changelogs. That can be lazily fetched whenever the user requests it. This will help in implementing a No Internet state in the app and will reduce the number of crashes that people experience.

Hmm, app loads latest versions too, not only changelogs. These are loaded in HomeViewModel

@Skrilltrax
Copy link
Contributor Author

We can fetch them after entering the HomeFragment. Internet speed shouldn't really be a reason for crash.

@X1nto
Copy link
Member

X1nto commented Aug 7, 2020

Just pushed new commits which fetch Json data asynchronously, without blocking the UI thread. This should fix the issue with splash screen getting stuck for some users with slow internet. e9c3e7a

@Skrilltrax
Copy link
Contributor Author

Skrilltrax commented Aug 8, 2020

@X1nto That's great. I was working on something similar but couldn't complete it. Another improvement that I would like to suggest would be using a Model class with retrofit to fetch all the data as a single object rather than fetching each item separately.

@X1nto
Copy link
Member

X1nto commented Aug 30, 2020

@Skrilltrax Added model class in b1e0db8 and honestly, I should've done it earlier. Fetching stuff is much easier now and doesn't require declaring thousand different variables

@Skrilltrax
Copy link
Contributor Author

Skrilltrax commented Aug 31, 2020

@X1nto I checked the commit and it looks great. However, your model class still fetches each item by itself which technically isn't the responsibility of a Model. Fetching should be done by a Repository. This can be changed by making a Data Class. I can link you to some examples if you want or I'll just make a PR as soon as I get time from college and work.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants