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

app slow to show configured servers on start-up #291

Open
ghost opened this issue Jul 6, 2020 · 14 comments
Open

app slow to show configured servers on start-up #291

ghost opened this issue Jul 6, 2020 · 14 comments

Comments

@ghost
Copy link

ghost commented Jul 6, 2020

It takes around ~2 seconds before the server is shown, until that time this is what is shown:

signal-2020-07-06-152754

Is there network activity at this stage? What is fetched? Why is it so slow? Why does it not happen in the background?

@dzolnai
Copy link
Collaborator

dzolnai commented Jul 20, 2020

The server_list, and organization_list JSONs and their signatures are fetched, then verified against each other.
After that the JSON needs to be parsed, and the view should be updated. This should all happen under 1 sec in optimal circumstances, but on a slower connection or device it might take a bit longer.

@ghost
Copy link
Author

ghost commented Jul 20, 2020

organization_list.json should NEVER be fetched on (subsequent) start-up, only when choosing an organization, which typically only happens once... As for server_list.json that could easily be a background thread that doesn't impact the loading of the UI.

@ghost
Copy link
Author

ghost commented Jul 20, 2020

@dzolnai
Copy link
Collaborator

dzolnai commented Jul 20, 2020

Oh sorry, I thought this was the search screen, my mistake.
There the app waits for the server_list, but I have applied a fix to not wait if the user does not have an organization added.
Otherwise it will wait 3 seconds, because we have some retry-after-failure which checks again after 3 seconds.
This is required because sometimes it wants to refresh this list when coming back from a disconnect, and it needs some time to get a working connection again.

@dzolnai
Copy link
Collaborator

dzolnai commented Jul 21, 2020

Should be better now.

@dzolnai dzolnai closed this as completed Jul 21, 2020
@ghost
Copy link
Author

ghost commented Jul 22, 2020

It is not. Just showing a banner that it is refreshing the list is not okay. There should be no banner and no delay. There is no need for network activity at all! At least not on the main thread. If it can update in the background, fine.

@ghost ghost reopened this Jul 22, 2020
dzolnai added a commit that referenced this issue Aug 24, 2020
@dzolnai
Copy link
Collaborator

dzolnai commented Aug 24, 2020

Server list is cached now for one hour.

@dzolnai dzolnai closed this as completed Aug 24, 2020
@ghost
Copy link
Author

ghost commented Aug 24, 2020

Actually you are not allowed to do that. The web server indicates information about caching!

Cache-Control: no-cache

See also: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Cache-Control

@ghost
Copy link
Author

ghost commented Aug 24, 2020

Just to make it clear: no-cache does NOT mean that you can't cache it, just that you need to validate at the server before using it (again).

@dzolnai
Copy link
Collaborator

dzolnai commented Aug 24, 2020

Okay, but that means that there has to be a network call, which you did not want:

Just showing a banner that it is refreshing the list is not okay. There should be no banner and no delay. There is no need for network activity at all!

@dzolnai dzolnai reopened this Aug 24, 2020
@ghost
Copy link
Author

ghost commented Aug 24, 2020

We are only talking about server_list.json in this case, as that is the only file that is potentially retrieved when showing the servers. So technically you are correct. When you use server_list.json to extract the display_name of the configured servers you would need to make a network request when showing the server list.

Using a conditional request you can skip a lot of work if the file on the server didn't change though (304 response), no need to verify the signature, no need to parse the JSON, no need to update the model...

I don't recall exactly what gets called when though.

Edit: not to mention this could be done in the background, not needing to block at all :)

@dzolnai
Copy link
Collaborator

dzolnai commented Aug 24, 2020

Using a conditional request you can skip a lot of work if the file on the server didn't change though (304 response), no need to verify the signature, no need to parse the JSON, no need to update the model...

That would be true if the models would be cached on that level (like they are since the last update).
But the HTTP cache works on the response level, so the only step we are saving here is fetching the response data. After that, the signature verification and JSON parsing still happen, because the app will not know if the response is a cached one or not (which it shouldn't).

Edit: not to mention this could be done in the background, not needing to block at all :)

All the processing already happens in the background. You see the loading bars until there is something to show, otherwise the user does not know why the list is empty.

@ghost
Copy link
Author

ghost commented Aug 24, 2020

Maybe this is the best we can do then. I'm just a bit worried about choosing a caching yourself, i.e. 1 hour, especially if the user cannot override this except by clearing browser cache (I guess)?

@dzolnai
Copy link
Collaborator

dzolnai commented Aug 24, 2020

To clear it, the app data has to be cleared, which removes all other saved data as well. I can reduce the 1 hour though.

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

No branches or pull requests

1 participant