Java synchronization with entries in a static Map

I have a static Map, which I need to synchronize access to. The map is keyed by a user id. I want to optimize the synchronization such that I don't block all threads where I could block only threads that relate to the same user id.

private static Object s_lock = new Object();
private static Map<String,User> s_users = new HashMap();
...
private someMethod() {
    synchronized(s_lock)
    {
        // keeping the global lock for as little as possible
        user=getMapEntry();
    }
    synchronized(user)   <-------- (1)
    {
        // time consuming operation
        // hopefully only blocking threads that relate to same user id.
    }
}
...
private User getMapEntry(String userId)
{
    if (s_users.containsKey(userId)) {
        user = s_users.get(userId);
    }
    else {
        user = new User();
        user.id = userId;
        s_开发者_开发问答users.put(userId, user);
    }
    return user;
}

My question is - at (1) I am assuming that I am not holding the 'global' sync lock, but as the s_users map is static, are the entries effectively static, meaning that I am still holding the global lock (i.e. sync'ing on the class object)?

Nope, you're good: each map-entry is a separate object, so synchronizing on a map-entry won't synchronize on the map, nor on the class that owns the map.

(By the way, your #getMapEntry(...) method actually returns a value, rather than an entry. A map entry contains references both to a key and to a value.)

You are not sync'ing on the class object but I it is doing what you think it is doing:

• Different threads are accessing the Map using the shared s_lock. But I think you can just as well make s_users final and synchronize on that

• Different thread accessing the same User object execute the " time consuming operation " in sequence.

The only point of caution I see is if the Object keep changing in the map for the same User. As long as you have only one User object for one userId you are fine.

I have done something similar but synchronized on userId.intern(). It has its disadvantages but worked well for my case.

This should work; the second block syncs only over this user.

Small test:

public class Test {

    protected class User {
        String id;
    }
    private static Object s_lock = new Object();
    private static Map<String,User> s_users = new HashMap<String, User>();

    public void someMethod(String id) throws InterruptedException {
        User user;
        synchronized(s_lock)
        {
            // keeping the global lock for as little as possible
            user = getMapEntry(id);
        }
        synchronized(user)
        {
            System.out.println("Waiting for user: "+user.id);
            Thread.sleep(10000);
        System.out.println("Finished waiting for user: "+user.id);
        }
    }

    private User getMapEntry(String userId)
    {
        User user;

        if (s_users.containsKey(userId)) {
            user = s_users.get(userId);
        }
        else {
            user = new User();
            user.id = userId;
            s_users.put(userId, user);
        }

        return user;
    }

    public static void main(String[] args) throws InterruptedException {
        final Test test = new Test();

        for(int i = 0; i < 10; i++) {
            final int j = i;

            new Thread() {
                public void run() {
                    try {
                        test.someMethod(String.valueOf(j % 5));
                    } catch (InterruptedException e) {}
                }
            }.start();
        }
    }
}

Output:

Waiting for user: 0
Waiting for user: 1
Waiting for user: 3
Waiting for user: 2
Waiting for user: 4
Finished waiting for user: 0
Finished waiting for user: 1
Waiting for user: 1
Waiting for user: 0
Finished waiting for user: 3
Finished waiting for user: 2
Waiting for user: 3
Waiting for user: 2
Finished waiting for user: 4
Waiting for user: 4
Finished waiting for user: 0
Finished waiting for user: 1
Finished waiting for user: 3
Finished waiting for user: 2
Finished waiting for user: 4

So it's fine.