Java synchronization and performance in an aspect

Question

I just realized that I need to synchronize a significant amount of data collection code in an aspect but performance is a real concern. If performance degrades too much my tool will be thrown out. I will be writing ints and longs individually and to various arrays, ArrayLists and Maps. There will be multiple threads of an application that will make function calls that will be picked up by my aspect. What kind of things should I look out for that will negatively affect performance? What code patterns are more efficient?

In particular I have a method that calls many other data recording methods:

void foo() {
    bar();
    woz();
    ...
}

The methods mostly do adding an incrementing of aspect fields

void bar() {
    f++; // f is a field of the aspect
    for (int i = 0; i < ary.length; i++) {
        // get some values from aspect point cut
        if (some condiction) {
            ary[i] += someValue; // ary a field of the aspect
        }
     }
 }

Should I synchronize foo, or bar, woz and others individually, or should I move all the code in bar, woz, etc into foo and just synchronize it? Should I synchronize on this, on a specifically created synchronization object:

private final Object syncObject = new Object();

(see this post), or on individual data elements within the methods:

ArrayList<Integer> a = new ArrayList<Integer>();

void bar() {    
    synchronize(a) {
        // synchronized code
    }
}

Answer 1

Concurrency is extremely tricky. It's very easy to get it wrong, and very hard to get right. I wouldn't be too terribly worried about performance at this point. My first and foremost concern would be to get the concurrent code to work safely (no deadlocks or race conditions).

But on the issue of performance: when in doubt, profile. It's hard to say just how different synchronization schemes will affect performance. It's even harder for us to give you suggestions. We'd need to see a lot more of your code and gain a much deeper understanding of what the application does to give you a truly useful answer. In contrast, profiling gives you hard evidence as to if one approach is slower than another. It can even help you identify where the slowdown is.

There are a lot of great profiling tools for Java these days. The Netbeans and Eclipse profilers are good.

Also, I'd recommend staying away from raw synchronization altogether. Try using some of the classes in the java.util.concurrency package. They make writing concurrent code much easier, and much less error prone.

Also, I recommend you read Java Concurrency in Practice by Brian Goetz, et al. It's very well written and covers a lot of ground.

Answer 2

Upadte: since writing the below, I see you've updated the question slightly. Forgive my ignorance-- I have no idea what an "aspect" is-- but from the sample code you posted, you could also consider using atomics/concurrent collections (e.g. AtomicInteger, AtomicIntegerArray) or atomic field updaters. This could mean quite a re-factoring of your code, though. (In Java 5 on a dual-proc hyperthreading Xeon, the throughput of AtomicIntegerArray is significantly better than a synchronized array; sorry, I haven't got round to repeating the test on more procs/later JVM version yet-- note that performance of 'synchronized' has improved since then.)

Without more specific information or metrics about your particular program, the best you can do is just follow good program design. It's worth noting that the performance and optimisation of synchronization locks in the JVM has beed one of the areas (if not, the area) that has received most research and attention over the last few years. And so in the latest versions of JVM's, it ain't all that bad.

So in general, I'd say synchronize minimally without "going mad". By 'minimally', I mean so that you hold on to the lock for as less time as possible, and so that only the parts that need to use that specific lock use that specific lock. But only if the change is easy to do and it's easy to prove that your program is still correct. For example, instead of doing this:

synchronized (a) {
  doSomethingWith(a);
  longMethodNothingToDoWithA();
  doSomethingWith(a);
}

consider doing this if and only if your program will still be correct:

synchronized (a) {
  doSomethingWith(a);
}
longMethodNothingToDoWithA();
synchronized (a) {
  doSomethingWith(a);
}

But remember, the odd simple field update with a lock held unnecessarily probably won't make much tangible difference, and could actually improve performance. Sometimes, holding a lock for a bit longer and doing less lock "housekeeping" can be beneficial. But the JVM can make some of those decisions, so you don't need to be tooo paranoid-- just do generally sensible things and you should be fine.

In general, try and have a separate lock for each set of methods/accesses that together form an "independent process". Other than that, having a separate lock object can be a good way of encapsulating the lock within the class it's used by (i.e. preventing it from being used by outside callers in a way you didn't predict), but there's probably no performance difference per se from using one object to another as the lock (e.g. using the instance itself vs a private Object declared just to be a lock within that class as you suggest), provided the two objects would otherwise be used in exactly the same way.

Answer 3

If you compile the aspect into the application then you will have basically no performance hit, if you do it at runtime (load-type weaving) then you will see a performance hit.

If you have each aspect be perinstance then it may reduce the need for synchronization.

You should have as little synchronization as possible, for as short a time as possible, to reduce any problems.

If possible you may want to share as little state as possible between threads, keeping as much local as possible, to reduce any deadlock problems.

More information would lead to a better answer btw. :)

Answer 4

Rule of thumb is not to synchronize on this - most of the times it is a performance hit - all methods are synchronized on one object.

Consider using locks - they'a very nice abstraction and many fine features like, trying to lock for a time period, and then giving up:

       if(commandsLock.tryLock(100, TimeUnit.MILLISECONDS)){
   try {
    //Do something
   } finally{
    commandsLock.unlock();
   }
  }else{
   //couldnt acquire lock for 100 ms
  }

I second opinion on using java.util.concurrent. I'd make two levls of synchronization

• synchronize collection access (if it is needed)
• synchronize field access

Collection access

If your collection are read-only ie no elements get removed-inserted (but elements may change) i would say that you should use synchronized collections (but this may be not needed...) and dont synchronize iterations:

Read only:

for (int i = 0; i < ary.length; i++) {
    // get some values from aspect point cut
    if (some condiction) {
        ary += someValue; // ary a field of the aspect
    }
 }

and ary is instance obtained by Collections.synchronizedList.

Read-write

synchronized(ary){
    for (int i = 0; i < ary.length; i++) {
        // get some values from aspect point cut
        if (some condiction) {
            ary += someValue; // ary a field of the aspect
        }
     }
 }

Or use some concurrent collections (like CopyOnWriteArrayList) which is ingerentently therad safe.

Main difference is that - in first read-only wersion any number of threads may iterate over this collections, and in second only one at a time may iterate. In both cases only one therad at a time should increment any given field.

Field access

Synchronize incrementations on fields separately from synchronizing iterations.

like:

  Integer foo = ary.get(ii); 
  synchronized(foo){
      foo++;
  }

Get rid of synchronization

1. Use concurrent collections (from java.util.concuttent - not from `Collections.synchronizedXXX', latter still need synchronizing on traversal).
2. Use java.util.atomic that enable you to atomically incrememt fields.

Something you should watch:

Java memory model - its a talk that gives very nice understanding on how synchronizations and data aligment in JAVA works.