Skip to content

Sync with pty process using XOFF and XON#447

Merged
Tyriar merged 11 commits intoxtermjs:masterfrom
Tyriar:425_xon_xoff_on_280
Jan 11, 2017
Merged

Sync with pty process using XOFF and XON#447
Tyriar merged 11 commits intoxtermjs:masterfrom
Tyriar:425_xon_xoff_on_280

Conversation

@Tyriar
Copy link
Copy Markdown
Member

@Tyriar Tyriar commented Jan 3, 2017

Fixes #425

This PR adds the following:

  • Implements XOFF (ctrl+S) and XON (ctrl+Q)
  • Separates writes from the processing of the writes by means of a write buffer (Terminal.writeBuffer) and setTimeout(...,0) to start the write async
  • XOFF is sent to the pty process when a threshold is reached so that xterm.js can catch up
  • Writes are batched in separate async calls in order to prevent the possibility of overloading the UI thread. Calling them all async is bad for performance as it means that the renderer would need to catch up before every write.
  • Frames are now skipped in order to save CPU time for the processing of data, not useless rendering of output that will be replaces in ~1000/60ms

Notes:

  • Since things are not synced with the backing process programs such as time are now accurate!
  • ^C is now responsive
  • There is a large diff due to an indentation change in write (now innerWrite), you only need to look at the beginning and end of the function though

@Tyriar Tyriar added this to the 2.3.0 milestone Jan 3, 2017
@Tyriar Tyriar self-assigned this Jan 3, 2017
@Tyriar Tyriar requested a review from parisk January 3, 2017 23:57
@Tyriar
Copy link
Copy Markdown
Member Author

Tyriar commented Jan 4, 2017

Here are results from the perf test in #422 (comment) to show how this affects perf:

Before perf work CircularList XOFF/XON gnome-terminal
Print 1..500000 55s 8s 12s 3s
ls -lR ~ (524582 files) 68s 29s 22s 7s

Keep in mind while there is a regression, the UI is now completely responsive (at least on my laptop with these config values) and you can SIGINT the process at any time.

@Tyriar Tyriar added the work-in-progress Do not merge label Jan 4, 2017
@Tyriar
Copy link
Copy Markdown
Member Author

Tyriar commented Jan 4, 2017

Also this is what an ls -lR /usr/lib timeline looks like which I've been using in other issues:

image

@Tyriar Tyriar removed the work-in-progress Do not merge label Jan 9, 2017
@Tyriar
Copy link
Copy Markdown
Member Author

Tyriar commented Jan 9, 2017

Ready for review

@Tyriar Tyriar merged commit fc528f6 into xtermjs:master Jan 11, 2017
@Tyriar Tyriar deleted the 425_xon_xoff_on_280 branch January 11, 2017 05:21
@Tyriar Tyriar modified the milestone: 2.3.0 Feb 2, 2017
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 this pull request may close these issues.

2 participants