Optimize Ref.Atomic #9500
Optimize Ref.Atomic #9500
Conversation
| current | ||
| } | ||
| def getAndSet(a: A)(implicit unsafe: Unsafe): A = | ||
| ref.asInstanceOf[AtomicReference[A]].getAndSet(a) |
There was a problem hiding this comment.
unfortunately the asInstanceOf is necessary as the implicit unsafe will cause the compiler to choose UnsafeAPI#method.
| import zio.test._ | ||
|
|
||
| object RefSpec extends ZIOBaseSpec { | ||
|
|
||
| def spec = suite("RefSpec")( | ||
| suite("Atomic")( | ||
| test("race") { |
There was a problem hiding this comment.
Was race once implemented with Ref?
|
|
|
|
||
| def modify[B](f: A => (B, A))(implicit unsafe: Unsafe): B = { | ||
| var loop = true | ||
| var b: B = null.asInstanceOf[B] | ||
| while (loop) { | ||
| val current = value.get | ||
| val current = ref.asInstanceOf[AtomicReference[A]].get |
There was a problem hiding this comment.
could this whole fn be written as:
def modify(f) =
var b = null
ref.updateAndGet: a =>
tup = f(a)
b = tup._1
tup._2
b
?
There was a problem hiding this comment.
This implementation is more efficient. In fact one would argue the most efficient would be to use weakCompareAndSet but I think we can leave that one as is for now
How about adding a method like this, would it still cause mima to fail? I think even if it does it might be better to have it as there is a small chance that we're accessing the underlying AtomicReference in some zio-* library for performance reasons: @deprecated(...)
private[zio] def value: AtomicReference[A] = unsafe.asInstanceOf[AtomicReference[A]] |
| @@ -41,7 +41,8 @@ object ThreadLocalBridge { | |||
|
|
|||
| private class FiberRefTrackingSupervisor extends Supervisor[Unit] { | |||
|
|
|||
| private val trackedRefs: Ref.Atomic[Set[(FiberRef[_], Any => Unit)]] = Ref.Atomic(new AtomicReference(Set.empty)) | |||
There was a problem hiding this comment.
Hopefully we were not using the apply method like this in other zio-* libraries 😰
There was a problem hiding this comment.
I could introduce an apply method that gets the initial state from the AtomicRef. Is that preferable?
UnsafeAPIwith theAtomicRefclass to avoid pointer chasing withinUnsafeAPImethods, as well as allocated spaceAtomicRefwhere possible - reducing generated bytecode sizeBenchmark shows minor improvement, but it's just using
update:https://jmh.morethan.io/?gists=a2e47cdd533e03f456ad1210fefd7172,07a439a09644cedac645fedaf3fe8de6
I don't have time to add other benchmarks now, but I suspect this will be an improvement all around.
Other notes:
RefandSubscriptionRefas sealed, which should be binary compatible....getintoString. I think it's unnecessary and not suspended, so probably not worth exposing.