Improve channel map operations in respect of thread-safety #655
Conversation
… call to the concurrent hash map.
|
@QuintinWillison could you please remove unused imports? |
|
Thanks, @vzhikserg ... that was rather sloppy of me! 🤦 |
| // Hence there's the slight inefficiency of creating newChannel when it may not be | ||
| // needed because there is an existingChannel. | ||
| final Channel newChannel = new Channel(AblyRealtime.this, channelName, channelOptions); | ||
| final Channel existingChannel = map.putIfAbsent(channelName, newChannel); |
There was a problem hiding this comment.
You can use map.get(channelName) for concurrentHashmap too right?
https://docs.oracle.com/javase/7/docs/api/java/util/concurrent/ConcurrentHashMap.html#get(java.lang.Object)
There was a problem hiding this comment.
I don't understand your point. That is the method that this refactor is pretty much focussed on avoiding the use of. I've replaced two calls (one to get, followed by a subsequent conditional call to put, with a single call to atomic putIfAbsent as backed by the concurrent hash map).
Perhaps you need to re-read the issue to see what we're aiming to solve here. 🤔
There was a problem hiding this comment.
Actually, I was looking at the comment slight inefficiency of creating newChannel, if we use map.get(channelName), we can avoid it. I feel it's a tradeoff, in most cases, the client will try to get an existing channel rather than creating a new one.
Fixes #649.