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

Cross check load time is too slow #8

Open
moveson opened this issue Jun 4, 2023 · 4 comments
Open

Cross check load time is too slow #8

moveson opened this issue Jun 4, 2023 · 4 comments
Assignees

Comments

@moveson
Copy link

moveson commented Jun 4, 2023

For events with many entrants, the Cross Check view is very slow to load on older devices. For reference, we'll use the Quad Rock 50 event in the staging environment, which has 470 entrants.

Times using an iPad Mini Gen 3 running iOS 12.5.7:

Initial load of the Cross Check view: about 25 seconds
Subsequent loads of the Cross Check view: about 20 seconds
Filter changes (such as to show only Expected runners): about 4 seconds

Delays using Gen 1 and Gen 2 iPad Minis are even longer. I have heard reports of load times exceeding 1 minute.

These delays can be a real problem, because the device is completely useless in the period of time while the screen is loading, and there is no way to cancel the process. So an operator who is entering live times might tap "Cross Check" to see the status and might miss runners coming through while the Cross Check screen is loading. At least one RD has gone so far as to instruct users to never tap on Cross Check ever, for this very reason.

We should figure out what causes load times to run so long and see if we can improve performance such that Cross Check becomes useful for these larger events.

@moveson
Copy link
Author

moveson commented Jun 4, 2023

I just tested an event with 441 entrants in the production environment on an iPad Mini Gen 2 running iOS 9 12. Cross Check load time was about 45 seconds.

@mrozanski
Copy link
Collaborator

I'll take a look.

@mrozanski
Copy link
Collaborator

From email thread:

MARIANO - What we know:

In a local debugging environment the view loads in less than 5 seconds for 470 efforts (using iOS 16 which is the default on Xcode today).
In that context, it is not possible to troubleshoot and pinpoint the cause of the problem you're reporting.
The only way would be to reproduce it either in a debugging environment, or (worst case) using different builds with changes made blindly that you would install one by one and give me feedback. But that also requires that you can use Testflight or Firebase App Dist on the problematic devices which I'm not sure you can. And it's also a painful and tedious process I don't know if we want to go through.

At the code level, we also know that there are a number of things happening in that view that may be responsible, and that some of its patterns and libraries used are long outdated. Let's also keep in mind that updating dependencies is a big risk given the constraints imposed by OST Remote having to be usable on iOS 9. I don't want to go there and touch any dependency or anything that's not really necessary, at least so long as we want to keep supporting those older ipads.

Next steps:

I will try to revive my old macbook and try to debug locally on a legacy iOS simulator in case that lets me see the issue first hand.
I'm planning to timebox that effort so we don't go over a total of 5hs on this, and report back to you.

If you can install Testflight and test my builds on iOS 12 (the issue says the bug manifests in iOS 12, right?), please let me know, as that can be an alternative way to troubleshoot.

MARK - Sorry, to answer your question, I do see very slow load times on iOS 12 as well as iOS 9. But I think the slow load times have much more to do with the CPU than they do with the OS version.

I would say a 5-second load time for 470 efforts is much too long on a modern device/simulator. I was hoping there would be some obvious issue with the logic that builds the view, but I understand that issues with dependencies can be much harder to track down. Thanks in advance for your help.

MARIANO - I made some progress reproducing the issue.
There are a couple of things that may mitigate the slowness without changing any library that I'd like to try.I'm running the app on a rather slow simulator (iPhone SE gen 2) on my old laptop, which is also kinda slow compared to the M1 I use every day.
This allows me to get closer to the real life issue, with a load time of about 10 seconds.

It would be helpful to review the history of the Cross Check feature, I don't remember the requirements in detail, and I see a network call when that view is opened and filters are applied that tries to fetch "not expecteds" from the remote API.That looks a little off to me and it contradicts the "offline first" approach I thought we were taking.
MARIANO -
I've found this when searching through our email history:

On Wed, Aug 29, 2018 at 12:30 AM Mark Oveson (Gmail) <[email protected]> wrote:
Mariano and Luciano,
(...) 
4. "Do Not Expect" auto update: When the user syncs, the response would include a "doNotExpect" key that provides an array of bib numbers that either did not start or dropped at an earlier point on the course. OST Remote would then automatically mark these bib numbers as "Do Not Expect" in the cross-check screen.

...
Do you remember if it was specified that when rendering the Cross Check screen the app should try to sync again from the server?
Concerns:When I read that, I understand that the view should work offline, and use the latest response from a previous sync action. Right now however, the app is going to the server every time you open the cross check and when you apply a filter.I can't find the exact string doNotExpect in the code. Did that value change after the email conversation I quoted above?

MARK - Sounds like good progress. Indeed, Cross Check does attempt to call home to get further information. It will still function offline but the network call, if successful, provides additional functionality.

We ended up doing an entirely different endpoint for the "do not expect" data. The endpoint is:

GET /api/v1/event_groups/1/not_expected

And the JSON response looks like:

"{"data":{"bib_numbers":[101,105,109,111,114,134,140,222,333,444,777,999]}}"

@mrozanski
Copy link
Collaborator

I think we can rule out that the fetchNotExpected and the second dispatch_async have any effect on the render time based on the following tests.

On an older mac and Xcode running an iPhone SE 2nd gen 14.0

Time to finish rendering the initial state with 470 efforts:

11 seconds:

As it currently is in the codebase.

11 seconds:

If I only comment out fetchNotExpected and leave the second dispatch_async uncommented so this happens in its own thread

self.efforts = entriesThatShouldBeHere;
[self applyFilter];

11 - 17 seconds:

(multiple times testing in the same conditions give different results)

If I:

  • comment out fetchNotExpectedthed here
  • comment out the second dispatch_async but leave the callback contents (so they run synchronously)

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

2 participants