Skip to content

Conversation

BalmungSan
Copy link
Contributor

@BalmungSan BalmungSan commented Jun 17, 2025

On top of #4065

Provides a parallel of MapRef for AtomicCell, the AtomicMap.
It also includes some nice improvements for AtomicCell, we may move these to its own PR if you prefer.

@BalmungSan BalmungSan force-pushed the add-atomic-map branch 3 times, most recently from 488f0c7 to 81a8031 Compare June 18, 2025 03:42
Comment on lines +172 to +175
// 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] {
Copy link
Contributor Author

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 :)

Comment on lines +242 to +248
/**
* 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](
Copy link
Contributor Author

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] (
Copy link
Contributor Author

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.

Comment on lines +61 to +64
new AtomicCell.ConcurrentImpl(
ref = valuesMapRef(key),
lock = keyedMutex.lock(key)
)
Copy link
Contributor Author

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)
Copy link
Contributor Author

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] (
Copy link
Contributor Author

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.

Comment on lines +1158 to +1166
// #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")
Copy link
Contributor Author

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.

Copy link
Member

@djspiewak djspiewak left a 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.

@djspiewak djspiewak merged commit 083a635 into typelevel:series/3.x Sep 1, 2025
47 of 71 checks passed
@BalmungSan BalmungSan deleted the add-atomic-map branch September 1, 2025 15:09
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.

3 participants