Skip to content

Lockless diagnostics #2434

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 6 commits into from
Dec 5, 2021
Merged

Lockless diagnostics #2434

merged 6 commits into from
Dec 5, 2021

Conversation

pepeiborra
Copy link
Collaborator

@pepeiborra pepeiborra commented Dec 3, 2021

Fourth instalment of #2357

@jneira jneira requested a review from michaelpj December 3, 2021 12:13
-> STM (IO ())
recordDirtyKeys ShakeExtras{dirtyKeys} key file = do
modifyTVar' dirtyKeys $ \x -> foldl' (flip HSet.insert) x (toKey key <$> file)
return $ withEventTrace "recordDirtyKeys" $ \addEvent -> do
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm interesting. I guess we may end up with this happening a bit if we push STM out further and we hit more places where we want to do tracing. I wonder if this is a pattern that appears elsewhere: this is something like a "final" IO action that should be run only after the STM action succeeds.

Stderr output is very useful to diagnose test failures when interleaved with the LSP log. Sending it to a file disrupts this interleaving and makes it harder to retrieve from CI
@pepeiborra pepeiborra force-pushed the lockless-diagnostics branch from b802fcb to 43220a0 Compare December 5, 2021 13:09
@pepeiborra pepeiborra merged commit 84ece63 into master Dec 5, 2021
Comment on lines 248 to +250
let invalidateShakeCache = do
void $ modifyVar' version succ
recordDirtyKeys extras GhcSessionIO [emptyFilePath]
atomically $ recordDirtyKeys extras GhcSessionIO [emptyFilePath]
Copy link
Collaborator

Choose a reason for hiding this comment

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

I just noticed, this changes the type of invalidateShakeCache to :: IO (IO ()). I suppose that is unintentional?

Copy link
Collaborator

@fendor fendor Dec 9, 2021

Choose a reason for hiding this comment

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

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants