Skip to content
This repository has been archived by the owner on Jan 2, 2021. It is now read-only.

Benchmark suite #590

Merged
merged 12 commits into from
Jun 3, 2020
Merged

Benchmark suite #590

merged 12 commits into from
Jun 3, 2020

Conversation

pepeiborra
Copy link
Collaborator

The benchmark suite is designed to be run with cabal bench. It does not attempt to compare with past performance, I think that's better tackled as a separate task.

The module comment in the benchmark module contains some details on the benchmark methodology and experiments, which currently cover the basics:

  • hover
  • go to definition
  • completions
  • code actions
  • document symbols

I have not made any attempts to integrate with CI since I don't know how consistent the timings would be across runs.

Goes some way towards closing #242

@pepeiborra
Copy link
Collaborator Author

pepeiborra commented May 30, 2020

Example output:

$ cabal bench
Running 1 benchmarks...
Benchmark ghcide-bench: RUNNING...
starting test
Unpacking to bench/example/Cabal-3.2.0.0/
/Users/pepe/Dropbox/code/ghcide/dist-newstyle/build/x86_64-osx/ghc-8.10.1/ghcide-0.1.0/x/ghcide/build/ghcide/ghcide
    Alloc    Copied     Live     GC     GC      TOT      TOT  Page Flts
    bytes     bytes     bytes   user   elap     user     elap
ghcide version: 0.1.0 (GHC: 8.10.1) (PATH: /Users/pepe/Dropbox/code/ghcide/dist-newstyle/build/x86_64-osx/ghc-8.10.1/ghcide-0.1.0/x/ghcide/build/ghcide/ghcide) (GIT hash: b759359f457da747f5c4e8659cc2ce08c8441fcd)
Starting LSP server...
If you are seeing this in a terminal, you probably should have run ghcide WITHOUT the --lsp option!
...
...
TOTAL hover = 2.56s (10 repetitions)
TOTAL getDefinition = 2.67s (10 repetitions)
TOTAL documentSymbols = 1.43s (100 repetitions)
TOTAL documentSymbols after edit = 2.90s (100 repetitions)
TOTAL completions after edit = 4.53s (10 repetitions)
TOTAL code actions = 0.36s (10 repetitions)
TOTAL code actions after edit = 3.60s (10 repetitions)
Benchmark ghcide-bench: FINISH

@mpickering
Copy link
Contributor

Looks useful @pepeiborra

Once my branch starts getting merged in then some of these tests will become trivial lookups in a HashMap so will probably have to be modified to show anything useful.

@pepeiborra pepeiborra mentioned this pull request May 30, 2020
Copy link
Collaborator

@ndmitchell ndmitchell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is awesome!

The one feature I'd have liked was the ability to do profiling. That requires installing ghcide with profiling (a user step), passing +RTS -p when starting ghcide (easy to change below) and then arranging for ghcide to kill itself rather than be forcibly terminated by lsp-test, which I didn't have a good sense on how to do. Of course, this feature could definitely come later.

bench/Main.hs Outdated Show resolved Hide resolved
bench/Main.hs Outdated Show resolved Hide resolved
bench/Main.hs Outdated Show resolved Hide resolved
bench/Main.hs Show resolved Hide resolved
bench/Main.hs Show resolved Hide resolved
bench/Main.hs Outdated Show resolved Hide resolved
@pepeiborra
Copy link
Collaborator Author

This is awesome!

The one feature I'd have liked was the ability to do profiling. That requires installing ghcide with profiling (a user step), passing +RTS -p when starting ghcide (easy to change below) and then arranging for ghcide to kill itself rather than be forcibly terminated by lsp-test, which I didn't have a good sense on how to do. Of course, this feature could definitely come later.

I implemented a nice way of allowing ghcide to exit cleanly which I think should work fine with profiling, although I have not checked that.

Another issue is that the benchmark invokes ghcide once per experiment, and afaik there is no way to redirect the profiling artifacts to a specific path, so the user would need to select only one experiment (or we change the test suite to run every experiment in a different folder).

@ndmitchell
Copy link
Collaborator

When I tried sending a close to ghcide it didn't exit, merely stopped responding to messages, so worth validating (I could well have screwed it up). Running ghcide on a single benchmark at a time seems what people would want to do, so no problem with that.

Note that you might need to roll the stack.yaml files or explicitly override to get the latest version of extra included.

@pepeiborra
Copy link
Collaborator Author

pepeiborra commented May 31, 2020

When I tried sending a close to ghcide it didn't exit, merely stopped responding to messages, so worth validating (I could well have screwed it up).

Maybe you didn't send a shutdown message followed by an exit message? That seems to work for me, at least the +RTS -s stats are being printed on exit. The only problem I found is that lsp-test kills the server before it gets a chance to actually exit, so I send the exit messages explicitly and then sleep for a bit.

Running ghcide on a single benchmark at a time seems what people would want to do, so no problem with that.

That's right, but then more user steps are needed, because the benchmark suite does not support selecting experiments to run.

@ndmitchell
Copy link
Collaborator

Ah, that would be it - I think I only sent a shutdown. Good to know, and even better that with this benchmark I don't need to know!

bench/Main.hs Outdated Show resolved Hide resolved
bench/Main.hs Outdated Show resolved Hide resolved
@mpickering
Copy link
Contributor

Thanks Pepe. I left a few comments but nothing critical to stop the merge imo.

Copy link
Collaborator

@cocreature cocreature left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, thanks!

@cocreature cocreature merged commit 5a754e1 into haskell:master Jun 3, 2020
pepeiborra added a commit to pepeiborra/ide that referenced this pull request Dec 29, 2020
* Initial benchmark suite, reusing ideas from Neil's post

https://neilmitchell.blogspot.com/2020/05/fixing-space-leaks-in-ghcide.html

* Add an experiment for code actions without edit

* formatting

* fix code actions bench script

* error handling + options + how to run

* extract Positions and clean up imports

(Neil's review feedback)

* replace with Extra.duration

* allow ImplicitParams

* add bench to the cradle

* applied @mpickering review feedback

* clean up after benchmark

* remove TODO
pepeiborra added a commit to pepeiborra/ide that referenced this pull request Dec 29, 2020
* Initial benchmark suite, reusing ideas from Neil's post

https://neilmitchell.blogspot.com/2020/05/fixing-space-leaks-in-ghcide.html

* Add an experiment for code actions without edit

* formatting

* fix code actions bench script

* error handling + options + how to run

* extract Positions and clean up imports

(Neil's review feedback)

* replace with Extra.duration

* allow ImplicitParams

* add bench to the cradle

* applied @mpickering review feedback

* clean up after benchmark

* remove TODO
pepeiborra added a commit to pepeiborra/ide that referenced this pull request Dec 29, 2020
* Initial benchmark suite, reusing ideas from Neil's post

https://neilmitchell.blogspot.com/2020/05/fixing-space-leaks-in-ghcide.html

* Add an experiment for code actions without edit

* formatting

* fix code actions bench script

* error handling + options + how to run

* extract Positions and clean up imports

(Neil's review feedback)

* replace with Extra.duration

* allow ImplicitParams

* add bench to the cradle

* applied @mpickering review feedback

* clean up after benchmark

* remove TODO
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants