Low performance in Reports tab for users with thousands of places

[latin-panda]

Describe the bug
Report tab is slow on instances with thousands of places, some numbers:

training-2: 72k takes 20s
training: 1.9M takes 2min

To Reproduce

1. Log in as admin (desktop, browser)
2. Load the reports tab
3. Load the people tab
4. Load the reports tab
   On the second load, the reports tab is very slow, maxes out a CPU, and eventually logs in the browser console Uncaught RangeError: Maximum call stack size exceeded.

Expected behavior
Should not be slow.

Environment

• Instance: chis training instances
• Browser: Firefox & Chrome
• Client platform: MacOS
• App: webapp
• Version: verified in 4.2.0 and 4.4.0, possible also in 3.17

Additional context
Angular seems to have infinite recursion from internal calls from refreshView(tView, lView, templateFn, context) to refreshEmbeddedViews(lView) and back again.

[latin-panda]

For the record, things that we considered; not necessarily we're going to do it exactly like this, but these are ideas that can help improve performance:

• Work out what's causing the recursion in angular
• Consider pagination or autocomplete search instead of listing all places in a tree
• Delay rendering all filters until the filter button is clicked - we know they're not used much
• Permission to turn filters on/off to online and offline users (admin users will always see filters)

[latin-panda]

@garethbowen I've pushed the last commit, and this is ready for handover and get it to the finish line. Thanks a lot!

[garethbowen]

I have reproduced this issue on 3.17.x.

[garethbowen]

Testing plan...

• Create 1000 identical places.
• Ensure one has a hierarchy to test expansion
• Use top to watch CPU usage for the browser process
• Load the Reports tab, click People, click Reports

Before the change: CPU spikes to 100% for a few seconds
After the change: CPU never gets to 100% and settles back to 0 quickly

Testing matrix...

• Firefox, Chrome, Android webview
• Feature flag for material UI on vs off

[kennsippell]

Hey @garethbowen - I'm getting some questions from both CARES app and NSSD app about how much this fix will impact performance for their offline users. CARES users have ~5600 contacts on their device (dunno if that's helpful). SInce you have this repro setup already - are there some time measurements before/after the fix which would be helpful to answer this? Maybe CPU throttling?

[ngaruko]

Started to test this on local.
Steps, FWIIW: (WIP

1. Run cht on local
2. Using cht-core, modified const USERNAME = 'medic'; and const PASSWORD = 'password'; and DB_NAME: 'medic', to use my local credentials.
3. Created a js file with the following code (with slight modifications re paths to files)

const uuid = require('uuid').v4;
const placeFactory = require('../../tests/factories/cht/contacts/place');
const places = [];
for (let i = 0; i < 1000; i++) {
  const place = placeFactory.place().build({name:`Test Place ${i}`, _id: uuid, type: 'district_hospital' });
  places.push(place);
}
utils.saveDocs(places); 

Now I have 1000 extra places to test with... (Now getting my head around how to read/understand outputs from top command)

[ngaruko]

Just checking if I am doing the right thing @garethbowen . I can see a jump in CPU usage popping up for a while. Is this what we are supposed to check (note this is on master, but I also saw some spike with the branch, so I wanted to check before continuing) > See recording

[garethbowen]

@kennsippell Yes it might help them. Are the contacts places or people? The way to easily check is to open the places filter on the reports tab. If it's a long list (1k or above) then they will probably be getting this error. If places in the filter are being rendered one by one, then you've hit this issue for sure. This issue only impacts the Reports tab and you can use the other tabs just fine as long as you never go to the reports.

On my beefy desktop loading 1k places went from about 3 seconds of 100% CPU to basically instant. Much of this happens in the background so if you navigate to another screen it still feels sluggish until it's finished processing the places.

Happy to jump on a call to debug specific apps if that's helpful!

[garethbowen]

@ngaruko Looks right to me. The processes have different names in different OSes but the plugin-conta looks like a Firefox process on OSX. That's the one that spikes to 100% for a few seconds when you load the Reports tab so I'd say that looks like you're reproducing it just fine. Please try the PR and see if plugin-conta behaves better.

[ngaruko]

Thanks @garethbowen . I got a little confused on the PR, there was another process spiking to over 100% (com.apple.vi), but the plugin-conta definitely looks OK

[garethbowen]

@ngaruko Thanks. Can you also test that the filters, particularly the place filter, still functions as you'd expect? Also keep an eye on performance as you test the filters...

[ngaruko]

@garethbowen from a performance POV, it looks ok. However, some search terms bring results that do not contain any of the elements of the term, ie in this screenshot, search term 65, results include places which have nothing like 65 in their name or in the details. Haven't tested this on master yet, so it might have to do with the way the filter works.

Recording here

[garethbowen]

IIRC short search terms are ignored so searching for "65" is the same as no filter.

The change was actually in the Places filter on the Reports tab where it looks like you're testing the Freetext (or Search) filter on the People tab. Can you confirm you tested all the filters on the Reports tab?

[ngaruko]

All the filters on the reports tab work as expected, but then again I didn't load a lot of reports, enough to gauge the performance. Is that critical @garethbowen ?

[garethbowen]

I was mostly interested in double checking the filters work, but with and without the feature flag turned on. I had tested the performance thoroughly as part of the fix. Thanks!

[craig-landry]

I understand we were hoping #8577 would resolve this issue. Today on traffic-lights it was noted that the issue persists.

[kennsippell]

@craig-landry To clarify - we were hoping this issue would ease some of the client-side performance complaints we have been hearing from NSSD and CARES projects. Although this is an important performance improvement and does seem to net gains for some users -- it doesn't seem to net significant gains for offline users in these specific two projects. The reason is the shape of their contact data - one has a lot of places but they aren't directly under the user's facility and the other has a lot of contacts but they aren't places.

[garethbowen]

Merged and backported to 4.4.x, 4.3.x, 4.2.x, and 3.17.x.

[michaelkohn]

I read through all the comments on this issue and it's not clear to me what the actual solution was and what, if anything, will look different to users (other than it hopefully being faster now). Was it one of the options mentioned in this comment?

[garethbowen]

From the PR the 3 changes were...

1. Preventing running old filter's code related to element scroll.
2. Loading facilities in the filter only when the sidebar is open, and we haven't fetched the facilities yet. We had to change how the default facility filter works
3. Loading forms in the filter only when the sidebar is open, and we haven't subscribed to the forms yet.

Number 1 doesn't change the UI in any way. This was the main issue and was caused by a mix up between the old filter code and the new filter code.

Number 2 and 3 may introduce slight delays when opening the filter sidebar because we're delaying rendering the filter options from page load to filter open. I think this is a good tradeoff because most people don't open the filters on most page loads. Other than the slight delay, there should be no UX change.