6

I'm trying to make sure I understand the performance implications of synchronized in java. I have a couple of simple classes:

public class ClassOne {

    private ClassTwo classTwo = new ClassTwo();

    public synchronized void setClassTwo(int val1, int val2) {
        classTwo.setVal(val1);
        classTwo.setVal2(val2);
    }

    public static void main(String[] args) {
        ClassOne classOne = new ClassOne();
        classOne.setClassTwo(10, 100);
    }

}

public class ClassTwo {

    private int val;
    private int val2;

    public synchronized void setVal(int val) {
        this.val = val;
    }

    public synchronized void setVal2(int val2) {
        this.val2 = val2;
    }

}

So, as you can see in the previous example, I'm synchronizing on ClassOne.setClassTwo and ClassTwo.setVal and ClassTwo.setVal2. What I'm wondering is if the performance is exactly the same if I remove the synchronization on ClassTwo.setVal and ClassTwo.setVal2, like so:

public class ClassTwo {

    private int val;
    private int val2;

    public void setVal(int val) {
        this.val = val;
    }

    public void setVal2(int val2) {
        this.val2 = val2;
    }

}

They are functionally equivalent in this scenario (assuming no other classes are using these classes), but wondering how much overhead (if any) there is in having more synchronization.

mainstringargs
  • 13,563
  • 35
  • 109
  • 174
  • 2
    How many concurrent Thread(s) do you have? – Elliott Frisch Jul 29 '14 at 17:34
  • 1
    Why don't you try it yourself? You can measure time in Java. – Jashaszun Jul 29 '14 at 17:36
  • 2
    put it under heavy load with many threads calling it. There should be a difference. You don't need the setVal methods to be synchronized and it should save you some time under heavy load. – Rocky Pulley Jul 29 '14 at 17:36
  • *They are functionally equivalent,* - I don't think so.. `public synchronized void setClassTwo` - you are actually synchronizing on an instance of `class1`. The *synchronized* keywords on `setVal()` and `setVal2()` are *necessary* to ensure that other threads (apart from the one running `setClassTwo()` don't update the values of ClassTwo instance). *Synchronizing* `setVal()` gets a lock on *ClassTwo*. – TheLostMind Jul 29 '14 at 17:51
  • 1
    Creating a meaningful benchmark is not as straight forward. See this article https://wikis.oracle.com/display/HotSpotInternals/MicroBenchmarks – Alexander Pogrebnyak Jul 29 '14 at 17:56

3 Answers3

5

Will there be overhead? Yes.

Will there be much overhead? Depends.
If there's one thread only, then the answer is "No", even in these ancient times uncontended synchronization was quick, supposedly they made it even better since.

So what happens if there is more than 1 thread? Well here's the problem: The 2 versions you posted are not functionally equivalent. Why? Because the submethods you are calling are public methods of a public class. Hence they can be called outside your setClassTwo and hence - have no synchronization.

The other thing to note is that they synchronize on different monitors. The second version synchronizes on 1 monitor only, while the original - on two.

TL;DR

Leave the synchronized on the methods that need to be synchronized, don't just expect the caller to synchronize (unless it's embedded in the class API). If the calling methods do the synchronization right, there will be no contention and the overhead will be very small, and if they fail to do it somehow (through someone calling your method directly, for example), then you get contention and larger overhead - but you still have thread safety.

Community
  • 1
  • 1
Ordous
  • 3,844
  • 15
  • 25
  • Thanks for the answer. When I said they are "functionally equivalent", I should've added "in this scenario". – mainstringargs Jul 29 '14 at 17:56
  • @dubdubdubdot - *in this scenario*, there is only *one thread*. :P. So, effectively, *synchronization* is useless. (unless you consider the added overhead of locking / unlocking.. ) – TheLostMind Jul 29 '14 at 17:58
  • @dubdubdubdot It's difficult to imagine a situation where they would be functionally equivalent except having just these 2 classes and a single thread. As soon as you introduce someone who locks on the `classTwo` object, not even touching the variables, they are different. – Ordous Jul 29 '14 at 18:09
1

In your first case, you can create multiple threads and call setVal() on ClassTwo directly and not worry about memory inconsistencies (setVal() on ClassTwo is synchronized ). In your second case, you will have to be ready for unexpected results if you run multiple threads and call setVal() directly. Also, if you are always certain that setVal() will only be called from setClassTwo(), then I suggest you synchronize on Class2 instance using a synchronized block and keep setVal() and setVal2() as synchronized. Rule of thumb - only synchronize what can be accessed concurrently.

TheLostMind
  • 35,966
  • 12
  • 68
  • 104
  • 2
    As with all rules of thumb - there's one in the opposite direction: *Synchronize anything that can be accessed concurrently and deals with shared state unless you explicitly made sure concurrent access is dealt with properly*. The conflict between these two rules in this case highlights another small problem - if an object is shared between threads, you should not allow modification of said state from other objects - do it internally in that object while maintaining proper synchronization. – Ordous Jul 29 '14 at 18:23
  • @Ordous - well.. Couldn't agree more :) – TheLostMind Jul 30 '14 at 05:31
1

How you synchronize is also dependent on how you plan to consume ClassTwo. If writes are rare, and reads are frequent, a readwritelock may give you better performance at larger scales. The reads are typically hindered only if a write is in process because read locks are shared by multiple reader threads, while writes block all until the write is complete.

http://docs.oracle.com/javase/7/docs/api/java/util/concurrent/locks/ReadWriteLock.html

I hope this helps.

Jeff C.
  • 91
  • 4