Reputation: 629
Simply speaking, my problem is as follows:
I have an class named Test and it has 3 double arrays,named array1, array2,array3 respectively(assuming they have the same length).It has a major function named Estep, and I need to implement this function with multi-threading to improve speed.
In Estep, I need to modify array1 and array2, and array3 is only for read.I define an indicator variable named which_array to indicator which array to modify.I passed the three arrays to Estep and modify array1 and array2.
My code is as follows and I test it and it works fine as the modification of array1 and array2 in Estep can be seen in Test class. but I still have some doubt if it is absolutely right. I'm wondering if I should add volatile to three array1 in Test class or should I need some synchronization mechanism?.
Any advice would be appreciated!
public class Test{
double[] array1;
double[] array2;
double[] array3;
//for simplifization, I omitted the code for allocation and initialization of the the arrays.
public void EstepInTest()
{
final CountDownLatch countdown = new CountDownLatch(2);
Estep modifyArray1 = new Estep(array1, array2, array3, 1,countdown);
Thread t1 = new Thread( firstHalf);
t1.start();
Estep modifyArray2 = new Estep(array1, array2, array3, 2,countdown);
Thread t2 = new Thread(secondHalf);
t2.start();
try{
countdown.await();
}catch (InterruptedException e) {
e.printStackTrace();
}
//do next things
}
}
class Estep implements Runnable{
double[] array1;
double[] array2;
double[] array3;
int which_array;
Estep(double[] array1, double[] array2, double[] array3, int which_array, CountDownLatch cd)
{
this.array1 = array1;
this.array2 = array2;
this.array3 = array3;
this.which_array = which_array;
this.cd = cd;
}
public void run(){
if( this.which_array == 1){
for( int i = 0; i < array1.length; ++i )
{
array1[i] = array1[i] + array3[i];
}
}else{
for( int i = 0; i < array2.length; ++i )
{
array2[i] = array2[i] + array3[i];
}
}
cd.countDown();
}
Upvotes: 1
Views: 2234
Reputation: 691943
It is right. CountDownLatch.await()
guarantees that everything done before it returns happens-before everything done after it has returned.
Your code would be cleaner and shorter if you simply passed the array to modify to the Estep constructor, instead of passing two arrays, and a magic number to indicate which one to modify.
EDIT:
regarding the access to array3
, it's safe as well, because, as explained in the documentation of the concurrent package, a call to start
on a thread happens-before any action in the started thread. So the two threads will see every modification made to array3
that has been made before starting them.
Also, note that making an array volatile only makes the array reference volatile. It doesn't make its elements volatile.
Upvotes: 2
Reputation: 3260
I'm not that familiar with countdown latch.
You can also synchronize the array object itself. You can do this by setting up the array as a monitor object to be synchronized in the getters and setters. This should prevent any access to the array that affects your current line, and will block the minimum amount of code from happening in an asynchronous way.
// In the original test class
public int getArray1(int index) {
unsynchronized_statements...
int result;
synchronized(array1) {
result = array1[index]...
}
return result;
}
Upvotes: 0
Reputation: 11
Consider to pass to Estep only two arrays, one for modification next for reading. In this case you do not need redundant which-array variable and so general code in run
Upvotes: 0