Skip to content

Conversation

@cfieber
Copy link

@cfieber cfieber commented Nov 24, 2014

fix usage of concurrent map for gauge lock creation

fix usage of concurrent map for gauge lock creation
@wilkinsona
Copy link
Member

Thanks for the PR.

Is there any reason why you've taken your approach as opposed to something like this:

Object lock = this.gaugeLocks.get(name);
if (lock == null) {
    Object newLock = new Object();
    lock = this.gaugeLocks.putIfAbsent(name, newLock);
    if (lock == null) {
        lock = newLock;
    }
 }

The above, I think, will perform better where there's an existing lock as there's a single call to get() and that's it. Notably, it avoids the creation of newLock unless there's a good chance it'll be used. In the case where there isn't an existing lock the above is probably slightly more expensive as there are two interactions with the map. Optimising for the case where the lock already exists feels like the right approach to me. Do you have experience to the contrary?

@wilkinsona wilkinsona added the status: waiting-for-feedback We need additional information before we can continue label Nov 25, 2014
@philwebb philwebb modified the milestones: 1.2.0, 1.1.10 Dec 1, 2014
@philwebb philwebb added the type: bug A general bug label Dec 1, 2014
@philwebb philwebb closed this in 49858a0 Dec 1, 2014
@philwebb philwebb removed the status: waiting-for-feedback We need additional information before we can continue label Dec 1, 2014
philwebb added a commit that referenced this pull request Dec 1, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: bug A general bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants