Skip to content
This repository has been archived by the owner on Sep 20, 2024. It is now read-only.

Loader UI: Speed issues of loader with sync server #3199

Merged
merged 2 commits into from
May 18, 2022

Conversation

iLLiCiTiT
Copy link
Member

Brief description

A little bit enhanced loader UI speec when sync server is enabled.

Description

Cache sync server related information for some time to avoid refresh of sync server on each selection change.

Additional info

Loader was refreshing sync server module, it's setting for all projects on each change of context in loader UI which is also happening separatelly for subset widget and representation widget.

This is a "quick" change, full fix would require to modify some logic in sync server and partially rewrite how loader UI logic works.

Testing notes:

Loader didn't change but should be faster when sync server is enabled.

@ynbot
Copy link
Contributor

ynbot commented May 17, 2022

Task linked: OP-3261 Loader speed issues

@iLLiCiTiT iLLiCiTiT self-assigned this May 17, 2022
@iLLiCiTiT iLLiCiTiT requested review from kalisp and 64qam May 17, 2022 16:12

repre_icons = lib.get_repre_icons()
repre_icons = lib.get_repre_icons()
Copy link
Collaborator

@BigRoy BigRoy May 17, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you want to cache this too? Or isn't this called as frequently?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know. I just made quick fix so it's usable in production...

@BigRoy
Copy link
Collaborator

BigRoy commented May 17, 2022

I feel like this caching mechanic should somehow actually be IN the Site Sync module and directly in the Loader. Like maybe be a class loaded from the Module and used in the Loader so that the logic is where its closer related to?

Hmm - not sure it's easily doable now. But maybe something to note for the future to see if we can separate this better.

@BigRoy
Copy link
Collaborator

BigRoy commented May 17, 2022

This is a "quick" change, full fix would require to modify some logic in sync server and partially rewrite how loader UI logic works.

If we are merging this lets keep an issue open somewhere to remind us of doing that cleanup.

@iLLiCiTiT
Copy link
Member Author

iLLiCiTiT commented May 17, 2022

I feel like this caching mechanic should somehow actually be IN the Site Sync module and directly in the Loader.

I would say that not. This is logic using Sync server, but it is Loader tool specific. Hard to tell at this moment. Proper fix will need a lot of work before we get there.

If we are merging this lets keep an issue open somewhere to remind us of doing that cleanup.

Opened #3201

@iLLiCiTiT iLLiCiTiT merged commit 85c5b7a into develop May 18, 2022
@iLLiCiTiT iLLiCiTiT deleted the enhancement/OP-3261_Loader-speed-issues branch May 18, 2022 08:40
@mkolar mkolar added the type: enhancement Enhancements to existing functionality label May 24, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type: enhancement Enhancements to existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants