zanker_swe

> My answer to that is in general, no.

Because '...' can include arbitrary side-effect inducing statements that can't be moved around without affecting the behavior. As the poster discovered.

Sorry, I meant to write:

  synchronized(this) {
    a = AcquireLock();
    for(c = 0; c < 100; c++) {
        f();
    }
  }
  ReleaseLock(a);

which is inline with what the blog post was proposing.

To repeat the blog is a question:

  for (...) {
    synchronized (obj) {
      // something
    }
  }

…​could it optimize into this?

  synchronized (this) {
    for (...) {
       // something
    }
  }

My answer to that is in general, no.

>Just looking at your final example, I can't understand how you can say that is unsafe

Let me change the example a bit. Say we have two locks aL and bL, that we must always acquire in the order aL first and then bL.

Following the rule, say we write code like this:

  import java.util.concurrent.locks.ReentrantLock;
  class X {
    private static ReentrantLock aL = new ReentrantLock();
    private static ReentrantLock bL = new ReentrantLock();
    static int x = 0;
    static int c = 0;
    static public void main(String[] args) {
	for(aL.lock(); c < 100; c++) {
	  synchronized(bL) {
		x = x + 0x42;
	  }
	}
	aL.unlock();
    }
  }

If I understood it right, the blog post was asking a question whether JVM can transform this to:

  import java.util.concurrent.locks.ReentrantLock;
  class X {
    private static ReentrantLock aL = new ReentrantLock();
    private static ReentrantLock bL = new ReentrantLock();
    static int x = 0;
    static int c = 0;
    static public void main(String[] args) {
      synchronized(bL) {
        for(aL.lock(); c < 100; c++) {
            x = x + 0x42;
        }
        aL.unlock();
      } // end synnchronized
    }
  }

Since the locks are now acquired in a different order, does that not qualify as observable behavior?

>And it's unsafe? For what definition of safety?

Let me try to answer with an example. Let us say, we have original code like this:

  synchronized(this) {
   a();
   b();
 }
 c();
 synchronized(this) {
   d();
   e();
 }

It would be unsafe (in general) to transform the above code to

  synchronized(this) {
    a();
    b();
    c();
    d();
    e();
  }

Simply because Compiler does not know (again, in general) what may happen during the execution of c(). However, the following transformation is safe (the timing behavior changes, but as you point out, that is not a guarantee programmers should expect).

  synchronized(this) {
    p();
    q();
  }
  synchronized(this) {
    r();
    s();
  }

to

  synchronized(this) {
    p();
    q();
    r();
    s();
  }

since there is nothing happening between the two synchronized sections.

For the for loop, an example of code where pulling the synchronized statement out of the loop is problematic:

  for(a = AcquireLock(), c = 0; c < 100; c++) {
    synchronized(this) {
      f();
    }
  }
  ReleaseLock(a);

I don't think it is safe to transform to

  a = AcquireLock();
  synchronized(this) {
  for(c = 0; c < 100; c++) {
      f();
    }
  }
  ReleaseLock(a);

Perhaps you (and the original blog post) assume we are only talking about movement of code after ensuring that such movement is safe, but it was not clear from the document.

Regarding the question in the 1st topic: (https://shipilev.net/jvm-anatomy-park/1-lock-coarsening-for-...)

" for (...) { synchronized (obj) { // something } }

…​could it optimize into this?

synchronized (this) { for (...) { <----XXX---- // something } } "

The answer would be no in general, I think, since it is unsafe.

Moving the entire loop into the synchronized block would make the statement marked XXX above, i.e. "for (...)" execute inside the synchronized block, which has the potential to change the semantics (for example, the statement may include an rpc, and we don't want to make that rpc under the lock).

The earlier optimization of

synchronized (this) { statement1; }

synchronized (this) { statement2; }

to

synchronized (this) { statement1; statement2; }

is safe.