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

Oni2 struggles with a simple stress test #296

Open
Apostolique opened this issue May 2, 2019 · 11 comments
Open

Oni2 struggles with a simple stress test #296

Apostolique opened this issue May 2, 2019 · 11 comments
Labels
A-ux Area: Overall user experience and aesthetics bug Something isn't working I-crash It crashes. Is very bad! I-slow User-facing performance issue

Comments

@Apostolique
Copy link

I did a simple stress test in Oni2. I start with a blank buffer. I write Hello on the first line. Then I yank the line and paste it 3 million times using 3000000p. This crashes Oni2.

I can do the same with neovim with no issues. The lines are pasted instantly. In fact, I can paste an incredible amount of lines. Neovim will accept 20 million lines without any issues.

@CrossR CrossR added bug Something isn't working I-slow User-facing performance issue labels May 2, 2019
@bryphe
Copy link
Member

bryphe commented May 2, 2019

Thanks @Apostolique ! This will be a good test case for us.

For this particular issue - we'll need to track down if we're actually getting the buffer lines via the buffer update, and if so, if we're choking on that update (and at what point).

Planning on doing some stability / perf work in June prior to dropping releases. We need to build up an infra to run these sorts of tests.

@bryphe
Copy link
Member

bryphe commented May 2, 2019

A couple other stress test cases I'm interested in:

  • Open buffer, repeatedly (for small, medium, large files)
  • Switch between buffers, repeatedly
  • Repetitive text edits (the case you called out is a good start)

@tcoopman
Copy link
Contributor

tcoopman commented May 2, 2019

A good test might also be to define a macro and execute it a lot of times. I've noticed that in some editors emulating vim that macro's can be excruciating slow.
Of course this is basically the same as repetitive text edits..

@somebody1234
Copy link

somebody1234 commented May 3, 2019

Huh, Fatal error: exception Stack overflow happened to me the first time for some reason
1m lines works okay sometimes, might need more testing

@bryphe
Copy link
Member

bryphe commented May 4, 2019

Thanks for trying it out @somebody1234 ! If you run with the ONI2_DEBUG environment variable set to true - we should get a stack trace as well (which might give some clues on where in the pipeline things are failing).

@CrossR
Copy link
Member

CrossR commented May 5, 2019

The error you get on pasting is as follows:

Fatal error: exception Stack overflow
Raised by primitive operation at file "src/editor/Neovim/NeovimApi.re", line 55, characters 2-17
Called from file "list.ml", line 106, characters 12-15
Called from file "list.ml", line 106, characters 12-15
Called from file "list.ml", line 106, characters 12-15
Called from file "src/Core/Tick.re", line 33, characters 8-25
Called from file "list.ml", line 88, characters 20-23
Called from file "src/Core/Tick.re", line 39, characters 22-50
Called from file "src/Core/App.re", line 62, characters 4-23
Called from file "src/glfw.re", line 192, characters 19-31
Called from file "src/editor/bin/Oni2.re", line 166, characters 0-1

The first line is unlocking a mutex, then most of it goes into revery code by the looks of it.

@bryphe
Copy link
Member

bryphe commented May 7, 2019

Thanks @CrossR ! Looks like there is a bug somewhere in one of our handlers for messages in NeovimApi: https://github.com/onivim/oni2/blob/master/src/editor/Neovim/NeovimApi.re

(At least, one of the places where we are calling withMutex 🤔 - likely due to a large volume of notifications).

@bryphe
Copy link
Member

bryphe commented May 7, 2019

We must be using a List operation somewhere in processing these notifications that isn't tail recursive (using constant stack space).

@bryphe
Copy link
Member

bryphe commented May 7, 2019

Stack trace is pointing to a use of List.iter: https://github.com/ocaml/ocaml/blob/20d2b67a39750ef334dad3aab13766646c523569/stdlib/list.ml#L106. But it's interesting - thought that should be tail recursive.

@CrossR
Copy link
Member

CrossR commented Jul 8, 2019

I've given this a try out with the latest master, and can confirm that with a libvim build I was able to past 3 + 6 million lines with no trouble. Undoing that took a moment and hung the editor until the undo completed, but I think that the terminal versions also have a hiccup on an undo that big. Scrolling after this was slower, but not very slow.

Pasting 20 million also worked, with a slight delay on paste, and very slow scrolling after.
Undoing that managed to cause the editor to hang and not respond, but I also don't know how realistic a 20 million line edit is to test with.

@bryphe
Copy link
Member

bryphe commented Jul 9, 2019

Nice, thanks for testing this out @CrossR ! Seems like we're improved from where we were.

It'd be nice to get the undo case working (I suspect that either libvim is sending a lot of events in that case, or we're doing something in response to a large buffer update that is problematic).

I'd also like to add some benchmark / stress tests to reason-libvim - just having some of those tests in place at that layer can help us isolate where we need to focus on performance. For example - if reason-libvim on its own, without UI, undos 20mil lines without a hiccup, we know the problem is like in the Onivim 2 layer.

@glennsl glennsl added I-crash It crashes. Is very bad! A-ux Area: Overall user experience and aesthetics labels Nov 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ux Area: Overall user experience and aesthetics bug Something isn't working I-crash It crashes. Is very bad! I-slow User-facing performance issue
Projects
None yet
Development

No branches or pull requests

6 participants