I would like to understand which approach is correct/better performant, using synchronized method or synchronized on specific resource? My understanding is that acquiring lock on specific object for which the concurrent modification needs to be avoided will be faster as all the other threads which are trying to modify other objects are free to proceed.
Example below code is synchronized on this for "createSession" method
class Solution {
public String getSessionKey(int user) {
if (validateSessionKey(user)) {
return someConcurrentHashMap.get(user).getKey();
}
throw new InvalidSessionException(SESSION_NOT_FOUND);
}
public synchronized String createSession(int user) {
if (validateSessionKey(user)) {
return someConcurrentHashMap.get(user).getKey();
}
UserSession session = new UserSession(user, generateSessionId(), LocalDateTime.now(ZoneOffset.UTC));
someConcurrentHashMap.put(user, session);
sessionKeyToUser.put(session.getKey(), user);
return someConcurrentHashMap.get(user).getKey();
}
public int getUserBySessionKey(String sessionKey) {
return sessionKeyToUser.getOrDefault(sessionKey, -1);
}
}
Below code acquires lock on specific object if it exists else on this.
class Solution {
private final Object createSessionLock = new Object();
public String getSessionKey(int user) {
if (validateSessionKey(user)) {
return someConcurrentHashMap.get(user).getKey();
}
throw new InvalidSessionException(SESSION_NOT_FOUND);
}
public String createSession(int user) {
// if user exists then synchronized on the user object
// else synchronized on lock object
if (someConcurrentHashMap.containsKey(user)) {
return createForExistingUser(user);
}
return createForNewUser(user);
}
public int getUserBySessionKey(String sessionKey) {
return sessionKeyToUser.getOrDefault(sessionKey, -1);
}
private String createForExistingUser(int user) {
if (validateSessionKey(user)) {
return someConcurrentHashMap.get(user).getKey();
}
UserSession userSession = someConcurrentHashMap.get(user);
synchronized (userSession) {
if (validateSessionKey(user)) {
return someConcurrentHashMap.get(user).getKey();
}
UserSession session = new UserSession(user, generateSessionId(), LocalDateTime.now(ZoneOffset.UTC));
someConcurrentHashMap.put(user, session);
sessionKeyToUser.put(session.getKey(), user);
return someConcurrentHashMap.get(user).getKey();
}
}
private String createForNewUser(int user) {
if (validateSessionKey(user)) {
return someConcurrentHashMap.get(user).getKey();
}
synchronized (createSessionLock) {
if (validateSessionKey(user)) {
return someConcurrentHashMap.get(user).getKey();
}
UserSession session = new UserSession(user, generateSessionId(), LocalDateTime.now(ZoneOffset.UTC));
someConcurrentHashMap.put(user, session);
sessionKeyToUser.put(session.getKey(), user);
return someConcurrentHashMap.get(user).getKey();
}
}
}
CodePudding user response：
Better approach is to go with the synchronized on specific resource. If we are going to synchronize in method level then a lot of threads will be in waiting state and also there will be possible situation where a lot of things inside a method that can be access by multiple threads leaving only the important block of code that requires thread safety which does not make sense. Most of the time we can go with resource level synchronize to efficiently utilise the available resources.
CodePudding user response：
My understanding is that acquiring lock on specific object for which the concurrent modification needs to be avoided will be faster as all the other threads which are trying to modify other objects are free to proceed.
I think you could be mis-understanding the idea of fine-grained locking. Suppose you have a program in which a large body of data is shared by multiple threads. Suppose also that different threads often only want to access small parts of that data, and often those parts don't overlap.
You face a choice: It's easy and foolproof to use just one lock, to protect the entire database. But it can be more efficient to have many locks that each protect a small part of it. The first way is the most extreme version of coarse grained locking, and the second way is "fine grained." Fine-grained locking may be more efficient, but it means more work for you the programmer: More code to write, more code to test. And, if different threads sometimes do need to access overlapping parts of the database, then fine-grained locking carries the risk of deadlock.
I think the part that you might not understand is the significance of which objects you choose to use as locks.
In Java there is no significance to which object you choose to use as a lock. In Java, when one thread enters a synchronized (foobar) block, the only thing it prevents is, it prevents other threads from synchronizing on the same object, foobar, at the same time. In particular, it does not prevent other threads from accessing foobar.
Synchronizing on some object is completely independent from using the same object in any other way.
The real argument against using so-called "synchronized methods" is that when you do it, the object on which you are synchronizing most likely is a publicly visible object.
If your class Solution with its public synchronized String createSession(...) method is part of a library, then that opens the possibility that some client of your library could use a Solution instance as a lock. It wouldn't be a smart idea for them to do it, but they are allowed to do it. And, if they do it, then their code is using the same object for locking that your code uses. That could lead to weird interactions between their code and yours.
What's worse, is. If you later change when your code synchronizes on a Solution instance, it could break their code when they upgrade to the newest version of your library.
If, instead you only ever synchronize on private final Object lock=new Object() objects, then you avert that problem, and maybe reinforce the illusion among your library clients that your library is "robust" and "reliable."