load modules concurrently #4545 #4549
Conversation
|
@f-f anything blocking merge here? this change looks lovely |
|
We'll need another review from the core team - could anyone take a look? |
rhendric
left a comment
There was a problem hiding this comment.
Nits aside, I'm surprised that this is reported to decrease memory use because my understanding is that mapConcurrently will spawn every one of these tasks at once. Has that been confirmed on very large projects? What about consumption of other resources—open file handles or something?
I'd be more comfortable with this if we used primitives like pooledMapConcurrently instead. OTOH, I don't do much maintenance of the Ide modules so if the team members who do are happy with this I won't block it.
| import Language.PureScript.Ide.Usage (findUsages) | ||
| import System.Directory (getCurrentDirectory, getDirectoryContents, doesDirectoryExist, doesFileExist) | ||
| import System.FilePath ((</>), normalise) | ||
| import Control.Concurrent.Async.Lifted (mapConcurrently, mapConcurrently_) |
There was a problem hiding this comment.
Nit: insert into the imports in alphabetical order, please.
| import Control.Concurrent.Async.Lifted (mapConcurrently) | ||
| import Control.Monad.Trans.Control (MonadBaseControl) |
| import Language.PureScript qualified as P | ||
| import Language.PureScript.Errors.JSON qualified as P | ||
| import Language.PureScript.Ide.Filter.Declaration (DeclarationType(..)) | ||
| import Control.Monad.Trans.Control (MonadBaseControl) |
|
Also, I think this does merit its own changelog entry. |
Description of the change
On large projects loading the modules concurrently lowers memory usage substantially. Should help address #4545
Checklist:
Relevant checklist points addressed in #4546