Skip to content

HLS benchmarks#3117

Merged
pepeiborra merged 15 commits into
masterfrom
hls-benchmarks
Aug 25, 2022
Merged

HLS benchmarks#3117
pepeiborra merged 15 commits into
masterfrom
hls-benchmarks

Conversation

@pepeiborra

@pepeiborra pepeiborra commented Aug 20, 2022

Copy link
Copy Markdown
Collaborator

Port the ghcide benchmark suite to HLS, adding support to benchmark plugin "configurations" independently.

This includes the following changes to the ghcide benchmark suite:

  • Support for "configurations" which are defined as sets of plugin ids.
    The benchmark will be run with only these plugins enabled and all others disabled
  • Support for configurable concurrency.

cpp-options: -Dhls_brittany

executable haskell-language-server
library plugins

@fendor fendor Aug 20, 2022

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

this might make it impossible to use stack for loading HLS into HLS. I.e. HLS needs to be configured to use cabal for development with this change due to commercialhaskell/stack#4564

Not a biggie, just fyi

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Finally time to drop the stack descriptors and the Circle CI

@pepeiborra
pepeiborra force-pushed the hls-benchmarks branch 4 times, most recently from 3ca8f60 to bc907e9 Compare August 21, 2022 07:08
@pepeiborra
pepeiborra marked this pull request as ready for review August 21, 2022 08:08
@michaelpj

Copy link
Copy Markdown
Collaborator

Great!

Is there a reason not to delete the ghcide benchmarking code? Would make the diff simpler too, since I assume the benchmark runner fails are copies of the existing ghcide ones, and then it would just be a move.

@pepeiborra

Copy link
Copy Markdown
Collaborator Author

Great!

Is there a reason not to delete the ghcide benchmarking code? Would make the diff simpler too, since I assume the benchmark runner fails are copies of the existing ghcide ones, and then it would just be a move.

I am actively deleting it. But maybe I missed some code.

@pepeiborra

Copy link
Copy Markdown
Collaborator Author

This is ready for review. CI is setup to run benchmarks for the plugin configurations defined in bench/config.yaml:

  • None: no plugins
  • All
  • Ghcide: just the ghcide plugins
  • Core: Ghcide plugins + call hierarchy + code range + eval + pragmas. This is an arbitrary selection
  • : configurations for each individual plugin, excluding formatting plugins since none of the benchmark experiments exercise STextDocumentFormatting.

configurations:
- None: []
- Core:
- callHierarchy
- codeRange
- eval
- ghcide-code-actions-bindings
- ghcide-code-actions-fill-holes
- ghcide-code-actions-imports-exports
- ghcide-code-actions-type-signatures
- ghcide-completions
- ghcide-type-lenses
- pragmas
- Ghcide:
- ghcide-code-actions-bindings
- ghcide-code-actions-fill-holes
- ghcide-code-actions-imports-exports
- ghcide-code-actions-type-signatures
- ghcide-completions
- ghcide-type-lenses
- All:
- alternateNumberFormat
- callHierarchy
- changeTypeSignature
- class
- codeRange
- eval
- explicitFixity
- floskell
- fourmolu
- gadt
- ghcide-code-actions-bindings
- ghcide-code-actions-fill-holes
- ghcide-code-actions-imports-exports
- ghcide-code-actions-type-signatures
- ghcide-completions
- ghcide-type-lenses
- hlint
- importLens
- moduleName
- ormolu
- pragmas
- qualifyImportedNames
- refineImports
- rename
- stylish-haskell
- alternateNumberFormat
- brittany
- callHierarchy
- changeTypeSignature
- class
- codeRange
- eval
- explicitFixity
- floskell
- fourmolu
- gadt
- ghcide-code-actions-bindings
- ghcide-code-actions-fill-holes
- ghcide-code-actions-imports-exports
- ghcide-code-actions-type-signatures
- ghcide-completions
# - ghcide-core # implicitly included in all configurations
# - ghcide-hover-and-symbols # implicitly included in all configurations
- ghcide-type-lenses
- haddockComments
- hlint
- importLens
- moduleName
- ormolu
- pragmas
- qualifyImportedNames
- refineImports
- rename
- retrie
- splice
- stan
- stylish-haskell
- tactics

The results show that hls-hlint-plugin is, unsurprisingly, the most expensive plugin, followed closely by Tactics.

There is certainly margin for improvement:

  1. Generated configurations: currently the list of configurations needs to be manually updated when new plugins are added (or removed).
  2. Conditional configurations: the 9.2 run is benchmarking a lot of plugins that are not included in the 9.2 build of HLS. These configurations are equivalent to the None configuration.
  3. Sharding: currently it takes CI ~4h to run all the configurations for the Cabal example, ideally we would split the work across multiple machines.

Comment thread bench/Main.hs
@michaelpj

Copy link
Copy Markdown
Collaborator

I am actively deleting it. But maybe I missed some code.

Excellent. There are still some files in ghcide/bench, though?

@pepeiborra

pepeiborra commented Aug 21, 2022

Copy link
Copy Markdown
Collaborator Author

I am actively deleting it. But maybe I missed some code.

Excellent. There are still some files in ghcide/bench, though?

The ghcide test suite references them, so a bit more work is required to move them.

