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

Backport BetterScreen into pyte #47

Open
superbobry opened this issue Jun 15, 2016 · 8 comments
Open

Backport BetterScreen into pyte #47

superbobry opened this issue Jun 15, 2016 · 8 comments

Comments

@superbobry
Copy link

Currently BetterScreen duplicates a lot of pyte.Screen functionality. Clearly duplication is no good when it comes to software, as the bugs need to be fixed independently in multiple copies of the code. The same goes for other maintenance changes like performance optimizations.

I would very much like to make Screen extensible so there is no need for duplication. Would you be interested in such a PR?

@jonathanslenders
Copy link
Member

Yes, I am interested. Personally, I don't have the time to do a lot of work on this right now, but all help is appreciated.

However, it's not necesarily easy. For a part, the duplication is because BetterScreen uses a different data structure. One that is very close to what prompt-toolkit internally uses. This makes the rendering very efficient. Many other changes are made in order to be as efficient as possible.
Some other changes:

  • full color support
  • CPR support (cursor position response)
  • in BetterStream are some changes to make the parsing even more efficient.

So, if we can remove some duplication, I'd very much appricate that. However, if for instance it involves adding function calls in expensive code, I'd rather not do that.
I also really don't mind any backwards-incompatible changes, if we have to.

Jonathan

@superbobry
Copy link
Author

superbobry commented Jun 23, 2016

Faster parsing and CPR were backported and released as 0.5.2, so Screen improvements are the next logical step.

Do you have a benchmark suite I can use to ensure there're no regressions?

@jonathanslenders
Copy link
Member

Hi @superbobry, there's a tool called vttest which tests xterm capabilities. It's mostly what I use, but not all of xterm is implemented.

@superbobry
Copy link
Author

Thanks for the pointer. I remember using vttest for pyte as well at some point, but IIRC it doesn't do performance testing, right?

@jonathanslenders
Copy link
Member

No, it doesn't. Sometimes I measure the time it takes to execute for instance time find /var. Or if you want something with escape sequences. Just log the input of any interaction to a file and replay that.

@superbobry
Copy link
Author

Progress update: I've backported more Stream speedups. With the latest master pyte.Stream is on par with BetterStream.

Benchmark pyte.Stream pymμx.stream.BetterStream
cat GPL-3 41.8ms ± 0.6ms 41.7ms ± 0.5ms
find /etc 92.7ms ± 2.3ms 92.0ms ± 3.7ms
10s trace of htop 106μs ± 2μs 109μs ± 6μs
man man 1.09μs ± 0.03μs 848ns ± 17ns
mc 1.09μs ± 0.03μs 859ns ± 77ns

You've done a very impressive job speeding Stream up!

@superbobry
Copy link
Author

Quick question before I proceed to BetterScreen. There seem to be quite a few changes compared to the original code. Do you happen to have any tests for them?

@jonathanslenders
Copy link
Member

Hi @superbobry,

Sorry for the late reply. (I've been busy, lately.) For BetterScreen, I don't have any tests. Almost everything that I tested was manually because of the visual feedback. I think it's not that easy to write good unit tests for this.

For the last few months, I've been using pymux fulltime for all my work. Actually, it never seems to crash, and all terminal applications that I use, work fluent.

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

No branches or pull requests

2 participants