Core Apps: Patient dashboard to load encounters in batches

Hi all,

We need to have some sort of pagination (or batch loading) of the encounters on the patient dashboard. When there are too many encounters to display, this page could take up to minutes to load, while at the same time just paralysing the backend for every other logged in user.

Culprit here in VisitDetailsFragmentController.java:

for (Encounter encounter : visitWrapper.getSortedEncounters()) {
  encounters.add(parseEncounterToJson.createEncounterJSON(authenticatedUser, encounter));
}

createEncounterJSON(..) when called many times hits some serious performance issues.

I would like to load the encounters by batches whose size would be set through a GP. And, for instance, add a ‘load more’ button that would trigger the loading of the next batch of encounters. That’s easy enough to do for the first batch, but it’s the JavaScript here in visitDetailsTemplate.gsp that bothers me:

[[ _.each(encounters, function(encounter) { ]]
  [[= encounterTemplates.displayEncounter(encounter, patient) ]]
[[ }); ]]

Admittedly I don’t know much about Underscore.js and those templates patterns. (Q.) I guess my question is: how can I resend an updated encounters array to the client (array that is the previous one + a new batch of encounters) and have this above JS loop to be re-evaluated so that the display is updated accordingly? Or otherwise, according to you, what would be the least costly way to achieve this basic pagination system?

Any ideas/suggestions would be really welcome!

Cc: @darius, @mogoodrich, @dkayiwa

1 Like

@mksd, I don’t really have anything useful to say, but I’m going to reply anyway so you don’t wait for me to eventually reply with something intelligent.

Currently, that screen is an unholy mess that includes, IIRC, templates in GSP, in underscore, and in knockout. And it’s extremely difficult to follow the logic unless you spend hours tracing it all through. The right thing to do is completely rewrite this using just one single client-side technology (i.e. Angular). (Actually when I was still at PIH I started doing a new implementation of the visit dashboard for the PIH-EMR, and @mogoodrich and others have now finished this off and put it into prod, but it’s not a drop-in replacement, and I don’t know how much work it would take to generalize it beyond the PIH use case.)

You’re asking for a quick fix though. For that I would look at:

  • Could it be that parseEncounterToJson.createEncounterJSON is just really inefficient and you can trivially speed it up?
  • Just hack the page so it only loads one page, e.g. visit.page?visit=uuid&fromEncounter=0&size=20 (with links to the previous and next pages). But you’d implement this on the server side, so fetching the next page is doing another entire page load with fromEncounter=20&size=20.

Thanks @darius, that’s really insightful. And your second suggestion is exactly the kind of input that I needed.

I 100% agree that this page should be deprecated and replaced with something better both from the clinical and technology standpoints. But at this juncture there is only development room for a quick fix that makes the visible problem go away. The target implementation that this applies to may be migrated to Bahmni anyway, and in that case there is no budget/time/room to look at more than just a quick fix.

I’ll look into your suggestions, especially #2, and I’ll report back.

How many times? Hundreds, thousands? Can you profile it ?

If in a real world use case there are not so many encounters per patient, maybe it’s not worthwhile to do the hack.

Our case is a real-world case: at the hospital in question, care providers have to be ‘careful’ not to click on the handful of inpatients that have hundreds of encounters in their inpatient visit.

I have no time to profile this in depth since there is an absolute emergency to get the patch out, and @darius’s idea leveraging on additional URL parameters sounds easy enough to get it sorted quickly. However to give some insight I can say that things are already problematic in the 200 encounters zone.

200 is a very low number, it’s weird that it takes so long.

I agree that adding paging is a good solution but it’s not clear to be cheaper than locating the performance problem and eventually fixing it. I don’t say to do a full profiling, sometimes just debugging shows where milliseconds are spent. Could be lots of Hibernate objects loaded for nothing, JSON serialization …

Although I agree with @lluismf’s assertion that ideally the behaviour of ParseEncounterToJson.createEncounterJSON(User, Encounter) should be studied to highlight (and solve) the actual performance issues, would it be ok to create an issue and submit a PR that addresses solely the paging issue for now?

@darius, @dkayiwa?

If yes I’ll create the issue right away. I have already the UI side working with basic paging through URL params, I now need to implement the backend and update/add unit tests.

To everyone: let’s keep in mind that this is an almost invisible disruption to anyone else relying on the current behaviour of the page. The GP controlling the page/encounter batch size will be defaulted to something big enough (like 100 or so) so that no change should be observed to the webapp’s behaviour. And for those that might, like us, hit the problem zone, well, they’ll have a way out through this GP.

@mksd i personally have no problem with you doing such a pull request. :slight_smile:

Great! And do you require a RA-* JIRA issue or things can be discussed here, on IRC… etc?

@mksd, yes, go ahead and do this.

Please create a JIRA ticket in the RA project, and share a link to it here.

@mksd we generally prefer having discussions from talk, because of the bigger audience. The JIRA tickets can just link the talk thread.

Here: RA-1244.

I changed VisitDetailsFragmentControllerTest to have a visit with 1000 encounters and it is fast (almost immediate), but a unit test with an in-memory DB is not appropriate for load testing of course. No idea why the performance is so bad. Maybe it’s not the controller ?