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.

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

Unload once per linkable instead of once per splice #3390

Merged
merged 2 commits into from Dec 16, 2022
Merged

Conversation

wz1000
Copy link
Collaborator

@wz1000 wz1000 commented Dec 9, 2022

We need to unload old linkables before we can load in new linkables. However,
the unload function in the GHC API takes a list of linkables to keep (i.e.
not unload). Earlier we unloaded right before loading in new linkables, which
is effectively once per splice. This can be slow as unload needs to walk over
the list of all loaded linkables, for each splice.

Instead, now we unload old linkables right after we generate a new linkable and
just before returning it to be loaded. This has a substantial effect on recompile
times as the number of loaded modules and splices increases.

Copy link
Collaborator

@michaelpj michaelpj left a comment

Choose a reason for hiding this comment

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

Just my usual plea for documentation.

@@ -160,8 +160,7 @@ computePackageDeps env pkg = do

data TypecheckHelpers
= TypecheckHelpers
{ getLinkablesToKeep :: !(IO (ModuleEnv UTCTime))
, getLinkables :: !([NormalizedFilePath] -> IO [LinkableResult])
{ getLinkables :: !([NormalizedFilePath] -> IO [LinkableResult])
Copy link
Collaborator

Choose a reason for hiding this comment

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

haddock?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

added.

@@ -1119,7 +1118,10 @@ getLinkableRule recorder =
-- Record the linkable so we know not to unload it
whenJust (hm_linkable =<< hmi) $ \(LM time mod _) -> do
compiledLinkables <- getCompiledLinkables <$> getIdeGlobalAction
liftIO $ void $ modifyVar' compiledLinkables $ \old -> extendModuleEnv old mod time
liftIO $ modifyVar compiledLinkables $ \old -> do
let !to_keep = extendModuleEnv old mod time
Copy link
Collaborator

Choose a reason for hiding this comment

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

Surely some of the explanation you put in the PR description should be written in the code for future people?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added.

Copy link
Collaborator

Choose a reason for hiding this comment

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

where?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sorry, forgot to include the file. I've updated it now.

@wz1000 wz1000 force-pushed the wip/fast-unload branch 2 times, most recently from 1cb8c91 to 41fd786 Compare December 15, 2022 12:34
@wz1000 wz1000 merged commit 13af254 into master Dec 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants