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.

Sign up for GitHub

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

Improve performance of NormalizedFilePath #3067

Merged
merged 17 commits into from
Sep 8, 2022
Merged

Improve performance of NormalizedFilePath #3067

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
0fd58f5
upgrade lsp
kokobd Aug 27, 2022
5363f87
modify default benchmark config
kokobd Aug 27, 2022
d435a41
upgrade lsp
kokobd Aug 29, 2022
19d87ff
use lsp master
kokobd Sep 2, 2022
fc3d8f7
temp: compare benchmark with previous commit
kokobd Sep 2, 2022
1868368
Merge remote-tracking branch 'origin/master' into kokobd/os-path
kokobd Sep 3, 2022
3fa1858
use text in NormalizedFilePath
kokobd Sep 3, 2022
b33c782
upgrade to lsp master
kokobd Sep 4, 2022
5db0317
fix stack config
kokobd Sep 4, 2022
85e2172
remove obsolete dir form ghcide.cabal
kokobd Sep 4, 2022
e220d39
Merge branch 'master' into kokobd/os-path
kokobd Sep 6, 2022
1c2a11d
run pre-commit without args
kokobd Sep 7, 2022
d006ef0
Merge branch 'kokobd/os-path' of github.com:haskell/haskell-language-…
kokobd Sep 7, 2022
2bb1c19
Revert "run pre-commit without args"
kokobd Sep 7, 2022
912673f
Merge branch 'master' into kokobd/os-path
kokobd Sep 8, 2022
5256023
remove unnecessary tests
kokobd Sep 8, 2022
fb75394
Merge branch 'kokobd/os-path' of github.com:haskell/haskell-language-…
kokobd Sep 8, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
110 changes: 56 additions & 54 deletions bench/config.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ versions:


# - 1.8.0.0
# - upstream: origin/master
- upstream: origin/master
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think we should compare with origin/master by default, so that we can see if a PR makes the performance better or worse.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, I disabled it temporarily when merging the benchmark suite PR because upstream/master lacked some fixes that were required to run it. Thanks for enabling it!

# - HEAD~1
- HEAD

Expand All @@ -95,26 +95,28 @@ configurations:
# The implicitly included plugins are:
# - ghcide-core
# - ghcide-hover-and-symbols
- 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-completions
- ghcide-type-lenses
- Refactor:
- ghcide-code-actions-bindings
- ghcide-code-actions-fill-holes
- ghcide-code-actions-imports-exports
- ghcide-code-actions-type-signatures

# Uncomment below sections if needed
# - 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-completions
# - ghcide-type-lenses
# - Refactor:
# - ghcide-code-actions-bindings
# - ghcide-code-actions-fill-holes
# - ghcide-code-actions-imports-exports
# - ghcide-code-actions-type-signatures
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Enabling all of these is very slow to run, and the generated chart contains too many lines and is too hard to read. A good default is only to enable the "All" group.

We can see the difference.
Before:
image
After:
image

Copy link
Collaborator

Choose a reason for hiding this comment

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

Seems reasonable

- All:
- alternateNumberFormat
- callHierarchy
Expand All @@ -141,36 +143,36 @@ configurations:
- 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
# - 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
17 changes: 14 additions & 3 deletions cabal.project
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,13 +81,24 @@ source-repository-package
location: https://github.com/wz1000/hie-bios
tag: aa73d3d2eb89df0003d2468a105e326d71b62cc7

-- Needed for ghcide-bench until a new release of lsp-test is out
-- Remove me when a new version of lsp is released
source-repository-package
type:git
location: https://github.com/haskell/lsp
subdir: lsp
tag: b0f8596887088b8ab65fc1015c773f45b47234ae

source-repository-package
type:git
location: https://github.com/haskell/lsp
subdir: lsp-types
tag: b0f8596887088b8ab65fc1015c773f45b47234ae

source-repository-package
type:git
location: https://github.com/haskell/lsp
subdir: lsp-test
tag: c95eb06c70c35f1e13c37ed11a7d9e5b36bfa2e8
-- https://github.com/haskell/lsp/pull/450
tag: b0f8596887088b8ab65fc1015c773f45b47234ae

allow-newer:
-- ghc-9.4
Expand Down
2 changes: 1 addition & 1 deletion ghcide/ghcide.cabal
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,7 @@ test-suite ghcide-tests
record-hasfield
if impl(ghc < 9.3)
build-depends: ghc-typelits-knownnat
hs-source-dirs: test/cabal test/exe test/src bench/lib
hs-source-dirs: test/cabal test/exe test/src
ghc-options: -threaded -Wall -Wno-name-shadowing -O0 -Wno-unticked-promoted-constructors
main-is: Main.hs
other-modules:
Expand Down
6 changes: 1 addition & 5 deletions ghcide/src/Development/IDE/Types/Location.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,7 @@ toNormalizedFilePath' "" = emptyFilePath
toNormalizedFilePath' fp = LSP.toNormalizedFilePath fp

emptyFilePath :: LSP.NormalizedFilePath
#if MIN_VERSION_lsp_types(1,3,0)
emptyFilePath = LSP.normalizedFilePath emptyPathUri ""
#else
emptyFilePath = LSP.NormalizedFilePath emptyPathUri ""
#endif
emptyFilePath = LSP.emptyNormalizedFilePath

-- | We use an empty string as a filepath when we don’t have a file.
-- However, haskell-lsp doesn’t support that in uriToFilePath and given
Expand Down
5 changes: 1 addition & 4 deletions hls-plugin-api/src/Ide/PluginUtils.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,10 +218,7 @@ fullRange s = Range startPos endPos
lastLine = fromIntegral $ length $ T.lines s

subRange :: Range -> Range -> Bool
subRange smallRange range = _start smallRange >= _start range && _end smallRange <= _end range

positionInRange :: Position -> Range -> Bool
positionInRange p (Range sp ep) = sp <= p && p < ep -- Range's end position is exclusive, see https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#range
subRange = isSubrangeOf

-- ---------------------------------------------------------------------

Expand Down
18 changes: 1 addition & 17 deletions hls-plugin-api/test/Ide/PluginUtilsTest.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,21 +9,5 @@ import Test.Tasty.HUnit

tests :: TestTree
tests = testGroup "PluginUtils"
[ positionInRangeTest
pepeiborra marked this conversation as resolved.
Show resolved Hide resolved
]

positionInRangeTest :: TestTree
positionInRangeTest = testGroup "positionInRange"
[ testCase "single line, after the end" $
positionInRange (Position 1 10) (Range (Position 1 1) (Position 1 3)) @?= False
, testCase "single line, before the begining" $
positionInRange (Position 1 0) (Range (Position 1 1) (Position 1 6)) @?= False
, testCase "single line, in range" $
positionInRange (Position 1 5) (Range (Position 1 1) (Position 1 6)) @?= True
, testCase "single line, at the end" $
positionInRange (Position 1 5) (Range (Position 1 1) (Position 1 5)) @?= False
, testCase "multiline, in range" $
positionInRange (Position 3 5) (Range (Position 1 1) (Position 5 6)) @?= True
, testCase "multiline, out of range" $
positionInRange (Position 3 5) (Range (Position 3 6) (Position 4 10)) @?= False
[
]
9 changes: 6 additions & 3 deletions stack-lts16.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,9 +87,12 @@ extra-deps:
- constraints-extras-0.3.0.2@sha256:013b8d0392582c6ca068e226718a4fe8be8e22321cc0634f6115505bf377ad26,1853
- some-1.0.1@sha256:26e5bab7276f48b25ea8660d3fd1166c0f20fd497dac879a40f408e23211f93e,2055
- unliftio-core-0.2.0.1@sha256:9b3e44ea9aacacbfc35b3b54015af450091916ac3618a41868ebf6546977659a,1082
- lsp-1.5.0.0
- lsp-types-1.5.0.0
- lsp-test-0.14.0.3
- git: [email protected]:haskell/lsp
commit: b0f8596887088b8ab65fc1015c773f45b47234ae
subdirs:
- lsp
- lsp-types
- lsp-test
- stm-containers-1.1.0.4
- stm-hamt-1.2.0.6@sha256:fba86ccb4b45c5706c19b0e1315ba63dcac3b5d71de945ec001ba921fae80061,3972
- primitive-extras-0.10.1
Expand Down
9 changes: 6 additions & 3 deletions stack-lts19.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,12 @@ extra-deps:
- refinery-0.4.0.0@sha256:fe3a43add8ff1db5cfffee7e7694c86128b1dfe62c541f26e25a8eadf9585610,1663
- retrie-1.1.0.0
- stylish-haskell-0.14.2.0@sha256:fffe1c13ad4c2678cf28a7470cac5d3bf20c71c36f09969e3e5f186787cceb7c,4321
- lsp-1.5.0.0
- lsp-types-1.5.0.0
- lsp-test-0.14.0.3
- git: [email protected]:haskell/lsp
commit: b0f8596887088b8ab65fc1015c773f45b47234ae
subdirs:
- lsp
- lsp-types
- lsp-test
- co-log-core-0.3.1.0

configure-options:
Expand Down
9 changes: 6 additions & 3 deletions stack.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,12 @@ extra-deps:
- hiedb-0.4.1.0@sha256:fb20c657d9ecc91701b00dffcf4bbd77cb83720a1f9d867badd77ea227973135,2875
- implicit-hie-0.1.2.7@sha256:82bbbb1a8c05f99c8af3c16ac53e80c8648d8bf047b25ed5ce45a135bd736907,3122
- implicit-hie-cradle-0.5.0.0@sha256:4276f60f3a59bc22df03fd918f73bca9f777de9568f85e3a8be8bd7566234a59,2368
- lsp-1.5.0.0
- lsp-test-0.14.0.3
- lsp-types-1.5.0.0
- git: [email protected]:haskell/lsp
commit: b0f8596887088b8ab65fc1015c773f45b47234ae
subdirs:
- lsp
- lsp-types
- lsp-test
- monad-dijkstra-0.1.1.3@sha256:d2fc098d7c122555e726830a12ae0423ac187f89de9228f32e56e2f6fc2238e1,1900
- retrie-1.2.0.1
- co-log-core-0.3.1.0
Expand Down