-
-
Notifications
You must be signed in to change notification settings - Fork 388
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.
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
Fix quadratic memory usage in GetLocatedImports #4318
Merged
Merged
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
d143131
to
ac33075
Compare
fendor
approved these changes
Jun 13, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Please write down this subtle reasoning in the code! Otherwise we will be unlikely to retain this property over time. |
At startup `GetLocatedImports` is called on all known files. Say you have 10000 modules in your project then this leads to 10000 calls to GetLocatedImports running concurrently. In `GetLocatedImports` the known targets are consulted and the targetsMap is created by mapping the known targets. This map is used for introducing sharing amongst filepaths. This operation copies a local copy of the `target` map which is local to the rule. ``` let targetsMap = HMap.mapWithKey const targets ``` So now each rule has a hashmap of size 10000 held locally to it and depending on how the threads are scheduled there will be 10000^2 elements in total allocated in hashmaps. This used a lot of memory. Solution: Return the normalising map in the result of the `GetKnownTargets` rule so it is shared across threads. Fixes #4317
ac33075
to
b704b59
Compare
I added the comment to the documentation of 'normalisingMap'. |
michaelpj
approved these changes
Jun 13, 2024
netbsd-srcmastr
pushed a commit
to NetBSD/pkgsrc
that referenced
this pull request
Jan 31, 2025
## 2.9.0.0 - Bindists for GHC 9.10.1 by @wz1000, @jhrcek, @michaelpj - More hls-graph reliability improvements by @soulomoon - Refactoring of test suite runners by @soulomoon - Fixes in multiple home units support by @wz1000 ### Pull Requests - Fix quadratic memory usage in GetLocatedImports ([#4318](haskell/haskell-language-server#4318)) by @mpickering - Bump stack configs + CI to 9.6.5 and 9.8.2 ([#4316](haskell/haskell-language-server#4316)) by @jhrcek - Add support for Fourmolu 0.16 ([#4314](haskell/haskell-language-server#4314)) by @ brandonchinn178 - Code action to remove redundant record field import (fixes #4220) ([#4308](haskell/haskell-language-server#4308)) by @battermann - Use restricted monad for plugins (#4057) ([#4304](haskell/haskell-language-server#4304)) by @awjchen - 4301 we need to implement utility to wait for all runnning keys in hls graph done ([#4302](haskell/haskell-language-server#4302)) by @soulomoon - Call useWithStale instead of useWithStaleFast when calling ParseCabalFields ([#4294](haskell/haskell-language-server#4294)) by @VeryMilkyJoe - test: add test documenting #806 ([#4292](haskell/haskell-language-server#4292)) by @develop7 - ghcide: drop ghc-check and ghc-paths dependency ([#4291](haskell/haskell-language-server#4291)) by @wz1000 - Limit number of valid hole fits to 10 ([#4288](haskell/haskell-language-server#4288)) by @akshaymankar - Add common stanza to completion data ([#4286](haskell/haskell-language-server#4286)) by @VeryMilkyJoe - FindImports: ThisPkg means some home unit, not "this" unit ([#4284](haskell/haskell-language-server#4284)) by @wz1000 - Remove redudant absolutization in session loader ([#4280](haskell/haskell-language-server#4280)) by @soulomoon - Bump to new lsp versions ([#4279](haskell/haskell-language-server#4279)) by @michaelpj - Put more test code into pre-commit ([#4275](haskell/haskell-language-server#4275)) by @soulomoon - Delete library ghcide test utils ([#4274](haskell/haskell-language-server#4274)) by @soulomoon - Delete testUtil from ghcide-tests ([#4272](haskell/haskell-language-server#4272)) by @soulomoon - CI change, only run bench on performance label ([#4271](haskell/haskell-language-server#4271)) by @soulomoon - Migrate WatchedFileTests ([#4269](haskell/haskell-language-server#4269)) by @soulomoon - Migrate UnitTests ([#4268](haskell/haskell-language-server#4268)) by @soulomoon - Migrate SafeTests ([#4267](haskell/haskell-language-server#4267)) by @soulomoon - Migrate SymlinkTests ([#4266](haskell/haskell-language-server#4266)) by @soulomoon - Remove unused and outdated CHANGELOG files ([#4264](haskell/haskell-language-server#4264)) by @fendor - Enable cabal flaky test ([#4263](haskell/haskell-language-server#4263)) by @soulomoon - Migrate RootUriTests ([#4261](haskell/haskell-language-server#4261)) by @soulomoon - Migrate PreprocessorTests ([#4260](haskell/haskell-language-server#4260)) by @soulomoon - Migrate PluginSimpleTests ([#4259](haskell/haskell-language-server#4259)) by @soulomoon - Migrate ClientSettingsTests ([#4258](haskell/haskell-language-server#4258)) by @soulomoon - Unify critical session running in hls ([#4256](haskell/haskell-language-server#4256)) by @soulomoon - Bump cachix/cachix-action from 14 to 15 ([#4255](haskell/haskell-language-server#4255)) by @dependabot[bot] - Bump haskell-actions/setup from 2.7.2 to 2.7.3 ([#4254](haskell/haskell-language-server#4254)) by @dependabot[bot] - Bump haskell-actions/setup from 2.7.2 to 2.7.3 in /.github/actions/setup-build ([#4253](haskell/haskell-language-server#4253)) by @dependabot[bot] - Shorter file names completion ([#4252](haskell/haskell-language-server#4252)) by @VenInf - Fix progress start delay ([#4249](haskell/haskell-language-server#4249)) by @michaelpj - Bump cachix/install-nix-action from 26 to 27 ([#4245](haskell/haskell-language-server#4245)) by @dependabot[bot] - Bump haskell-actions/setup from 2.7.1 to 2.7.2 ([#4244](haskell/haskell-language-server#4244)) by @dependabot[bot] - Bump haskell-actions/setup from 2.7.1 to 2.7.2 in /.github/actions/setup-build ([#4243](haskell/haskell-language-server#4243)) by @dependabot[bot] - Enable test for #717 ([#4241](haskell/haskell-language-server#4241)) by @soulomoon - Remove Pepe from CODEOWNERS ([#4239](haskell/haskell-language-server#4239)) by @michaelpj - Fix resultBuilt(dirty mechanism) in hls-graph ([#4238](haskell/haskell-language-server#4238)) by @soulomoon - Support for 9.10 ([#4233](haskell/haskell-language-server#4233)) by @wz1000 - Refactor hls-test-util and reduce getCurrentDirectory after initilization ([#4231](haskell/haskell-language-server#4231)) by @soulomoon - [Migrate BootTests] part of #4173 Migrate ghcide tests to hls test utils ([#4227](haskell/haskell-language-server#4227)) by @soulomoon - Actually enable pedantic flag in ci flags job ([#4224](haskell/haskell-language-server#4224)) by @jhrcek - Cleanup cabal files, ghc compat code, fix ghc warnings ([#4222](haskell/haskell-language-server#4222)) by @jhrcek - Another attempt at using the lsp API for some progress reporting ([#4218](haskell/haskell-language-server#4218)) by @michaelpj - [Migrate diagnosticTests] part of #4173 Migrate ghcide tests to hls test utils ([#4207](haskell/haskell-language-server#4207)) by @soulomoon - Prepare release 2.8.0.0 ([#4191](haskell/haskell-language-server#4191)) by @wz1000 - Stabilize the build system by correctly house keeping the dirtykeys and rule values [flaky test #4185 #4093] ([#4190](haskell/haskell-language-server#4190)) by @soulomoon - hls-cabal-plugin: refactor context search to use `readFields` ([#4186](haskell/haskell-language-server#4186)) by @fendor - 3944 extend the properties api to better support nested configuration ([#3952](haskell/haskell-language-server#3952)) by @soulomoon ## 2.8.0.0 - Bindists for GHC 9.6.5 - New hls-notes plugin (#4126, @jvanbruegge) - Floskell, hlint and stylish-haskell plugins enabled for GHC 9.8 - Improvements for hls-graph increasing robustness (#4087, @soulomoon) - Improvements to multi-component support (#4096, #4109, #4179, @wz1000, @fendor) ### Pull Requests - Bump haskell-actions/setup from 2.7.0 to 2.7.1 ([#4189](haskell/haskell-language-server#4189)) by @dependabot[bot] - Bump haskell-actions/setup from 2.7.0 to 2.7.1 in /.github/actions/setup-build ([#4188](haskell/haskell-language-server#4188)) by @dependabot[bot] - Fix ghcdie-tests CI ([#4184](haskell/haskell-language-server#4184)) by @soulomoon - Fix ghc and hlint warnings, fix formatting ([#4181](haskell/haskell-language-server#4181)) by @jhrcek - Allow users to specify whether to use `cabal`'s multi-repl feature ([#4179](haskell/haskell-language-server#4179)) by @fendor - Improve parsing of import suggestions extending multiple multiline imports (fixes #4175) ([#4177](haskell/haskell-language-server#4177)) by @jhrcek - move ghcide-tests to haskell-language-server.cabal and make it depend on hls-test-utils ([#4176](haskell/haskell-language-server#4176)) by @soulomoon - enable ThreadId for when testing ([#4174](haskell/haskell-language-server#4174)) by @soulomoon - Drop Legacy Logger from Codebase ([#4171](haskell/haskell-language-server#4171)) by @fendor - get rid of the `unsafeInterleaveIO` at start up ([#4167](haskell/haskell-language-server#4167)) by @soulomoon - Remove EKG ([#4163](haskell/haskell-language-server#4163)) by @michaelpj - Mark plugins as not buildable if the flag is disabled ([#4160](haskell/haskell-language-server#4160)) by @michaelpj - Fix references to old CPP names in tests, update tests ([#4159](haskell/haskell-language-server#4159)) by @jhrcek - Bump haskell-actions/setup from 2.6.3 to 2.7.0 ([#4158](haskell/haskell-language-server#4158)) by @dependabot[bot] - Bump haskell-actions/setup from 2.6.3 to 2.7.0 in /.github/actions/setup-build ([#4157](haskell/haskell-language-server#4157)) by @dependabot[bot] - Remove dead code in ghcide and hls-graph for priority ([#4151](haskell/haskell-language-server#4151)) by @soulomoon - Bump haskell-actions/setup from 2.6.2 to 2.6.3 in /.github/actions/setup-build ([#4150](haskell/haskell-language-server#4150)) by @dependabot[bot] - Bump haskell-actions/setup from 2.6.2 to 2.6.3 ([#4149](haskell/haskell-language-server#4149)) by @dependabot[bot] - Run ExceptionTests in temporary directory ([#4146](haskell/haskell-language-server#4146)) by @fendor - hls-eval-plugin: Replicate #4139 ([#4140](haskell/haskell-language-server#4140)) by @mattapet - Update comment in refactor tests ([#4138](haskell/haskell-language-server#4138)) by @jhrcek - Update contact info in docs ([#4137](haskell/haskell-language-server#4137)) by @jhrcek - hls-notes-plugin: Do not error if no note is under the cursor ([#4136](haskell/haskell-language-server#4136)) by @jvanbruegge - improve logging in semantic tokens rule ([#4135](haskell/haskell-language-server#4135)) by @soulomoon - Bump softprops/action-gh-release from 1 to 2 ([#4133](haskell/haskell-language-server#4133)) by @dependabot[bot] - Bump cachix/install-nix-action from 25 to 26 ([#4132](haskell/haskell-language-server#4132)) by @dependabot[bot] - Use Set.member instead of Foldable.elem ([#4128](haskell/haskell-language-server#4128)) by @jhrcek - hls-notes-plugin: Initial implementation ([#4126](haskell/haskell-language-server#4126)) by @jvanbruegge - Enable floskell and hlint plugins for ghc 9.8 ([#4125](haskell/haskell-language-server#4125)) by @jhrcek - Integrate stylish-haskell into hls executable with ghc 9.8 ([#4124](haskell/haskell-language-server#4124)) by @jhrcek - Reduce usage of partial functions ([#4123](haskell/haskell-language-server#4123)) by @jhrcek - Benchmark: Enable 9.6, 9.8 ([#4118](haskell/haskell-language-server#4118)) by @soulomoon - Bump haskell-actions/setup from 2.6.1 to 2.6.2 in /.github/actions/setup-build ([#4116](haskell/haskell-language-server#4116)) by @dependabot[bot] - Bump haskell-actions/setup from 2.6.1 to 2.6.2 ([#4115](haskell/haskell-language-server#4115)) by @dependabot[bot] - eval: more robust way to extract comments from ParsedModule ([#4113](haskell/haskell-language-server#4113)) by @jhrcek - Improve isolation of build artefacts of test runs ([#4112](haskell/haskell-language-server#4112)) by @fendor - Improve handling of nonsense rename attempts ([#4111](haskell/haskell-language-server#4111)) by @jhrcek - Exit with non-zero exitcode if wrapper fails to launch ([#4110](haskell/haskell-language-server#4110)) by @fendor - Replace checkHomeUnitsClosed with a faster implementation ([#4109](haskell/haskell-language-server#4109)) by @wz1000 - Don't distribute gifs or plugin readmes ([#4107](haskell/haskell-language-server#4107)) by @fendor - Remove locale workaround for Module name that conatins non-ascii characters ([#4106](haskell/haskell-language-server#4106)) by @fendor - Track extra-source-files of plugins more accurately ([#4105](haskell/haskell-language-server#4105)) by @fendor - remove non-ascii name ([#4103](haskell/haskell-language-server#4103)) by @soulomoon - Add cabal-gild as a cabal file formatter plugin ([#4101](haskell/haskell-language-server#4101)) by @fendor - Remove more workarounds for GHCs < 9.2 (#4092) ([#4098](haskell/haskell-language-server#4098)) by @jhrcek - session-loader: Don't loop forever when we don't find a file in any multi component ([#4096](haskell/haskell-language-server#4096)) by @wz1000 - Prepare release 2.7.0.0 ([#4095](haskell/haskell-language-server#4095)) by @fendor - Remove more workarounds for GHCs < 9.0 ([#4092](haskell/haskell-language-server#4092)) by @jhrcek - Fix hls-graph: phantom dependencies invoke in branching deps (resolve #3423) ([#4087](haskell/haskell-language-server#4087)) by @soulomoon - Rename only if the current module compiles (#3799) ([#3848](haskell/haskell-language-server#3848)) by @sgillespie - Reintroduce ghc-lib flag for hlint plugin ([#3757](haskell/haskell-language-server#3757)) by @RaoulHC
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
At startup
GetLocatedImports
is called on all known files. Say you have 10000 modules in your project then this leads to 10000 calls to GetLocatedImports running concurrently.In
GetLocatedImports
the known targets are consulted and the targetsMap is created by mapping the known targets. This map is used for introducing sharing amongst filepaths. This operation copies a local copy of thetarget
map which is local to the rule.So now each rule has a hashmap of size 10000 held locally to it and depending on how the threads are scheduled there will be 10000^2 elements in total allocated in hashmaps. This used a lot of memory.
Solution: Return the normalising map in the result of the
GetKnownTargets
rule so it is shared across threads.Fixes #4317