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

Slow loading of "ALL Guides" tab #375

Closed
perfprobe opened this issue Jul 6, 2017 · 9 comments
Closed

Slow loading of "ALL Guides" tab #375

perfprobe opened this issue Jul 6, 2017 · 9 comments
Assignees

Comments

@perfprobe
Copy link

perfprobe commented Jul 6, 2017

Dear developers,

We are applying our performance diagnosis tool PerfProbe to debug the long latency for clicking "Guides" -> "ALL" tab. We observe that the loading time for this user interaction is quite long (on average around 25 seconds and can increase to longer than 45 seconds in our test environment). Through its system-wide profiling and tracing, PerfProbe discovers that the source of extra delay results from longer delay in network blocking for object downloading during the execution of Android's API call libcore/io/Posix.recvfromBytes, which is invoked by get method calls inside getAllGuides method call in INaturalistService class. Based on our investigation of the source code, the getAllGuides method call is issuing sequential HTTP GET request for the link "guides.json?/per_page=200&page=x" page by page.

We hope the findings from our tool can be helpful for your debugging. We are also interested in helping improving the performance of this interaction. One suggestion to improve the latency that we can come up with is to limit the number of results retrieved through HTTP GET request and add a "Load more" option in the UI for loading more results. Please let use know if it will work or not. Thanks for your attention!

Steps to reproduce:
  1. Click the Menu tab
  2. Click "Guides" tab
  3. Click "ALL" tab
Environment:

App version: 1.5.10 (207)
Android version: Android 4.4.4
Device: Nexus 4

@kueda
Copy link
Member

kueda commented Jul 6, 2017

If you would like to implement such a change, we await your pull request.

@perfprobe
Copy link
Author

Thanks very much for your quick response! Sure, we will make a pull request and try to implement our suggested improvement in the current code base.

@perfprobe
Copy link
Author

perfprobe commented Oct 9, 2017

@kueda I have implemented my suggested strategy and validated its improvement of the UI responsiveness. This fix consistently ensures loading time not more than 6 seconds and achieves 8x speedup on the worst-case loading time. Below are the source files I made changes on top of the code base (version 1.6.1, vcode221) and would like to make a pull request to push my implementation to a new branch (i.e., loadmore_v1.6.1_vcode221). My implementation adds around 90 lines of code in total. Could you please grant me pull request permission for pushing my branch? Thanks very much!

modified:   iNaturalist/src/main/java/org/inaturalist/android/BaseTab.java
modified:   iNaturalist/src/main/java/org/inaturalist/android/INaturalistService.java
modified:   iNaturalist/src/main/res/layout/project_list.xml
modified:   iNaturalist/src/main/res/values/strings.xml

@kueda
Copy link
Member

kueda commented Oct 9, 2017

Hey @perfprobe. You don't need extra permissions to make a pull request. Just fork this repo, commit the changes in your fork, and issue the pull request from there. Best practice is usually to make an issue-specific branch in your fork, e.g. 375-slow-loading-of-all-guides-tab or something, and issue the PR from there, but just forking and making the PR is enough for us to check out your proposed changes.

@perfprobe
Copy link
Author

perfprobe commented Oct 9, 2017

Hi @kueda Thanks very much for your instructions! I have made a pull request following your suggestions.

@tiwane
Copy link

tiwane commented Dec 7, 2017

Just noting that loading the all guides tab still takes a long time.

@perfprobe
Copy link
Author

Hi @tiwane That's possible, as my pull request has not yet been merged. I am working with the developer to integrate my pull request into the current code base. Will keep you posted.

@budowski
Copy link
Contributor

Eventually, we went with a simple solution - just show the first page of results for the "All guides" tab. Since this amounts to 200 results, that is plenty - if a user is looking for a specific guide, he can just search for it (instead of a lot of scrolling).
Thanks for the effort @perfprobe, we appreciate it!

@perfprobe
Copy link
Author

Right, that will significantly reduce the user waiting time. You are welcome! @budowski

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants