-
Notifications
You must be signed in to change notification settings - Fork 0
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
contact_list apdex: research and development cycle 2 #98
Comments
Main research outcome
cc @garethbowen do you have any suggestions I could consider or comment on anything I may have missed that could improve performance ? Perhaps insights from previous discussions / code health issues. |
As this came up in a production instance I wouldn't be so quick to write it off as an outlier. We should put a limit on what's rendered on the RHS because showing 5k children is useless anyway. The main improvement in the search service is to do with freetext queries which don't behave as expected and slow down the entire application by generating indexes that are far too large. This isn't a quick win because we need to verify how people use the search service before we go changing how it behaves, but I think it will have a massive impact on the overall performance of the application. I recommend taking a scientific approach to these changes and measuring what improvements could be gained by a massive reduction in index size, for example, removing the field name prefix in the index (which I suspect is never used) and/or indexing only the name field. There is some more info on the forum: https://forum.communityhealthtoolkit.org/t/unexpected-search-results/3288 |
Gareth makes good points above
Possible Performance ImprovementHaving not found code inefficiencies calling out to be improved (kudos to the devs ), I experimented with reducing the number of docs fetched.
Performance when loading 50 records
Performance when loading 25 records
cc @michaelkohn and @n-orlowski is changing the number of docs fetched something we can consider ?
|
The reality is that on many of the mobile devices being used by CHWs, there are only 8-10 rows shown at a time anyway so they'd have to scroll through 2-5 pages of households before the second "load" happens (if we set it at 25). Desktop view will have more rows in the list but they'll also have better memory/processors. Also, I note that you mention testing with "Good 3G" but since these are offline users, I would not expect any network activity when loading the contact list anyway, if there is... that seems wrong. Here's an example of a user on a low spec phone loading the contacts list. It takes about 8-9 seconds to load and there are about 6 rows shown at a time. This is also an older version of the CHT (bottom action bar instead of FAB) so they'd see maybe 1 more row if they had the FAB. From a UX perspective, I'd be open to changing to 25 if the performance was significantly better in our target setup (low spec phone) and it was a quick change... as long as @n-orlowski was OK with the UX. |
+1 changing to 25 with that loading data |
Conclusion of cycle 2
|
Cool, thanks @Benmuiruri . So the goal for Monday will be to get a baseline loading on low spec device, make the code change, test again, determine whether it is worth the change. I'm going to close this ticket and create another one for the next sprint. I'd also like to understand / explore the index update route in more detail as well. Let's follow up and think of a plan for that next week. |
Iteration 1 results here 👇
The text was updated successfully, but these errors were encountered: