Commit Graph

18 Commits

Author SHA1 Message Date
Pepe Iborra
0d4e3b9499
Fix diagnostics update bug (#959)
* Preventively switch to uninterruptible mask in withMVar'

withMVar' is used to update the shakeSession var and it's crucial that the
third argument is not interrupted.

'mask' can still be interrupted for I/O actions and, while we were careful to
ensure none was used, if it ever breaks it will lead to very hard to debug
problems.

* refactor: move to RuleTypes

* Add a TestRequest to wait for arbitrary ide actions

Closes #955

* expectCurrentDiagnostics

* Add a test suite for cancellation

* Introduce --test-no-kick to fix cancellation tests reliability

* delete unsafeClearDiagnostics (unused)

* GetModSummaryWithoutTimestamps - remove StringBuffer

Since the contents of the buffer are not tracked by the fingerprint.

* Fix diagnostics bug

Given a FOI F with non null typechecking diagnostics D, imagine the following scenario:

1. An edit notification for F is received, creating a new version
2. GetModTime is executed, producing 0 diagnostics.
  2.1 updateFileDiagnostics is called
  2.2 setStageDiagnostics is called
  2.3 LSP.updateDiagnostics is called with a new version, resetting all the
  diagnostics for F
  2.4 newDiags=[] in updateFileDiagnostics, which is different from D (the last
  published diagnostics), which enqueues a new publishDiagnostics [] in the
  Debouncer
3. An edit notification for F is received before typechecking has a chance to
run which undoes the previous edit
4. The debouncer publishes the empty set of diagnostics after waiting 0.1s
5. GetFileContents runs and since the contents of the file haven't changed since
the last time it ran, early cutoff skips everything donwstream

Since TypeCheck is skipped, the empty set of diagnostics stays published until
another edit comes.

The goal of this change is to prevent setStageDiagnostics from losing
diagnostics from other stages. To achieve this, we recover the old diagnostics
for all stages and merge them with the new stage.

* Fix hlint

* Use Map.insert for clarity

* Fix redundant imports

* Fix "code actions after edit" experiment"
2020-12-21 06:06:51 +00:00
Pepe Iborra
baafe2cb82
Prepare for v0.6.0 release (#940)
* Prepare for v0.6.0 release

* Credit @mpardalos for the opentelemetry work
2020-12-09 08:35:09 +00:00
Pepe Iborra
959db7b10b
Extract the benchmarking Shake rules to a standalone Cabal package (#941)
* [bench-hist] break down in rule functions

* Extract the benchmarking Shake rules to a shake-bench package

There's some room for reusing the rules used in the historic benchmarking suite
in other projects. This change makes that a bit easier and improves the
documentation and code structure.

The new structure is:
- lib:shake-bench - a Cabal library with functions to generate Shake rules
- ghcide:bench:benchHist - the ghcide instantiation of the above Shake rules

That's not to say that shake-bench is completely decoupled from ghcide -
there are still plenty of assumptions on how the benchmarks are organized, their
outputs, etc. But with a little bit of effort, it should be easy to make
these rules more reusable

* Fix nix build

* Fix license

* hlints and redundant imports

* more hlints

* Exclude shake-bench from the stack build
2020-12-07 15:03:15 +00:00
Pepe Iborra
e24a744a06
Opentelemetry traces and heapsize memory analysis (#922)
* Move tracing functions to own module

* Bump opentelemetry to 0.6.0

* Write Values map size to OpenTelemetry metric

* Trace all requests and notifications

Instead of doing it in `HoverDefinition`, do it in
with{Response,Notification,...}. These wrap all handlers, so this should
cover everything. It also means that the span covers the entire
processing time for the request, where before we missed the setup
happening in the with* functions.

* Add flag for OpenTelemetry profiling

Run GC regularly with --ot-profiling

* Add flag to enable OT profiling in benchmark

* Use heapsize instead of ghc-datasize

I renamed the fork to distringuish from the original.
It is still being pulled from git using stack. This will be addressed
once I can push the fork to hackage.

* Bump opentelemetry to 0.6.1 - fixes 8.6 build

* Use heapsize from hackage

* Address HLint messages

* Record size of each key independently

* Refactor `startTelemetry` function

* Remove delay between measuring memory loops

* Each key in values map gets own OT instrument

* Measure values map length more rarely

* Rename --ot-profiling to --ot-memory-profiling

* Add docs for how to use the opentelemetry output

* Add instructions to build release version of tracy

* Clarify dependencies in opentelemetry instructions

* Fix LSP traces

* otTraced: delete unused

* Extract types out of D.IDE.Core.Shake

to avoid circular module dependencies

* Extract startTelemetry out of D.IDE.Shake and upgrade to 0.2

No more segfaults

* [nix] install opentelemetry

* [nix] install tracy

* Fix merge wibble

* Measure recursive sizes with sharing

* Sort keys for cost attribution

* Remove debug traces

* Allocate less, group keys, clean up hlints

* Add -A4G to the flags used for --ot-memory-profiling

* Modularize D.IDE.Core.Tracing

I want to reuse this code more directly in the non lsp driver

* Direct driver: report closure sizes when --ot-memory-profiling

An eventlog memory analysis doesnt' seem so relevant since this mode is not
interactive, but we could easily produce both if wanted to

* Everything is reachable from GhcSessionIO, so compute it last

I suspect the ShakeExtras record is reachable from GhcSessionIO

* bound recursion and use logger

* hlint suggestions

* Fix 8.6 build

* Format imports

* Do the memory analysis with full sharing. GhcSessionIO last

* Fail fast in the memory analysis

* error handling

* runHeapsize now takes initSize as an input argument

* Trace Shake sessions

* Reduced frequency for sampling values length

* Drop the -fexternal-interpreter flag in the Windows stack build

* Produce more benchmark artifacts

* Fix stack descriptors to use heapsize-0.2 from Hackage

* Bump to heapsize-0.3.0

* Record completions snippets (#900)

* Add field for RecordSnippets to CachcedCompletion

* Initial version of local record snippets

* Supprt record snippet completion for non local declarations.

* Better integration of local completions with current implementation

* Clean up non-local completions.

* Remove commented code.

* Switch from String to Text

* Remove ununsed definition

* Treat only Records and leave other defintions as is.

* Differentiate Records from Data constructors for external declaration

* Update test to include snippet in local record completions expected list.

* Update completionTest to also compare insertText.

* Add test for record snippet completion for imported records.

* Hlint fixes

* Hlint fixes

* Hlint suggestions.

* Update type.

* Consolidate imports

* Unpack tuple with explicit names

* Idiomatic changes

* Remove unused variable

* Better variable name

* Hlint suggestions

* Handle exhaustive pattern warning

* Add _ to snippet field name suggestions

* Remove type information passed around but not used

* Update to list comprehension style

* Eliminate intermediate function

* HLint suggestions.

* Idiomatic list comprehension

Co-authored-by: Pepe Iborra <pepeiborra@me.com>

* [nix] use gitignore.nix (#920)

* Ignore import list while producing completions (#919)

* Drop any items in explicit import list

* Test if imports not included in explicit list show up in completions

* Update README.md (#924)

* Custom cradle loading (#928)

When using ghcide as a library, it may be desirable to host the hie.yaml file
in a location other than the project root, or even avoid the file system altogether

* Favor `lookupPathToId` over `pathToId` (#926)

* Favor `lookupPathToId` over `pathToId`

* Fix `typecheckParentsAction`

* Fix `needsCompilationRule`

* Return completion snippets only when client supports it (#929)

* Use the real client capabilities on completions

* Return completion snippets only when supported by the client

Restored from https://github.com/haskell/ghcide/pull/900

* Redundant import

* Fix stack windows build

Co-authored-by: Michalis Pardalos <m.pardalos@gmail.com>
Co-authored-by: Michalis Pardalos <mpardalos@gmail.com>
Co-authored-by: Guru Devanla <gdevanla@users.noreply.github.com>
Co-authored-by: Samuel Ainsworth <skainsworth@gmail.com>
2020-12-05 17:44:17 +00:00
Pepe Iborra
1e17ed9b77
GitHub actions (#895)
* Add Github action for benchmarks

* Change action name to benchmark

* Fix - remove empty env section

* Rename step

* Add steps to print and upload results

* Shrink the matrix of versions for benchmarking

* Enable benchmarks

* rename job

* Fix fetch

* bump actions/setup-haskell

* disable windows - bench script requires Cairo

* Delete Azure bench script

* add comment on git fetch call

* clean up cache key

* Update archive step
2020-11-07 19:57:50 +00:00
Pepe Iborra
c92ccfed6f
Disable CI benchmark suite (#893)
* Test the stack version in the benchmark CI script

* [bench script] specify cwd in findGhc

* Disable CI bench script
2020-11-07 11:35:18 +00:00
Pepe Iborra
10dbde048e
Interleave and pretty print benchmark results (#866)
* Interleave benchmark results

* Pretty print benchmark results
2020-10-11 22:33:25 +01:00
Pepe Iborra
7339784509
Run benchmarks on a list of examples (#864)
- Cabal 3.0.0.0
- haskell-lsp-types 0.22.0.0
2020-10-11 20:10:15 +01:00
Pepe Iborra
a52741838b
Allow to easily customise the example used for benchmarks (#838)
* [ghcide-bench] allow custom example

* [bench] allow custom example

* Add v0.4.0 entry for completeness

* Rename benchmark artifacts

bench/hist.yaml --> bench/config.yaml

bench-hist --> bench-results

* Fix Cabal file

* Fix tests

* No need for hardcoded experiment positions
2020-09-29 07:47:09 +01:00
Pepe Iborra
1d1f2db3bd
Enhance benchmarks & bug fixes (#823)
* parse allocations

* WaitForShakeQueue

* Measure user time and shake time in experiments

* clean ups

* Prevent a potential crash of the shake enqueue thread

* Fix a bug that was preventing reenqueud actions from getting flushed

* Avoid running the check-project action per file

What we really want is to check the project once per cradle

* Backwards compat.

* Review feedback

* Fix typo

Co-authored-by: Neil Mitchell <ndmitchell@gmail.com>

Co-authored-by: Neil Mitchell <ndmitchell@gmail.com>
2020-09-20 11:03:51 +01:00
Pepe Iborra
0d7cae9846
Improve hist benchmarks driver and add to CI (#770)
* Remove hardcoded --stack-yaml and upstream/master assumption

* support Cabal in bench suite

* add benchmark run to CI

Even if the time measurements are unreliable in a shared CI environment, the
memory usage will be an accurate indicator of space leaks

* Update bench/README

* use origin/master

* default to stack in benchmarks (for CI)

* ignore ghcide-bench and ghcide-preprocessor binaries too

* Review feedbacks

* Add the v0.3.0 tag in bench/hist.yaml

commented out to keep the CI time as tight as possible

* Add .artifactignore file to avoid publishing binaries in azure bench pipeline

* use default stack.yaml
2020-09-06 21:54:45 +01:00
Pepe Iborra
7dc6e2678a
Completions need not depend on typecheck of the current file (#670)
* Faster completions

* optimize withProgressVar

We never remove elements from the map so alter is unnecesary

* [ghcide-bench] accept ghcide options

* Expand completion tests suite

* hlints

* completions for local foreign decls

* Minor improvements for local completions

* Restore completion docs in legacy code path

* Compatibility with GHC < 8.8

* fix merge issue

* address review feedback
2020-07-06 15:06:10 +02:00
Pepe Iborra
bd51ad0a63
Make BenchHist non buildable by default and save logs (#666)
* [bench-hist] save messages to log file

And fix the commitid rule to always rerun the git query

* Do not build benchHist by default

Hopefully avoiding the additional dependencies for charts

* Simplify with FileStdout

* Add a flag for bench-hist

Could benchHist be a benchmark instead of an executable, removing the need
for this flag?

Almost. `stack bench` fails because `benchHist` cannot find `ghcide-bench`
in the path. It seems like a bad idea to have a benchmark that fails out of
the box

* Turn benchHist into a benchmark and ghcide-bench into an exe

This works out nicely because:

1. benchHist already runs ghcide-bench,
2. benchHist has additional deps, but ghcide-bench does not.
   (benchmark deps don't get built by default)
3. This is the only way I've found to get ghcide-bench in the PATH for benchHist

* Remove redundant dep on applicative-combinators

* Bump versions in stack-ghc-lib.yaml

* update lower bounds for extra in benchHist executable

* Update README guideline on benchmarks

* [benchHist] Fix the commitid rule to always rerun the git query

* fix caps
2020-06-29 11:00:53 +02:00
Pepe Iborra
ba4bdb2def
Send WorkDoneProgressEnd only when work is done (#649)
* send WorkDoneProgressEnd only when work done

* Progress reporting now spans over multiple overlapping kicks

* Repurpose benchmark experiments as tests

Fixes #650

* use stack to fetch from Hackage

* benchmark tests run with the same lsp-test config as other tests

* Fix stack cradle in benchmark

* Make stack unpack --silent

* Fix issues in "code actions after edit" experiment

- Repeated breaking edits make ghc run out of suggestions

- Diagnostics seem to come and go in-between edits, which leads to a timing
  issue when asking for code actions. The fix is to wait for diagnostics to be
  present before asking for code actions

* Fix stack.yaml generation in example project

* Fix getDefinition in GHC 8.4

Did it break before 0.2.0 or after?

* better naming for the progress event TVar

* stop progress reporting in shakeShut

https://github.com/digital-asset/ghcide/pull/649#discussion_r443408884

* hlint
2020-06-22 12:47:45 +02:00
Pepe Iborra
7e9326be08
Write a cabal.project file in the benchmark example (#640)
* Write a cabal.project file

As suggested in #617. Taken fron #624

* Write a cabal.project.local

Otherwise Cabal still errors out

* Override default hie dir

Otherwise .hi and .hie files end up in different locations, which causes the getDefinition experiment to fail the second time it's run.

This is because we assume in ghcide that .hi and .hie files have the same lifetimes, which is not true when the ..hie files are wiped but the .hi files aren't.
2020-06-15 15:48:21 +02:00
Pepe Iborra
0e96f61d1b
Performance analysis over time (#629)
* benchmark history script

* if HEAD no need to rebuild worktree

* add bench/README.md

* Enable all experiments

* Fix dependency tracking for git branches

* hlints

* Add stack84 extra-deps

* Identify failed experiments in graphs

* Filter our failed benchmarks from aggregate graphs

Otherwise they tend to distort the axis

* Improve graphs (more and easier to see colors)

* update cradles

* customizable output folder

* Cache the config for the duration of the script

Otherwise the script is vulnerable to config edits

* Allow omitting the git: field

* Ignore bench-hist intermediate artifacts

Handy for including bench-hist results in a branch while avoiding the
intermediate artifacts
2020-06-15 13:56:24 +02:00
Pepe Iborra
4e7b2fcdbb
More benchmarks (#625)
* Add a benchmark to track startup times

* Benchmark automation

disable benchmarks easily

save GC stats to file

cradle, rts, filter and samples options

path to ghcide

configurable example

--help

more detailed CSV output

hover after edit

pause for GC

configurable timeout

upgrade extra (required to build bench)

Include max residency in BenchRun

Include all details on output

* reduce threadDelay to avoid upsetting lsp-test

* Fix startup time measurement

* Added new edit experiment

* fix doc comment

* hlints

* Upgrade to lsp-test 0.11.0.2

* Flag failed experiments

* Update ghcide.cabal
2020-06-12 20:46:55 +02:00
Pepe Iborra
5a754e1bb9
Benchmark suite (#590)
* 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
2020-06-03 16:35:08 +02:00