synchronization ideas for a method

2
votes

I have a multithreaded class A which accesses the following insert() method of another class B (A has only a single instance of B).

Instead of making the entire method synchronized, are there any better ways to synchronize the following method? (to reduce the synchronization overhead)

private void insert(byte[] shardKey, byte[] queueKey, 
            byte[] value, PipelineMessageType msgType) {
        PipelineMessage pipelineMessage = new PipelineMessage(queueKey, 
                value, msgType);
        LinkedBlockingQueue<PipelineMessage> queue;
        JedisShardInfo shardInfo = shardedJedis.getShardInfo(shardKey);     // shardedJedis is an instance variable of this class
        String mapKey = shardInfo.getHost() + shardInfo.getPort();          
        queue = shardQueue.get(mapKey);         // shardQueue is an instance variable of this class                         
        boolean insertSuccessful = queue.offer(pipelineMessage);
        if(!insertSuccessful) {
            // perform the pipeline sync - flush the queue
            // use another thread for this

            // (processing of queue entries is given to another thread here)

            // queue would be empty now. Insert (k,v)
            queue.offer(pipelineMessage);
        }
    }

I tried to synchronize only the fragment which accesses the instance variables but there might be a scenario where 2 threads try to insert into a full queue and enter the if block. Then 2 threads might process the queue entries which I don't want to happen.

Any suggestions are appreciated. Thank you in advance.

2
javamultithreadingsynchronization
Have you measured "the synchronization overhead"? - Bruno Reis
No :), I just read that there would be an overhead in using synchronization. Is it not significant, especially when the method is called lot of times? - Raghava
I'd say unless you have experience with similar code, you'd better pay attention in writing it right, then measuring "the synchronization overhead", and only then try to optimize, if necessary. - Bruno Reis
Ok, I would do that. Thank u. - Raghava

2 Answers

1
votes

Seems to me that if JedisShardInfo would be a read-only item, then you should need to protect/synchronize it. So you could synchronize only from the line 

queue= ...

Otherwise, almost everything should be synchronized, except the first statement (declaration of pipeline message), and then I really wonder if it changes much compared to declaring the whole method synchronized.

Also, if you got other points of synchronization, I mean other methods or block codes that are synchronized on this, you should consider splitting them and synchronize on different data members of this depending on which data members you wish to protect from multi-threading : 

 Object lockerA = new Object() {};

 synchronized( lockerA )
 {}//sync

Well, not much to say. :)

Regards, Stéphane

1
votes

The key to correct synchronization is to follow this pattern:

synchronize(lockObjectForState) { // All code that alters state must also synchronise on the same lock

    while(!stateOkToProceed()) {
        try {
            lockForState.wait();   
        } catch (InterruptedException e) {
            // handle if your thread was interrupted deliberately as a single to exit, or spuriously (in which case do nothing)
        }
    }

    updateState();

    lockForState.notifyAll();
}

java.util.concurrent package offer many thread-safe implementations of classes needed to solve common threading problems. Consider using a BlockingQueue.