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

Generalized mlocking via MonadST #404

Merged
merged 6 commits into from
Jun 27, 2023
Merged

Generalized mlocking via MonadST #404

merged 6 commits into from
Jun 27, 2023

Conversation

tdammers
Copy link
Contributor

@tdammers tdammers commented May 1, 2023

This is an alternative approach to generalizing mlocking allocations over "arbitrary" monads (specifically, we want IO and IOSim).

#388 introduces a MonadMLock typeclass, in much the same way as io-classes provides MonadMVar, MonadDelay, etc.; this PR takes a different approach, based on the observation that all the mlocked allocation and raw memory manipulation APIs, while technically in IO, are morally in ST - their only effects are memory manipulations, there's no I/O going on, no randomness, etc. So we provide versions of all those primitives and FFI functions in ST (or rather, with a MonadST constraint), which automatically makes them available in IO and IOSim.

One complication of this is that we cannot use the MonadMLock typeclass to inject mlock allocation logging, like we did in #388; we solve this by changing the relevant APIs such that they take an explicit MLockedAllocator argument, and API-backards-compatible convenience wrappers that default the allocator to mlockedMalloc. And now we can implement mlocked allocation logging by passing a "decorated" allocator (which not only allocates, but also logs the allocation, and attaches deallocation logging code to the finalizer), while using the compatible wrappers outside of testing.

@tdammers tdammers marked this pull request as ready for review May 11, 2023 08:16
Copy link
Collaborator

@lehins lehins left a comment

Choose a reason for hiding this comment

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

This looks great. 👍

cardano-crypto-class/src/Cardano/Crypto/EqST.hs Outdated Show resolved Hide resolved
@tdammers tdammers force-pushed the tdammers/just-use-monad-st branch from 1c1fdfb to 7374962 Compare May 22, 2023 12:44
Copy link
Collaborator

@lehins lehins left a comment

Choose a reason for hiding this comment

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

Found one problem, but other than that I think it is ready to be merged

@tdammers tdammers force-pushed the tdammers/just-use-monad-st branch 2 times, most recently from 20f2fe3 to 1e67222 Compare June 15, 2023 07:26
Changes:

- Remove MonadSodium typeclass
- Express mlocked memory operations in terms of MonadST
- Get rid of ForgetMock testing (has been moot since we no longer depend
  on GC to reclaim mlocked memory)
- Remove `m` parameter from mlocked memory based typeclasses (KES,
  DSIGN)
- Simplify KES and DSIGN typeclass hierarchies
Copy link
Collaborator

@lehins lehins left a comment

Choose a reason for hiding this comment

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

Looks great!!! Thank you for doing the work

@lehins lehins merged commit cb31fe9 into master Jun 27, 2023
@lehins lehins deleted the tdammers/just-use-monad-st branch June 27, 2023 10:25
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.

2 participants