Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

stdout.write performance improvements #617

Open
ahaoboy opened this issue Jul 17, 2024 · 15 comments · Fixed by #618
Open

stdout.write performance improvements #617

ahaoboy opened this issue Jul 17, 2024 · 15 comments · Fixed by #618

Comments

@ahaoboy
Copy link
Contributor

ahaoboy commented Jul 17, 2024

Here is a simple example to fill the terminal and move a space:

When running with tjs, the terminal will flicker, but nodejs will run smoothly. It is not clear whether it is a platform problem of Windows, or the implementation of console.clear is different from deno/node.

let width = 0
let height = 0
if (typeof tjs !== 'undefined') {
  width = tjs.stdout.width
  height = tjs.stdout.height
} else {
  width = process.stdout.columns
  height = process.stdout.rows
}
let log;
if (typeof tjs !== 'undefined') {
  const encoder = new TextEncoder()
  log = s => tjs.stdout.write(encoder.encode(s))
} else {
  log = s => process.stdout.write(s)
}
let c = 0
const size = width * height

const sleep = n => new Promise(r => setTimeout(r, n));

(async function () {
  while (c < size) {
    console.clear()
    const s = new Array(size).fill('x')
    s[c] = ' '
    log(s.join(''))
    c++
    await sleep(16)
  }
})()



deno/console/clear

class CSI {
  static kClear = "\x1b[1;1H";
  static kClearScreenDown = "\x1b[0J";
}

clear = () => {
  this.indentLevel = 0;
  this.#printFunc(CSI.kClear, 1);
  this.#printFunc(CSI.kClearScreenDown, 1);
};

I tried running snake game with txiki, and it had no functional issues except for the terminal flickering.
https://github.com/ahaoboy/r-tui-txiki/blob/main/src/snake.tsx

@saghul
Copy link
Owner

saghul commented Jul 17, 2024

On txiki you want to await the write call, can you try that?

@ahaoboy
Copy link
Contributor Author

ahaoboy commented Jul 17, 2024

On txiki you want to await the write call, can you try that?

There is a 16ms interval between each output, perhaps due to the terminal support issue? So adding await does not seem to improve it

@saghul
Copy link
Owner

saghul commented Jul 17, 2024

Interesting. I'll take a look!

@ahaoboy
Copy link
Contributor Author

ahaoboy commented Jul 17, 2024

The problem can be improved by reducing the terminal size to some extent, I encountered a similar problem in rqjs, and realized it by refreshing the terminal every time, because it displayed a very large string without line breaks, and I'm not sure about txiki's logic yet.

pub fn print(s: String) {
print!("{s}");
let mut stdout = std::io::stdout();
stdout.flush().unwrap();
}

snake

Interesting. I'll take a look!

This issue should not be a high priority, different platforms and terminal tools may also have an impact

@saghul
Copy link
Owner

saghul commented Jul 18, 2024

Can you give #618 a try? It's not on par with Node, but I believe it has improved things quite a bit! The use of a different way to clear the screen was spot on!

@ahaoboy
Copy link
Contributor Author

ahaoboy commented Jul 18, 2024

Can you give #618 a try? It's not on par with Node, but I believe it has improved things quite a bit! The use of a different way to clear the screen was spot on!

D:/a/txiki.js/txiki.js/src/vm.c:222:tjs__promise_rejection_tracker: Assertion `(JS_IsException(event)) == (0)' failed.

@saghul
Copy link
Owner

saghul commented Jul 18, 2024

Oh so weird! I am not getting that on macOS! Looks like an unhandled rejection happened and creating the internal PromiseRejectionEvent failed, weird!

Do you have a full tracback by any chance?

@ahaoboy
Copy link
Contributor Author

ahaoboy commented Jul 18, 2024

Do you have a full tracback by any chance?

Does tjs have an option to generate tracelog? There is no such problem on Ubuntu, and the tjs.exe also does not work on Windows.

@ahaoboy
Copy link
Contributor Author

ahaoboy commented Jul 18, 2024

@saghul

This code throw Error on windows

      // Do blocking writes for TTYs:
      // https://github.com/nodejs/node/blob/014dad5953a632f44e668f9527f546c6e1bb8b86/lib/tty.js#L112
      if (!isStdin) {
        handle.setBlocking(true);
      }

@saghul
Copy link
Owner

saghul commented Jul 18, 2024

Thanks, that helps! I force-pushed with a fix. Blocking writes are only a thing on Windows when using pipes, so added an extra check.

Can you test again please?

@ahaoboy
Copy link
Contributor Author

ahaoboy commented Jul 18, 2024

Can you test again please?

Now it works!

@saghul
Copy link
Owner

saghul commented Jul 18, 2024

Does it perform better?

@ahaoboy
Copy link
Contributor Author

ahaoboy commented Jul 18, 2024

Does it perform better?

Unfortunately there doesn't seem to be

@saghul
Copy link
Owner

saghul commented Jul 18, 2024

On macOS I can tell the difference, since resetting the terminal is more expensive than clearing it, so still worth doing.

I'll keep this open and continue digging as time allows!

@saghul saghul reopened this Jul 18, 2024
@guest271314
Copy link
Contributor

Depending on what code is use tjs can be faster or slower than node.

My tests don't involve a TTY.

Here's my results re "performance" of tjs, qjs, node, deno, bun, SpiderMonkey's js, and V8's d8 (using GNU Coreutils' head to read standard input to d8), fastest to slowest, reading and writing 1 MB (1048576) bytes of JSON

0 'nm_qjs' 0.10419999998807906
1 'nm_bun' 0.12319999998807907
2 'nm_deno' 0.156
3 'nm_tjs' 0.1829000000357628
4 'nm_nodejs' 0.2165
5 'nm_spidermonkey' 0.3570999999642372
6 'nm_d8' 0.4243000000119209

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants