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

Two recompilation avoidance related bug fixes #3452

Merged
merged 2 commits into from
Jan 26, 2023
Merged

Conversation

wz1000
Copy link
Collaborator

@wz1000 wz1000 commented Jan 13, 2023

  1. Recompilation avoidance regresses in GHC 9.4 due to interactions between GHC and HLS's implementations.
    Avoid this by filtering out the information that causes the conflict
    See Recompilation avoidance in GHC 9.4 conflicts with recompilation avoidance in HLS #3450.

  2. The recompilation avoidance info GHC stores in interfaces can blow up to be
    extremely large when deserialised from disk. See https://gitlab.haskell.org/ghc/ghc/-/issues/22744
    Deduplicate these filepaths.

Copy link
Collaborator

@fendor fendor left a comment

Choose a reason for hiding this comment

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

Can't really judge the change itself, but one comment

ghcide/src/Development/IDE/Core/Compile.hs Outdated Show resolved Hide resolved
@michaelpj
Copy link
Collaborator

Do we definitely want to include the mitigation for issue 2? would it be better to just fix in GHC and let it be in the next minor 9.4 release?

@wz1000
Copy link
Collaborator Author

wz1000 commented Jan 13, 2023

Do we definitely want to include the mitigation for issue 2? would it be better to just fix in GHC and let it be in the next minor 9.4 release?

It is unlikely to be fixed in a minor GHC release because the simplest way to perform the fix involves an API change.

@michaelpj
Copy link
Collaborator

That's a shame :(

filePathMap = unsafePerformIO $ newIORef HashMap.empty
{-# NOINLINE filePathMap #-}

shareFilePath :: FilePath -> FilePath
Copy link
Collaborator

Choose a reason for hiding this comment

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

This and shareUsages are actually called in IO, so we could just have them be in IO and have slightly less unsafety.

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 moved shareFilePath to a different module, and I think it makes sense for it to be pure so that it can be used in other places in the future.

@guibou
Copy link
Collaborator

guibou commented Jan 21, 2023

I've tried this MR and I can confirm that HLS is way faster and seems to not rebuild multiple files at each changes.

However, some files are failing with:

No match in record selector usg_file_path

Because usg_file_path appears in this MR, looks like it is related.

And any change in any file which appears in my workspace error log with such error does not seem to be taken into account anymore in files which are depending on them.

@wz1000
Copy link
Collaborator Author

wz1000 commented Jan 23, 2023

However, some files are failing with:

No match in record selector usg_file_path

Because usg_file_path appears in this MR, looks like it is related.

And any change in any file which appears in my workspace error log with such error does not seem to be taken into account anymore in files which are depending on them.

Thanks, I've fixed this.

@guibou
Copy link
Collaborator

guibou commented Jan 23, 2023

Thanks, I've fixed this.

Thank you. I've tested and it is fast like it had never been before AND it does not fail like described above.

I'm pushing the fix in production tomorrow, I'll tell you if developers are happy or grumpy ;)

1. Recompilation avoidance regresses in GHC 9.4 due to interactions between GHC and HLS's implementations.
   Avoid this by filtering out the information that causes the conflict
   See https://gitlab.haskell.org/ghc/ghc/-/issues/22744.

2. The recompilation avoidance info GHC stores in interfaces can blow up to be
   extremely large when deserialised from disk. See https://gitlab.haskell.org/ghc/ghc/-/issues/22744
   Deduplicate these filepaths.
@wz1000 wz1000 added the merge me Label to trigger pull request merge label Jan 26, 2023
@mergify mergify bot merged commit 00f4e61 into master Jan 26, 2023
wz1000 added a commit that referenced this pull request Feb 9, 2023
1. Recompilation avoidance regresses in GHC 9.4 due to interactions between GHC and HLS's implementations.
   Avoid this by filtering out the information that causes the conflict
   See https://gitlab.haskell.org/ghc/ghc/-/issues/22744.

2. The recompilation avoidance info GHC stores in interfaces can blow up to be
   extremely large when deserialised from disk. See https://gitlab.haskell.org/ghc/ghc/-/issues/22744
   Deduplicate these filepaths.

(cherry picked from commit 00f4e61)
wz1000 added a commit that referenced this pull request Mar 14, 2023
1. Recompilation avoidance regresses in GHC 9.4 due to interactions between GHC and HLS's implementations.
   Avoid this by filtering out the information that causes the conflict
   See https://gitlab.haskell.org/ghc/ghc/-/issues/22744.

2. The recompilation avoidance info GHC stores in interfaces can blow up to be
   extremely large when deserialised from disk. See https://gitlab.haskell.org/ghc/ghc/-/issues/22744
   Deduplicate these filepaths.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge me Label to trigger pull request merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants