UEPlasticPlugin icon indicating copy to clipboard operation
UEPlasticPlugin copied to clipboard

Plugin hangs the editor for long periods while generating context menu entries

Open Acren opened this issue 1 year ago • 8 comments

Hey there,

We're currently using 1.11.0 in UE 5.4, and I've been noticing that when I hover an asset in the context menu the first time the editor will hang for 30-60 seconds.

It looks like in this case the plugin is waiting on cm commands synchronously.

FScopeLock::FScopeLock(FWindowsCriticalSection *) ScopeLock.h:39
PlasticSourceControlShell::RunCommand(const FString &, const TArray<…> &, const TArray<…> &, FString &, FString &) PlasticSourceControlShell.cpp:394
PlasticSourceControlUtils::RunCommand(const FString &, const TArray<…> &, const TArray<…> &, TArray<…> &, TArray<…> &) PlasticSourceControlUtils.cpp:52
PlasticSourceControlUtils::RunListLocks(const FPlasticSourceControlProvider &, const bool, TArray<…> &) PlasticSourceControlUtils.cpp:534
PlasticSourceControlUtils::GetLocksForWorkingBranch(const FPlasticSourceControlProvider &, const TArray<…> &) PlasticSourceControlUtils.cpp:558
FPlasticSourceControlMenu::GeneratePlasticAssetContextMenu(FMenuBuilder &, TArray<…>) PlasticSourceControlMenu.cpp:195

This can be quite frustrating when hovering something locks the editor for so long. Would it be possible to make this async?

Thanks, Sam

Acren avatar Aug 15 '24 05:08 Acren

Hi @Acren, thanks for the report.

I actually also found myself this issue quite recently; it's indeed the UI waiting for the status of locks! After digging I found out that it's a common issue across all source control providers including Perforce: the menu wait for SCC status, calling the serveur! At that time I couldn't find a way to populate a submenu asynchronously in Unreal, but perhaps I missed the proper way?

Independandlty from fixing the UI, potential fixes could be;

  1. to make sure that the lock status are always in the cache before hand
  2. if not, either making sure it never take so long (more on that on a following message)
  3. or relying on a default behaviour that doesn't require the lock status (not disabling entries in the menu)

In any case, it means that Unreal expect the provider to answer quickly; for sure in my own testing the issue wasn't as bad as yours, as I was only seeing sub second latencies. It means that I may have missed an important use case;

SRombauts avatar Aug 15 '24 10:08 SRombauts

Would you be able to send me the Unreal logs of a minimal session showing the issue? Ideally I could also make use of the logs from the CLI (cm.exe) https://github.com/SRombauts/UEPlasticPlugin#enable-debug-logs

If you prefer, creating an official support ticket might be better for confidentiality, and help us sort the priorities Unity Version Control support.

Many thanks, Sébastien

SRombauts avatar Aug 15 '24 10:08 SRombauts

Thanks @SRombauts, I'll gather those logs and create a ticket referencing this thread.

As for making the menus async, I noticed that there were some generic source control submenus in the editor that are async already. Check out AddAsyncMenuEntry in AssetSourceControlContextMenu.cpp, looks like it has a bespoke approach using custom widgets to show a loading state until it's done.

https://github.com/EpicGames/UnrealEngine/blob/575944b129b9d2f1a30424accdf026712e93bbd2/Engine/Plugins/Editor/AssetManagerEditor/Source/AssetManagerEditor/Private/AssetSourceControlContextMenu.cpp#L96-L119

Would be better if the editor had a unified way to approach it, but maybe you could do something similar?

Acren avatar Aug 15 '24 12:08 Acren

Thanks, that was the kind of magic I was looking for; I'll have to take a closer look and replicate the logic, that wouldn't be the first time copy past is needed. In the longer run this should perhaps be added to the UI framework I believe.

SRombauts avatar Aug 15 '24 16:08 SRombauts

Just to follow up on this, I did send through the logs on a ticket, did you get those?

Acren avatar Sep 03 '24 05:09 Acren

Yes I received your logs through the support team, thanks! I couldn't dedicate time to investigate on this until now, sorry about that.

I'll see if I can provide a refactor of how the plugin handle Locks, and improve the context menu

Related: Fix lock cache excessive calls

SRombautsU avatar Oct 17 '24 08:10 SRombautsU

Much thanks for that, from some initial testing it helps dramatically!

Acren avatar Oct 19 '24 08:10 Acren

Thanks for letting me know! I really wish I could have taken the time earlier, but we were all focused on Unite and Unity 6 launch in the past months. Now we have Unreal 5.5 and Fab.com launch in front of us 😊

SRombauts avatar Oct 19 '24 08:10 SRombauts