To be more precise, we have:

  1. The benchmark experiment definitions, which are in a library for reuse in the ghcide test suite.
  2. The ghcide-bench executable, which could be renamed to lsp-bench since it's not that ghcide specific anymore.

Both things could be moved to a standalone package lsp-bench with executable and test suite stanzas the haskell-language-server package for clarity.

@pepeiborra

Copy link
Copy Markdown
Collaborator Author

I took a look and ghcide-bench uses code from the ghcide test suite (D.I.Test.Diagnostic) that is quite tricky to extract. The path would be:

  1. move all the ghcide support test code to hls-test-utils,
  2. move ghcide-bench to its own package.

Part 1 is a bit scary, I don't know how much work it will be.

@michaelpj

Copy link
Copy Markdown
Collaborator

The path would be:

Yeah, step 1 looks scary, especially because then the ghcide tests would depend on hls-test-utils which depends on ghcide? Not sure if that would work.

Would we need to do step 2? Wouldn't we just fold it into the HLS benchmark suite?

@michaelpj michaelpj left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Haven't looked at the actual benchmarking code, I trust you on that, just superficial comments.

Comment thread .gitignore
Comment thread bench/Main.hs
Comment thread bench/README.md
Comment thread bench/README.md Outdated
Comment thread bench/config.yaml
Comment thread exe/Main.hs Outdated
Comment thread haskell-language-server.cabal
Comment thread src/HlsPlugins.hs
]
tests = testGroup "diagnostics providers" [ warningTests ]

basicTests :: TestTree

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

seems marginally useful, are we just losing this?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

We are losing the example plugins. There's hardly any point when we have a nice tutorial and 25? existing plugins that can serve as example

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

👍 Might as well delete their source too then?

@michaelpj

Copy link
Copy Markdown
Collaborator

Looks like you're going ahead with a ghcide-bench package.

@pepeiborra

Copy link
Copy Markdown
Collaborator Author

Looks like you're going ahead with a ghcide-bench package.

Yeah, but I avoided the scary step 1 by duplicating some things from the ghcide test suite. Can be deleted when/if they are extracted to a helper package

The main goal here is to move the Plugins module into an internal
library so that it can be reused from the benchmark suite.

In order to
make that easier, and since they hardly serve a purpose in a repository
with 25 plugins, I delete the Example and Example2 plugin descriptors
and their dependencies.
Port the ghcide benchmark suite to HLS and benchmark plugin
"configurations" independently.

This includes the following changes to the ghcide benchmark suite and
HLS:
- Support for "configurations" which are defined as sets of plugin ids.
  The benchmark will be run with only these plugins enabled and all
  others disabled
- Support for configurable concurrency. This relies on RTS -ol and -po
  flags to place the RTS traces in the target location rather than in
  the cwd

This change requires two commits, the next one places
ghcide/bench/hist/Main.hs into its final location to help 'git'
recognize the change as a file move
Goal is to display the formatted CSV (via column) one row per line
We parse maxResidency and allocatedBytes from the RTS -S output, but runSessionWithHandles kills the server without waiting for it to exit and these stats don't get logged.

The solution is to use runSessionWithHandles', but unfortunately it is internal and not exposed. I have raised a PR to expose it and in the meantime we need a source repo package.
@pepeiborra
pepeiborra merged commit d0e3e0f into master Aug 25, 2022
sloorush pushed a commit to sloorush/haskell-language-server that referenced this pull request Sep 12, 2022
* extract ghcide:experiments-types

* extract haskell-language-server:plugins and let go of examples

The main goal here is to move the Plugins module into an internal
library so that it can be reused from the benchmark suite.

In order to
make that easier, and since they hardly serve a purpose in a repository
with 25 plugins, I delete the Example and Example2 plugin descriptors
and their dependencies.

* HLS benchmark suite

Port the ghcide benchmark suite to HLS and benchmark plugin
"configurations" independently.

This includes the following changes to the ghcide benchmark suite and
HLS:
- Support for "configurations" which are defined as sets of plugin ids.
  The benchmark will be run with only these plugins enabled and all
  others disabled
- Support for configurable concurrency. This relies on RTS -ol and -po
  flags to place the RTS traces in the target location rather than in
  the cwd

This change requires two commits, the next one places
ghcide/bench/hist/Main.hs into its final location to help 'git'
recognize the change as a file move

* ghcide/bench/hist/Main.hs -> bench/Main.hs

* CI - fix artifact names for uniqueness

* disable shorten HLS step

* Do not store eventlogs to avoid out of disk space

* render durations up to milliseconds

* shorten titles

Goal is to display the formatted CSV (via column) one row per line

* exclude formatting plugin configurations

* Extract ghcide-bench to a standalone package

* ghcide-bench: fix stderr capturing

* Fix mem stats

We parse maxResidency and allocatedBytes from the RTS -S output, but runSessionWithHandles kills the server without waiting for it to exit and these stats don't get logged.

The solution is to use runSessionWithHandles', but unfortunately it is internal and not exposed. I have raised a PR to expose it and in the meantime we need a source repo package.

* feedbacks

* delete Example plugins
@fendor

fendor commented Oct 6, 2022

Copy link
Copy Markdown
Collaborator

PR seems to have removed the example plugins, an accident?

edit: i see, not an accident

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.

3 participants