[SMALLFIX]Avoid double synchronized #4812
Conversation
|
Merged build finished. Test PASSed. |
|
Test PASSed. |
|
Automated checks report:
All checks passed! |
|
@maobaolong thanks for looking at it. Out of curiosity, how did you find the double synchronized issue? For example, did you hit deadlock or through profiling? |
|
@yupeng9 , Thanks for your review, I find this while debug alluxio, I know double sychronized doesn't cause deadlock there, I just think it is not necessary to mark the outer function synchronized. |
|
@maobaolong could you explain it a bit more details here? when you say outer functions, do you mean |
|
@apc999 as far as I know, the three functions |
|
Would it make sense to make |
|
make sense, and which way will be the better way? @ @calvinjia |
|
@calvinjia PTAL. |
|
Merged build finished. Test PASSed. |
|
Test PASSed. |
|
@maobaolong Did you observe significant downside (ie. possible deadlock) or overhead for having the double synchronize? I don't think this should be expected (see: link) |
|
The following test class was shown and solve many puzzled. Let's diff myFourMethod myFiveMethod and my SevenMethod, after saw their bytecode, we can find that their bytecode were almost same, so java compiler help us do something. So. the useless synchronized will not influence performance, just useless only. package net.mbl.demo.concurrentdemo.synchrionzed;
/**
* net.mbl.demo.concurrentdemo.synchrionzed <br>
*
* @author maobaolong@139.com
* @version 1.0.0
*/
public class SyncTest {
public static void main(String[] args) {
final B b = new B();
// A synchronized method in the superclass acquires the same lock as one in the subclass
new Thread(){
public void run(){
int a=2;
a++;
b.myOneMethod();
a--;
}
}.start();
// Two synchronized method in the same instance acquires the same lock. In different critical
// region.
new Thread(){
public void run(){
int a=2;
a++;
b.myTwoMethod();
a--;
}
}.start();
new Thread(){
public void run(){
int a=2;
a++;
b.myThreeMethod();
a--;
}
}.start();
// A synchronized region in the same thread can re-entered.
new Thread(){
public void run(){
int a=2;
a++;
b.myFourMethod();
a--;
}
}.start();
new Thread(){
public void run(){
int a=2;
a++;
b.myFiveMethod();
a--;
}
}.start();
new Thread(){
public void run(){
int a=2;
a++;
b.mySixMethod();
a--;
}
}.start();
new Thread(){
public void run(){
int a=2;
a++;
b.mySevenMethod();
a--;
}
}.start();
}
}
class A {
public synchronized void myOneMethod(){
System.out.println("{ myOneMethod");
for(int i = 0; i < 10; i++){
System.out.println("myOneMethod " + i);
try {
Thread.sleep(1000L);
} catch (InterruptedException e) {
e.printStackTrace();
}
}
System.out.println("} myOneMethod");
}
}
class B extends A {
public synchronized void myTwoMethod(){
System.out.println("{ myTwoMethod");
for(int i = 0; i < 10; i++){
System.out.println("myTwoMethod " + i);
try {
Thread.sleep(1000L);
} catch (InterruptedException e) {
e.printStackTrace();
}
}
System.out.println("} myTwoMethod");
}
public synchronized void myThreeMethod(){
System.out.println("{ myThreeMethod");
for(int i = 0; i < 10; i++){
System.out.println("myThreeMethod " + i);
try {
Thread.sleep(1000L);
} catch (InterruptedException e) {
e.printStackTrace();
}
}
System.out.println("} myThreeMethod");
}
public synchronized void myFourMethod(){
System.out.println("{ myFourMethod");
myOneMethod();
System.out.println("} myFourMethod");
}
public void myFiveMethod(){
System.out.println("{ myFiveMethod");
myThreeMethod();
System.out.println("} myFiveMethod");
}
public void mySixMethod(){
System.out.println("{ mySixMethod");
for(int i = 0; i < 10; i++){
System.out.println("mySixMethod " + i);
try {
Thread.sleep(1000L);
} catch (InterruptedException e) {
e.printStackTrace();
}
}
System.out.println("} mySixMethod");
}
public synchronized void mySevenMethod(){
System.out.println("{ mySevenMethod");
mySevenMethod();
System.out.println("} mySevenMethod");
}
}The following are the bytecode of myFourMethod myFiveMethod and mySevenMethod. |
|
@maobaolong thanks for the investigation by analyzing JVM bytecodes. Your experiment is quite educational. Thanks |
No description provided.