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

Sync with pty process using XOFF and XON #447

Merged
merged 11 commits into from
Jan 11, 2017

Conversation

Tyriar
Copy link
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
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
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
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