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

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

Merged
iLLiCiTiT merged 2 commits into
developfrom
enhancement/OP-3261_Loader-speed-issues
May 18, 2022
Merged

Loader UI: Speed issues of loader with sync server#3199
iLLiCiTiT merged 2 commits into
developfrom
enhancement/OP-3261_Loader-speed-issues

Conversation

@iLLiCiTiT
Copy link
Copy Markdown
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
Copy Markdown
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 64qam and kalisp May 17, 2022 16:12
remote_provider = 'studio'

repre_icons = lib.get_repre_icons()
repre_icons = lib.get_repre_icons()
Copy link
Copy Markdown
Member

@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
Copy Markdown
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
Copy Markdown
Member

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
Copy Markdown
Member

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
Copy Markdown
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