mordant icon indicating copy to clipboard operation
mordant copied to clipboard

`TerminalInterfacePosix.readRawByte` uses exceptions for timeout

Open JakeWharton opened this issue 1 year ago • 2 comments

I think the restructuring of all these types happened concurrently with #206 because I don't remember seeing this behavior before (although I may have simply missed it).

I'm trying to read key events in a way that can also safely be exited. To do so I issue reads with tens-of-millisecond timeouts so that the coroutine can wake up to check whether it needs to stop. On the JVM I could use thread interrupts, but that doesn't help on native or the forthcoming NodeJS support.

As a result of the very short timeouts, exceptions for control flow absolutely destroy performance. Since this is reading a byte, maybe we can widen the return type to an Int (it's probably at least 32 bits in a register anyway) and use a sentinel value outside the 8-bit range to indicate timeout.

Here's what I'm seeing on .131-SNAPSHOT:

Exception in thread "main" java.lang.RuntimeException: Timeout reading from stdin (timeout=9.882666ms)
        at com.github.ajalt.mordant.terminal.terminalinterface.TerminalInterfaceJvmPosix.readRawByte-HG0u8IE(TerminalInterface.jvm.posix.kt:15)
        at com.github.ajalt.mordant.terminal.terminalinterface.TerminalInterfacePosix.readInputEvent_VtjQ1oo$read(TerminalInterface.posix.kt:147)
        at com.github.ajalt.mordant.terminal.terminalinterface.TerminalInterfacePosix.readInputEvent-VtjQ1oo(TerminalInterface.posix.kt:151)
        at com.github.ajalt.mordant.input.RawModeScope.readKeyOrNull-LRDsOJo(RawMode.kt:121)

JakeWharton avatar Aug 21 '24 02:08 JakeWharton

I switched to exceptions to make code simpler since TerminalInterface has to be public now. But if it's causing issues, I'll come up with something else.

ajalt avatar Aug 21 '24 15:08 ajalt

I hesitate to call it an issue, but I don't know any other way to cleanly be reading key events while also supporting cancelation except to use a low timeout and a fairly hot loop. I guess I could try to close the file descriptor and use an infinite timeout.

JakeWharton avatar Aug 21 '24 16:08 JakeWharton

I stopped using exceptions for the common case in #221. I'm boxing the byte values to simplify code, but if that's still too slow let me know and I can switch to a sentinel.

Something to keep in mind is that when setting up raw mode on POSIX, VTIME (the max wait time) is measured in 10ths of a second. I use the minimum VTIME=1 so the terminal is probably going to block for up to 100ms on each read call. I'm not sure polling faster than that is going to improve responsiveness.

If you aren't happy with polling, another solution you could consider would be to do infinite timeout reads on a background task into a buffer/channel. Then your main loop could do a cancellable suspending read on the buffer without polling. That seems a lot more complicated though, so it might not be worth it.

ajalt avatar Sep 01 '24 19:09 ajalt