mdedetrich commented on PR #371: URL: https://github.com/apache/incubator-pekko/pull/371#issuecomment-1592940763
@alexandru Thanks for the reply So to provide some context on how, why and where the concurrency in this specific circumstance matters the code we are dealing with is entirely inside an `Actor` `receive` block which is actually normally meant to be guaranteed to be thread safe (i.e. single thread), the reason why we need to make `nextId()` thread safe is because there is a `recoverWith` on a `Future` which ends up transitively calling `nextId()` within an `ExecutionContext`. This `ExecutionContext` is configurable and hence parameterised, its using the Akka default dispatcher which iirc is a `ForkJoinPool`. I have strong suspicion that we should be optimising for our happy case, because as you can infer from the usage of `recoverWith` this is probably not going to happen that often so while the usage of `synchronized` is typically ill advised, as you pointed out it is heavily optimized/uses assumptions that mirror the case we are dealing with. The thing is (and this is a general statement and not strictly about this concurrency issue at hand), its hard for us to get some real data on this and there are many variables at play. For example there are actually 3 versions of @Claudenw algorithm, each with different guarantees of how we want to handle collisions. The "simple" version doesn't handle collisions at all which with initial impressions appears might be the best solution because its going to get handled by the duplicate id changes merged at https://github.com/apache/incubator-pekko/pull/385. However if we get lots of collisions/transactions then one of his variants which mitigate collisions might end up overall being faster. And while we have a concurrency issue at hand, at the same time we are dealing with random numbers here which means we don't care about "wrong" answers, only if we get duplicates that we can't detect which might open up for some interesting optimisations. Nonetheless we are trying to work out these concurrency issues and very much appreciate your experience here (if you have some extra time to spare let me know and we can co-ordinate). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
