liamg / darktile

:waning_crescent_moon: Darktile is a GPU rendered terminal emulator designed for tiling window managers.
MIT License
3.04k stars 112 forks source link

ctrl+C not working well with lots of output #155

Closed daviesalex closed 3 years ago

daviesalex commented 5 years ago

Describe the bug ctrl+C often does not interrupt a long running command that produces output

To Reproduce Steps to reproduce the behavior:

  1. SSH to a box. Boxes that are further away (e.g. across an Ocean, ~100ms) will make this more obvious
  2. Run a command that generates a lot of output (ls -l a directory with many files, cat a file, etc.)
  3. Type Ctrl+C
  4. Observe that it does not quickly stop printing output. (Compare with something like putty)

Expected behavior Pressing ctrl+C should interrupt the output

daviesalex commented 5 years ago

I actually think this is a generic ctrl+c issue. Run a long running command and compare with putty:

image

vs

image

maxhora commented 5 years ago
maxhora commented 5 years ago

It seems that reader.ReadRune() (func (terminal *Terminal) Read() error from terminal/terminal.go) on Windows is executed in 4-5 times slower than on Linux. Processing time was checked on connection to remote vm in central US and running of ls -l command inside directory with thousands of files. Windows was running on more powerful hardware(core i7) and showed:

2019/01/30 15:01:47 Processed 100000 runes in 151.5918ms
2019/01/30 15:01:48 Processed 100000 runes in 139.6263ms
2019/01/30 15:01:48 Processed 100000 runes in 140.6239ms
2019/01/30 15:01:48 Processed 100000 runes in 147.6045ms
2019/01/30 15:01:48 Processed 100000 runes in 145.6105ms

Linux was running on local virtual machine(core i5) and showed:

2019/01/30 15:03:39 Processed 100000 runes in 34.541407ms
2019/01/30 15:03:39 Processed 100000 runes in 42.083165ms
2019/01/30 15:03:39 Processed 100000 runes in 44.535962ms
2019/01/30 15:03:39 Processed 100000 runes in 43.267556ms
2019/01/30 15:03:39 Processed 100000 runes in 23.610861ms

Difference in processing time is correlated with visual delay timeout after pressing of ctrl+c till the output printing stops on Linux and Windows.

maxhora commented 5 years ago

Modified source code:

// Read needs to be run on a goroutine, as it continually reads output to set on the terminal
func (terminal *Terminal) Read() error {

    buffer := make(chan rune, 0xffff)

    reader := bufio.NewReader(terminal.pty)
    counter := int64(0)
    start := time.Now()

    go terminal.processInput(buffer)
    for {
        r, _, err := reader.ReadRune()
        counter+=1
        if counter == 100000 {
            log.Printf("Processed 100000 runes in %s", time.Since(start))
            counter = 0
            start = time.Now()
        }
        if err != nil {
            if err == io.EOF {
                break
            }
            return err
        }
        buffer <- r
    }

    //clean exit
    return nil
}
maxhora commented 5 years ago

Confirmed, ConPTY output on Windows 10 1903 build 18323.1000 works in 3 times faster than on 1809. The visual timeout after pressing "ctrl+c" till the interruption takes less than 2 seconds under "the heavy work-load":

2019/01/31 13:56:15 Processed 100000 runes in 49.8735ms
2019/01/31 13:56:15 Processed 100000 runes in 47.8646ms
2019/01/31 13:56:15 Processed 100000 runes in 49.8698ms
2019/01/31 13:56:15 Processed 100000 runes in 48.867ms
2019/01/31 13:56:15 Processed 100000 runes in 46.8743ms
2019/01/31 13:56:15 Processed 100000 runes in 46.8755ms
maxhora commented 5 years ago

Interesting that OSCTerminators: map[rune]struct{}{0x00: {}}, doesn't work on Windows 18323 anymore.

I've just tried with *nux terminators and that resolved all appeared issues. It looks like Windows Console team made some progress in compatibility as well.

maxhora commented 5 years ago

Moving the Issue to Backlog. Hopefully, it should be fully resolved after jump into Windows 10 1903

liamg commented 3 years ago

Closed due to complete rewrite as part of bringing the project back to life, please create a new issue if still relevant. Thank you!