This repository has been archived by the owner on Jan 2, 2021. It is now read-only.
Interface file fixes #645
Merged
Interface file fixes #645
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This is a massive performance regression. Not sure how that happened: Will investigate, moving it to draft |
|
I have tested this locally and it seems to work as expected. Thanks! |
|
This is ready for review now. Updated benchmark results: |
This result seems strange |
Benchmarks are failing or succeeding with inaccurate timings because of multiple |
|
We should reuse the benchmark experiments as tests, to make sure they stay running. |
cocreature
reviewed
Jun 22, 2020
test/data/recomp/hie.yaml
Outdated
| @@ -0,0 +1 @@ | |||
| cradle: {cabal: {component: "lib:recomp"}} | |||
There was a problem hiding this comment.
Do we need this to be a cabal cradle? I’d rather limit that to stuff where we cannot easily use a direct-style cradle (mainly multi-component).
There was a problem hiding this comment.
@wz1000 any thoughts?
There was a problem hiding this comment.
I think a direct cradle would also work, I can make a PR by tonight if you don't get to it sooner.
There was a problem hiding this comment.
When I try switching to the trivial direct cradle with only the -ilib option, I see ghcide creating a lot more HscEnvEq environments, which results in a larger volume of NotWorkDoneProgressEnd messages which confuse the test and make it fail.
I think that multi-cradle requires cradles to enumerate all the modules, but I'm not sure how to do that with a direct cradle.
Test logs with my direct cradle:
> Executing task: nix-shell --run 'cabal test --test-show-details=always --test-options="-p iface-error-test-1"' <
Warning: The package list for 'hackage.haskell.org' is 44 days old.
Run 'cabal update' to get the latest list of available packages.
Resolving dependencies...
Build profile: -w ghc-8.10.1 -O1
In order, the following will be built (use -v for more details):
- ghcide-0.2.0 (lib) (configuration changed)
- ghcide-0.2.0 (exe:ghcide-test-preprocessor) (configuration changed)
- ghcide-0.2.0 (exe:ghcide) (configuration changed)
- ghcide-0.2.0 (test:ghcide-tests) (configuration changed)
Configuring library for ghcide-0.2.0..
Configuring executable 'ghcide-test-preprocessor' for ghcide-0.2.0..
Preprocessing executable 'ghcide-test-preprocessor' for ghcide-0.2.0..
Building executable 'ghcide-test-preprocessor' for ghcide-0.2.0..
Preprocessing library for ghcide-0.2.0..
Building library for ghcide-0.2.0..
Configuring executable 'ghcide' for ghcide-0.2.0..
Warning: The package has an extraneous version range for a dependency on an
internal library: ghcide -any && ==0.2.0, ghcide -any && ==0.2.0, ghcide -any
&& ==0.2.0. This version range includes the current package but isn't needed
as the current package's library will always be used.
Preprocessing executable 'ghcide' for ghcide-0.2.0..
Building executable 'ghcide' for ghcide-0.2.0..
Configuring test suite 'ghcide-tests' for ghcide-0.2.0..
Warning: The package has an extraneous version range for a dependency on an
internal library: ghcide -any && ==0.2.0, ghcide -any && ==0.2.0, ghcide -any
&& ==0.2.0. This version range includes the current package but isn't needed
as the current package's library will always be used.
Preprocessing test suite 'ghcide-tests' for ghcide-0.2.0..
Building test suite 'ghcide-tests' for ghcide-0.2.0..
Running 1 test suites...
Test suite ghcide-tests: RUNNING...
HIE
Interface loading tests
iface-error-test-1: --> {
"jsonrpc": "2.0",
"params": {
"rootUri": "file:///run/user/1000/snap.code/extra-dir-68570152739173",
"processId": 237877,
"rootPath": "/run/user/1000/snap.code/extra-dir-68570152739173",
"capabilities": {
"window": {
"workDoneProgress": true
},
"workspace": {
"didChangeWatchedFiles": {
"dynamicRegistration": true
},
"symbol": {
"symbolKind": {
"valueSet": [
1,
2,
3,
4,
5,
6,
7,
8,
9,
10,
11,
12,
13,
14,
15,
16,
17,
18,
19,
20,
21,
22,
23,
24,
25,
26
]
},
"dynamicRegistration": true
},
"workspaceEdit": {
"documentChanges": true
},
"didChangeConfiguration": {
"dynamicRegistration": true
},
"applyEdit": true,
"executeCommand": {
"dynamicRegistration": true
},
"workspaceFolders": true,
"configuration": true
},
"textDocument": {
"completion": {
"dynamicRegistration": true,
"contextSupport": true,
"completionItem": {
"tagSupport": {
"valueSet": [
1
]
},
"preselectSupport": true,
"commitCharactersSupport": true,
"snippetSupport": true,
"deprecatedSupport": true,
"documentationFormat": [
"plaintext",
"markdown"
]
},
"completionItemKind": {
"valueSet": [
1,
2,
3,
4,
5,
6,
7,
8,
9,
10,
11,
12,
13,
14,
15,
16,
17,
18,
19,
20,
21,
22,
23,
24,
25
]
}
},
"documentHighlight": {
"dynamicRegistration": true
},
"synchronization": {
"dynamicRegistration": true,
"willSave": true,
"didSave": true,
"willSaveWaitUntil": true
},
"definition": {
"dynamicRegistration": true
},
"signatureHelp": {
"dynamicRegistration": true,
"signatureInformation": {
"ocumentationFormat": [
"plaintext",
"markdown"
]
}
},
"references": {
"dynamicRegistration": true
},
"rangeFormatting": {
"dynamicRegistration": true
},
"codeAction": {
"codeActionLiteralSupport": {
"codeActionKind": {
"valueSet": [
"quickfix",
"refactor",
"refactor.extract",
"refactor.inline",
"refactor.rewrite",
"source",
"source.organizeImports"
]
}
},
"dynamicRegistration": true
},
"foldingRange": {
"lineFoldingOnly": false,
"dynamicRegistration": true
},
"onTypeFormatting": {
"dynamicRegistration": true
},
"codeLens": {
"dynamicRegistration": true
},
"documentSymbol": {
"hierarchicalDocumentSymbolSupport": true,
"symbolKind": {
"valueSet": [
1,
2,
3,
4,
5,
6,
7,
8,
9,
10,
11,
12,
13,
14,
15,
16,
17,
18,
19,
20,
21,
22,
23,
24,
25,
26
]
},
"dynamicRegistration": true
},
"colorProvider": {
"dynamicRegistration": true
},
"documentLink": {
"dynamicRegistration": true
},
"formatting": {
"dynamicRegistration": true
},
"implementation": {
"dynamicRegistration": true
},
"typeDefinition": {
"dynamicRegistration": true
},
"publishDiagnostics": {
"tagSupport": {
"valueSet": [
1,
2
]
},
"relatedInformation": true
},
"rename": {
"prepareSupport": true,
"dynamicRegistration": true
},
"hover": {
"contentFormat": [
"plaintext",
"markdown"
],
"dynamicRegistration": true
}
}
},
"trace": "off"
},
"method": "initialize",
"id": 0
}
ghcide version: 0.2.0 (GHC: 8.10.1) (PATH: /home/pepe/scratch/ghcide/dist-newstyle/build/x86_64-linux/ghc-8.10.1/ghcide-0.2.0/x/ghcide/build/ghcide/ghcide) (GIT hash: f07857babd6ca476ed1d89c1d8c189eec5017d51)
Starting LSP server...
If you are seeing this in a terminal, you probably should have run ghcide WITHOUT the --lsp option!
Started LSP server in 0.00s
<-- {
"tag": "RspInitialize",
"contents": {
"result": {
"capabilities": {
"typeDefinitionProvider": true,
"foldingRangeProvider": false,
"textDocumentSync": {
"openClose": true,
"change": 2,
"save": {}
},
"workspace": {
"workspaceFolders": {
"supported": true,
"changeNotifications": true
}
},
"implementationProvider": true,
"executeCommandProvider": {
"commands": [
"237883:ghcide:typesignature.add"
]
},
"renameProvider": false,
"colorProvider": false,
"definitionProvider": true,
"hoverProvider": true,
"codeActionProvider": true,
"completionProvider": {
"triggerCharacters": [
"."
],
"resolveProvider": false
},
"codeLensProvider": {},
"documentSymbolProvider": true
}
},
"jsonrpc": "2.0",
"id": 0
}
}
--> {
"jsonrpc": "2.0",
"params": {},
"method": "initialized"
}
--> {
"jsonrpc": "2.0",
"params": {
"textDocument": {
"languageId": "haskell",
"text": "module B(y) where\n\ny :: Int\ny = undefined\n",
"uri": "file:///run/user/1000/snap.code/extra-dir-68570152739173/B.hs",
"version": 0
}
},
"method": "textDocument/didOpen"
}
--> {
"jsonrpc": "2.0",
"params": {
"textDocument": {
"languageId": "haskell",
"text": "module P() where\nimport A\nimport B\n\nbar = x :: Int\n",
"uri": "file:///run/user/1000/snap.code/extra-dir-68570152739173/P.hs",
"version": 0
}
},
"method": "textDocument/didOpen"
}
[DEBUG] Set files of interest to: [NormalizedFilePath "/run/user/1000/snap.code/extra-dir-68570152739173/B.hs"]
[DEBUG] Restarting build session (aborting the previous one took 0.00s)
[INFO] Opened text document: file:///run/user/1000/snap.code/extra-dir-68570152739173/B.hs
[DEBUG] Set files of interest to: [NormalizedFilePath "/run/user/1000/snap.code/extra-dir-68570152739173/P.hs",NormalizedFilePath "/run/user/1000/snap.code/extra-dir-68570152739173/B.hs"]
[DEBUG] Finishing build session(exception: AsyncCancelled)
[DEBUG] Restarting build session (aborting the previous one took 0.00s)
[INFO] Opened text document: file:///run/user/1000/snap.code/extra-dir-68570152739173/P.hs
<-- {
"tag": "ReqWorkDoneProgressCreate",
"contents": {
"jsonrpc": "2.0",
"params": {
"token": "1"
},
"method": "window/workDoneProgress/create",
"id": 0
}
}
<-- {
"tag": "NotWorkDoneProgressBegin",
"contents": {
"jsonrpc": "2.0",
"params": {
"value": {
"kind": "begin",
"title": "Processing"
},
"token": "1"
},
"method": "$/progress"
}
}
[INFO] Consulting the cradle for "/run/user/1000/snap.code/extra-dir-68570152739173/P.hs"
<-- {
"tag": "NotCustomServer",
"contents": {
"jsonrpc": "2.0",
"params": "/run/user/1000/snap.code/extra-dir-68570152739173/P.hs",
"method": "ghcide/cradle/loaded"
}
}
[DEBUG] Session loading result: Right (ComponentOptions {componentOptions = [], componentRoot = "/run/user/1000/snap.code/extra-dir-68570152739173", componentDependencies = []})
<-- {
"tag": "ReqWorkDoneProgressCreate",
"contents": {
"jsonrpc": "2.0",
"params": {
"token": 0
},
"method": "window/workDoneProgress/create",
"id": 1
}
}
<-- {
"tag": "NotWorkDoneProgressBegin",
"contents": {
"jsonrpc": "2.0",
"params": {
"value": {
"kind": "begin",
"cancellable": false,
"title": "Setting up project extra-dir-68570152739173"
},
"token": 0
},
"method": "$/progress"
}
}
<-- {
"tag": "NotWorkDoneProgressEnd",
"contents": {
"jsonrpc": "2.0",
"params": {
"value": {
"kind": "end"
},
"token": 0
},
"method": "$/progress"
}
}
[INFO] Using interface files cache dir: /home/pepe/.cache/ghcide/main-da39a3ee5e6b4b0d3255bfef95601890afd80709
[INFO] Making new HscEnv[main]
[DEBUG] New Component Cache HscEnvEq: (([],Just HscEnvEq 2),fromList [("/run/user/1000/snap.code/extra-dir-68570152739173/hie.yaml",Just 2020-06-23 07:33:13.724195737 UTC)])
[DEBUG] Restarting build session (aborting the previous one took 0.00s)
[DEBUG] Finishing build session(exception: AsyncCancelled)
[INFO] Consulting the cradle for "/run/user/1000/snap.code/extra-dir-68570152739173/B.hs"
[DEBUG] Session loading result: Right (ComponentOptions {componentOptions = [], componentRoot = "/run/user/1000/snap.code/extra-dir-68570152739173", componentDependencies = []})
<-- {
"tag": "NotCustomServer",
"contents": {
"jsonrpc": "2.0",
"params": "/run/user/1000/snap.code/extra-dir-68570152739173/B.hs",
"method": "ghcide/cradle/loaded"
}
}
<-- {
"tag": "ReqWorkDoneProgressCreate",
"contents": {
"jsonrpc": "2.0",
"params": {
"token": 1
},
"method": "window/workDoneProgress/create",
"id": 2
}
}
<-- {
"tag": "NotWorkDoneProgressBegin",
"contents": {
"jsonrpc": "2.0",
"params": {
"value": {
"kind": "begin",
"cancellable": false,
"title": "Setting up project extra-dir-68570152739173"
},
"token": 1
},
"method": "$/progress"
}
}
<-- {
"tag": "NotWorkDoneProgressEnd",
"contents": {
"jsonrpc": "2.0",
"params": {
"value": {
"kind": "end"
},
"token": 1
},
"method": "$/progress"
}
}
[INFO] Using interface files cache dir: /home/pepe/.cache/ghcide/main-da39a3ee5e6b4b0d3255bfef95601890afd80709
[INFO] Using interface files cache dir: /home/pepe/.cache/ghcide/main-da39a3ee5e6b4b0d3255bfef95601890afd80709
[INFO] Making new HscEnv[main,main]
[DEBUG] New Component Cache HscEnvEq: (([],Just HscEnvEq 3),fromList [("/run/user/1000/snap.code/extra-dir-68570152739173/hie.yaml",Just 2020-06-23 07:33:13.724195737 UTC)])
[DEBUG] New Component Cache HscEnvEq: (([],Just HscEnvEq 4),fromList [("/run/user/1000/snap.code/extra-dir-68570152739173/hie.yaml",Just 2020-06-23 07:33:13.724195737 UTC)])
[DEBUG] Restarting build session (aborting the previous one took 0.00s)
[DEBUG] Finishing build session(exception: AsyncCancelled)
<-- {
"tag": "ReqRegisterCapability",
"contents": {
"jsonrpc": "2.0",
"params": {
"registrations": [
{
The terminal process terminated with exit code: 1
Terminal will be reused by tasks, press any key to close it.
There was a problem hiding this comment.
Oh that’s a good point! What happens if you just specify the module names after the -i option? I believe that’s how cabal repl does it.
There was a problem hiding this comment.
See pepeiborra#3
I didn't make the PR yesterday because I'm unable to compile the tests because of a weird linker error. Right now I've nuked my cabal store and am rebuilding the world from scratch. I will let you know how that goes.
I think this issue should not come up with my changes because I removed the lib directory
There was a problem hiding this comment.
Your direct cradle enumerates all the modules, which I didn't. I've merged it to the branch, let's see if the tests pass.
There's a comment about this in the commit message of the first commit. Reproducing it here:
So previously we were not handling the return value of Moreover, we were not loading all the transitive dependencies in the Ghc session used by |
pepeiborra
force-pushed
the
hi-file-changes
branch
from
f48e268 to
b8f97c1
Compare
|
rebased |
|
Thanks for the explanation! |
This started as a pure refactoring to clarify the responsibilities between ModIface and ModIfaceFromDisk, but ended up having some behaviour changes: 1. Regenerate interface when checkOldIface returns something other than UpToDate. This was a bug. 2. Do not generate a diagnostic when regenerating an interface. 2. Previously we conflated stale interface with other errors, and would regenerate in both cases. Now we only regenerate in the first case. Tentative fix for #597
Tentative fix for #614 TODO support stability
The previous changes were 10X slower, this is 20X faster than those, so 2X faster than upstream, for some benchmarks
The answer for a GetModification query is independent of the missingFileDiagnostics field (as the diagnostics are not part of the answer)
Co-authored-by: Moritz Kiefer <[email protected]>
pepeiborra
force-pushed
the
hi-file-changes
branch
from
b8f97c1 to
7cea8e3
Compare
Change recomp to direct cradle
cocreature
approved these changes
Jun 23, 2020
There was a problem hiding this comment.
Nice! Thanks @wz1000 for switching it over to a direct cradle.
pepeiborra
added a commit
to pepeiborra/ide
that referenced
this pull request
Dec 29, 2020
* Add test for inconsistent diagnostics * Refactoring ModIfaceFromDisk This started as a pure refactoring to clarify the responsibilities between ModIface and ModIfaceFromDisk, but ended up having some behaviour changes: 1. Regenerate interface when checkOldIface returns something other than UpToDate. This was a bug. 2. Do not generate a diagnostic when regenerating an interface. 2. Previously we conflated stale interface with other errors, and would regenerate in both cases. Now we only regenerate in the first case. Tentative fix for haskell/ghcide#597 * Split interface tests * Always recompile modules with TH splices Tentative fix for haskell/ghcide#614 TODO support stability * Fix expectDiagnostics in MacOs * Avoid File does not exist diagnostics for interface files Fixes haskell/ghcide#642 * Clarify interface tests * hlints * Performance fixes The previous changes were 10X slower, this is 20X faster than those, so 2X faster than upstream, for some benchmarks * formatting * Fix GetModificationTime identity The answer for a GetModification query is independent of the missingFileDiagnostics field (as the diagnostics are not part of the answer) * remove stale comment * Avoid calling ghcSessionDepsDefinition twice * Apply suggestions from code review Co-authored-by: Moritz Kiefer <[email protected]> * Code review feedback * Address review feedback https://github.com/digital-asset/ghcide/pull/645/files/49b0d9ac65399edf82a7a9cbbb8d8b5420458d8dhaskell/ghcide#r443383239 * Change recomp to direct cradle Co-authored-by: Zubin Duggal <[email protected]> Co-authored-by: Moritz Kiefer <[email protected]>
pepeiborra
added a commit
to pepeiborra/ide
that referenced
this pull request
Dec 29, 2020
* Add test for inconsistent diagnostics * Refactoring ModIfaceFromDisk This started as a pure refactoring to clarify the responsibilities between ModIface and ModIfaceFromDisk, but ended up having some behaviour changes: 1. Regenerate interface when checkOldIface returns something other than UpToDate. This was a bug. 2. Do not generate a diagnostic when regenerating an interface. 2. Previously we conflated stale interface with other errors, and would regenerate in both cases. Now we only regenerate in the first case. Tentative fix for haskell/ghcide#597 * Split interface tests * Always recompile modules with TH splices Tentative fix for haskell/ghcide#614 TODO support stability * Fix expectDiagnostics in MacOs * Avoid File does not exist diagnostics for interface files Fixes haskell/ghcide#642 * Clarify interface tests * hlints * Performance fixes The previous changes were 10X slower, this is 20X faster than those, so 2X faster than upstream, for some benchmarks * formatting * Fix GetModificationTime identity The answer for a GetModification query is independent of the missingFileDiagnostics field (as the diagnostics are not part of the answer) * remove stale comment * Avoid calling ghcSessionDepsDefinition twice * Apply suggestions from code review Co-authored-by: Moritz Kiefer <[email protected]> * Code review feedback * Address review feedback https://github.com/digital-asset/ghcide/pull/645/files/49b0d9ac65399edf82a7a9cbbb8d8b5420458d8dhaskell/ghcide#r443383239 * Change recomp to direct cradle Co-authored-by: Zubin Duggal <[email protected]> Co-authored-by: Moritz Kiefer <[email protected]>
pepeiborra
added a commit
to pepeiborra/ide
that referenced
this pull request
Dec 29, 2020
* Add test for inconsistent diagnostics * Refactoring ModIfaceFromDisk This started as a pure refactoring to clarify the responsibilities between ModIface and ModIfaceFromDisk, but ended up having some behaviour changes: 1. Regenerate interface when checkOldIface returns something other than UpToDate. This was a bug. 2. Do not generate a diagnostic when regenerating an interface. 2. Previously we conflated stale interface with other errors, and would regenerate in both cases. Now we only regenerate in the first case. Tentative fix for haskell/ghcide#597 * Split interface tests * Always recompile modules with TH splices Tentative fix for haskell/ghcide#614 TODO support stability * Fix expectDiagnostics in MacOs * Avoid File does not exist diagnostics for interface files Fixes haskell/ghcide#642 * Clarify interface tests * hlints * Performance fixes The previous changes were 10X slower, this is 20X faster than those, so 2X faster than upstream, for some benchmarks * formatting * Fix GetModificationTime identity The answer for a GetModification query is independent of the missingFileDiagnostics field (as the diagnostics are not part of the answer) * remove stale comment * Avoid calling ghcSessionDepsDefinition twice * Apply suggestions from code review Co-authored-by: Moritz Kiefer <[email protected]> * Code review feedback * Address review feedback https://github.com/digital-asset/ghcide/pull/645/files/49b0d9ac65399edf82a7a9cbbb8d8b5420458d8dhaskell/ghcide#r443383239 * Change recomp to direct cradle Co-authored-by: Zubin Duggal <[email protected]> Co-authored-by: Moritz Kiefer <[email protected]>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Includes potential fixes for several issues as well as new tests contributed by @wz1000
I still don't fully understand how much recompilation avoidance is provided by GHC and how much needs to be implemented by the application, so it's possible that further work is needed.