While the given approach for AtomicMap is interesting and performs well in the common case, it suffers from a few flaws: some fixable, some fundamental.
First, storing polymorphic values in the map (either true values or OneShotThunks, depending on whether the Thunk has been unboxed or not) means you have to give up the static type safety of the compiler and cast values carefully. There's at least two bugs in the current version of the code due to unsafe casting. However, this problem can be fixed with careful coding and thorough tests.
Second, the given code doesn't actually prevent thunks from executing more than once. If the thunk throws an exception when executed, the thunk will still be kept in the map and marked as "unexecuted", so the next thread that tries to read that value will execute it again (and possibly throw an exception again). This is troubling both because the thunk executes multiple times and because an update from one thread can cause another thread to throw an exception. This is possibly fixable with some clever coding, but it's unclear what the desired semantics should be if the thunk throws an exception. (Possibly tombstone the value or replace it with the previous value: as if the update with the throwing thunk had never happened.)
But a fundamental flaw in the entire approach is that AtomicMap imposes a certain kind of threading model on client code. The above-mentioned behavior of exceptions propagating to other threads is one symptom of this problem, but there are others. For example, if the thunk takes a while to execute, or worse yet blocks (either momentarily or indefinitely), then other threads interacting with the same key in the map will block waiting for the thunk to finish executing. This behavior is perhaps fine for many applications, but if you want your threads to operate with strict timeouts for blocking operations, or if you want your threads to be entirely nonblocking, then you can't safely use AtomicMap. The implementation forces a particular threading model on client code, which is deprived from choosing its own threading model.
Perhaps a better interface for AtomicMap to implement is ConcurrentMap[A, Future[B]]. This alternative map could still make all its operations atomic, but by returning a Future[B] (instead of a straight-up B), it gives client code the power to check whether an exception was thrown or to block with a timeout when getting the value. Then its up to the client to decide what threading model it feels most comfortable with.
Thanks for the comments, Jorge. What you describe as a fundamental flaw is perhaps better described as a careful engineering trade-off. AtomicMap is intended for specific use cases. If you are not dealing with an extremely high throughput, high concurrency write system, then AtomicMap may not be the right choice for you. If you are dealing with complicated write semantics that are likely to raise exceptions, AtomicMap may not be the right choice for you. In our application, we have a lot of threads writing at once. Exceptions in that path would indicate a pretty significant problem, far worse than how to handle them gracefully.
We have chosen to trade some flexibility in how the data structure can be used in exchange for an enormous increase in throughput. Your needs may differ, resulting in different trade offs.
First, storing polymorphic values in the map (either true values or OneShotThunks, depending on whether the Thunk has been unboxed or not) means you have to give up the static type safety of the compiler and cast values carefully. There's at least two bugs in the current version of the code due to unsafe casting. However, this problem can be fixed with careful coding and thorough tests.
Second, the given code doesn't actually prevent thunks from executing more than once. If the thunk throws an exception when executed, the thunk will still be kept in the map and marked as "unexecuted", so the next thread that tries to read that value will execute it again (and possibly throw an exception again). This is troubling both because the thunk executes multiple times and because an update from one thread can cause another thread to throw an exception. This is possibly fixable with some clever coding, but it's unclear what the desired semantics should be if the thunk throws an exception. (Possibly tombstone the value or replace it with the previous value: as if the update with the throwing thunk had never happened.)
But a fundamental flaw in the entire approach is that AtomicMap imposes a certain kind of threading model on client code. The above-mentioned behavior of exceptions propagating to other threads is one symptom of this problem, but there are others. For example, if the thunk takes a while to execute, or worse yet blocks (either momentarily or indefinitely), then other threads interacting with the same key in the map will block waiting for the thunk to finish executing. This behavior is perhaps fine for many applications, but if you want your threads to operate with strict timeouts for blocking operations, or if you want your threads to be entirely nonblocking, then you can't safely use AtomicMap. The implementation forces a particular threading model on client code, which is deprived from choosing its own threading model.
Perhaps a better interface for AtomicMap to implement is ConcurrentMap[A, Future[B]]. This alternative map could still make all its operations atomic, but by returning a Future[B] (instead of a straight-up B), it gives client code the power to check whether an exception was thrown or to block with a timeout when getting the value. Then its up to the client to decide what threading model it feels most comfortable with.