goblinfactory / konsole

Home of the simple console library consisting of ProgressBar, Window, Form, Draw & MockConsole (C# console progress bar with support for single or multithreaded progress updates) Window is a 100%-ish console compatible window, supporting all normal console writing to a windowed section of the screen, supporting scrolling and clipping of console output.
719 stars 62 forks source link

Scrollbar Position error when bottom of console window is reached. #94

Closed fritzjoostenz closed 4 months ago

fritzjoostenz commented 4 months ago

Hi Alan. I am working on Windows and finding if I run several batch jobs, each with its own set of parallel tasks with konsole.progressbars, then the progress bars do not position correctly when the bottom of the console window is reached. I made a video to show what I am experiencing:

Here are my machine and windows specs:

Processor 13th Gen Intel(R) Core(TM) i7-13700K 3.40 GHz Installed RAM 32.0 GB (31.8 GB usable) Device ID E83C7EF6-6BB4-483B-8D71-B72F0BCC489B Product ID 00325-96463-46633-AAOEM System type 64-bit operating system, x64-based processor Pen and touch No pen or touch input is available for this display

Edition Windows 11 Home Version 23H2 Installed on ‎17/‎02/‎2023 OS build 22631.3737 Experience Windows Feature Experience Pack 1000.22700.1009.0

Any work-around for this? My apologies if this is just because I am new to your framework. Many thanks!

goblinfactory commented 4 months ago

Morning Fritz

(though for you I think it's already afternoon) I'd like to allocate some time to look at writing a patch for the bug you raised, but in order to do that, can you please write the least bit of code you can think of that reproduces the problem? (drop the code here in this discussion) If you do that then i can immediately look at writing a patch using your sample as a unit test that passes when your sample renders as expected, without getting stuck at the bottom of the window. txs, Alan

fritzjoostenz commented 4 months ago

@goblinfactory - here is a class that illustrates the problem. My apologies in advance if this is only because of my improper use of the Parallel loop with a progress bar. If you call the public method from a console app, you should see the behaviour I illustrated in my video:

` public class Tester {

public void RunTasks()
{

    Console.WriteLine($"Starting all tasks");        
    int nTasks = 5;
    for (int i = 0; i < nTasks; i++)
    {            
     this.RunProcessesForTask(i);
    }
}

private void RunProcessesForTask(int iTask)
{

    Console.WriteLine($"Working on Task {iTask}");
    Random random = new Random();
    int nPRocesses = 20;

    //This works fine:
    //for (int i = 0; i < nPRocesses; i++)
    //{
    //    int n = random.Next(0, 500);
    //    ProgressBar bar = new ProgressBar(n);
    //    bar.Refresh(0, $"Executing Process {i}");
    //    this.RunProcess(bar, n, i);
    //}

    // This causes some bars to get "stuck" at bottom when scrolling
    Parallel.For(0, nPRocesses, i =>
    {
        int n = random.Next(0, 500);
        ProgressBar bar = new ProgressBar(n);
        bar.Refresh(0, $"Executing Process {i}");
        this.RunProcess(bar, n, i);
    });

}

private void RunProcess(ProgressBar bar, int n, int iProcess)
{        
    for (int i = 0; i < n; i++)
    {
        bar.Refresh(i, $"Executing Process {iProcess}");
    }

}

}

`

goblinfactory commented 4 months ago

Hi @fritzjoostenz I think I understand what is happening. I didn't realise you were using Parallel.For until you posted your code example. Parallel.For should not be used if you are going to be making any IO bound calls in the code. Parallel.For should only be used for highly CPU bound tasks, and when each task has completed, you can safely make IO calls, like UI updates to progress bars, but not in between. The way that Konsole ensures that different threads do not write to the same area of the screen or change colours or cursor position, is to provide a single interface and Goblinfactory.Konsole handles all the synchronisation. These locks will block a thread, (stop it executing) meaning that you will lose the small increases in optimisations that Parallel.For gives you.

I already have integration tests to test creating multiple progress bars on different threads using Task.Run, and creating enough ProgressBar 's to cause the screen to scroll.

You can see the test here https://github.com/goblinfactory/konsole/blob/master/src/Konsole.Tests.Slow/ProgressBarConcurrencyTests.cs

So to summarise, I'm not going to make any changes to Goblinfactory.Konsole to support Parallel.For.

If you still find that you are experiencing similar problems at the bottom of console window, then please send me a small bit of code that shows the problem and I will re-open this issue.

I'll leave the issue open for a few days so that you can investigate further, and if I dont hear back from you, I will close this issue and mark as "not a bug".

best regards,

Alan

fritzjoostenz commented 4 months ago

Thanks for looking into this @goblinfactory - Understood. I suspected the issue had something to do with my parallel for loop. Thanks again.

goblinfactory commented 4 months ago

Only a pleasure!

@fritzjoostenz It's easy to fall into the trap of super optimising code for performance, and accidentally write code that performs quite poorly. Writing highly performant (parallel and-or multitasking) code is very hard. There are some great libraries that can remove some of the complexities.

If you like this topic, and want to learn more I can recommend taking a look at TPL Dataflow, (find books or blog posts by Steven Taub) https://devblogs.microsoft.com/pfxteam/tag/dataflow/

if you're going to be providing a lot of user feedback, via progress bars etc, then the Humble Task.Run is often good enough, and the .net runtime scheduler will ensure that it makes best of use available cores. Goblinfactory.Konsole works well, has excellent test coverage and and is battle tested to be safely used with Task.Run.

best of luck! regards, Alan

fritzjoostenz commented 4 months ago

@goblinfactory - thanks Alan, you are a star! Agreed about the difficulties of using parallel processing. I suspect is it sub-optimal and will do some benchmarks and refactor.