SRombauts / UEGitPlugin

Unreal Engine 5 Git LFS 2 Source Control Plugin (beta)
http://srombauts.github.io/UEGitPlugin
MIT License
825 stars 164 forks source link

Actually sleep a little bit so as to not tick too often #142

Closed crevetor closed 4 years ago

crevetor commented 4 years ago

This fixes #139

Progress.Tick() is called too often otherwise and it contains :

// Sync the game thread and the render thread
FSlateApplication::Get().GetRenderer()->Sync();

Which seems like a bad thing to call too often.

I think the correct way to do this would be to make this code asynchronous and use the module's tick to tick the other components.

SRombauts commented 4 years ago

Thanks a lot, this was a very recent change that I got from a fork, made to improve performances.

Have you tested with a smaller value like 0.001f (1ms)?

crevetor commented 4 years ago

I haven't tested a lower value.

Carbon-NateBowman commented 4 years ago

First, the binaries were not included in the patch.

I Have now tested this. Test cases: A: move a folder containing 1 mesh. B: move a folder containing 45 meshes.

With the value of 0 its a quick crash on both. 0.001f A passes, B fails very quickly crashing as before. 0.01f, A passes, B fails eventually and crashes as before. 0.1f, A passes, it takes an eternity but will B ultimately crashes in the same way.

Even ticking the progress at a reduced relative rate eventually produced a crash.

        int counter = 0; 

        // ... then wait for its completion (thus making it synchronous)
        while(!InCommand.bExecuteProcessed)
        {
            // Tick the command queue and update progress.
            Tick();

            if(counter % 1000 == 0)
            {
                Progress.Tick();
            }

            counter++;

            // Sleep so we don't busy-wait so much.
            FPlatformProcess::Sleep(0.001f);
        }

however, setting the sleep time to 0f and just commenting out Progress.Tick(); solved the issue altogether, although the ui progress updates are understandably a bit broken.

Obviously this is needs more than just a quick bit of duct-tape.