-
Notifications
You must be signed in to change notification settings - Fork 560
Add AtomicMap
#4424
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
Add AtomicMap
#4424
Conversation
488f0c7 to
81a8031
Compare
| // Provides common implementations for derived methods that depend on F being an applicative. | ||
| private[effect] sealed abstract class CommonImpl[F[_], A]( | ||
| implicit F: Applicative[F] | ||
| ) extends AtomicCell[F, A] { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally, the implementations here would be part of the trait itself.
But since the trait doesn't know that there will be an Applicative[F] in scope, we can't provide them there.
Name suggestions are welcome :)
| /** | ||
| * Allows seeing a `AtomicCell[F, Option[A]]` as a `AtomicCell[F, A]`. This is useful not only | ||
| * for ergonomic reasons, but because some implementations may save space. | ||
| * | ||
| * Setting the `default` value is the same as storing a `None` in the underlying `AtomicCell`. | ||
| */ | ||
| def defaultedAtomicCell[F[_], A]( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inspired by MapRef.defaultedRef
| ): AtomicCellOptionOps[F, A] = | ||
| new AtomicCellOptionOps(atomicCell) | ||
|
|
||
| final class AtomicCellOptionOps[F[_], A] private[effect] ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure how useful these extensions are, but it felt good to have them for completeness of the API.
| new AtomicCell.ConcurrentImpl( | ||
| ref = valuesMapRef(key), | ||
| lock = keyedMutex.lock(key) | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We get the implementation by free thanks to the shapes of KeyedMutex and MapRef.
And, since the default implementation for the MapRef when F is actually a Sync uses a ConcurrentHashMap, this should be pretty efficient under the hood.
| implicit F: Applicative[F] | ||
| ) extends AtomicMap[F, K, V] { | ||
| override def apply(key: K): AtomicCell[F, V] = | ||
| AtomicCell.defaultedAtomicCell(atomicCell = atomicMap(key), default) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also delegates to the AtomicCell implementation.
| ): AtomicMapOptionOps[F, K, V] = | ||
| new AtomicMapOptionOps(atomicMap) | ||
|
|
||
| final class AtomicMapOptionOps[F[_], K, V] private[effect] ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The delegation here requires a little bit more boilerplate.
7979479 to
33ac93d
Compare
3b5f75d to
a5c0ca8
Compare
Co-authored-by: Yannick Heiber <yhe@famly.co>
a9e1329 to
d81546f
Compare
| // #4424, false warnings in CommonImpl due to lightbend-labs/mima#211 | ||
| ProblemFilters.exclude[DirectAbstractMethodProblem]( | ||
| "cats.effect.std.AtomicCell.modify"), | ||
| ProblemFilters.exclude[DirectAbstractMethodProblem]( | ||
| "cats.effect.std.AtomicCell.evalUpdate"), | ||
| ProblemFilters.exclude[DirectAbstractMethodProblem]( | ||
| "cats.effect.std.AtomicCell.evalGetAndUpdate"), | ||
| ProblemFilters.exclude[DirectAbstractMethodProblem]( | ||
| "cats.effect.std.AtomicCell.evalUpdateAndGet") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If lightbend-labs/mima#211 is ever fixed we should revert these.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is pretty reasonable and useful. :) Normally we don't like landing new features post RC, but this one is pretty limited in scope so I think it's fine.
On top of #4065
Provides a parallel of
MapRefforAtomicCell, theAtomicMap.It also includes some nice improvements for
AtomicCell, we may move these to its own PR if you prefer.