C言語とC++言語コミュのマルチスレッド　共有変数？

初めまして。始めたばかりですがマルチスレッドについて学んでいるものです。

下のプログラムは、π計算をするものなのですが、ひとつ目のスレッドは計算できるのですが、ふたつ目のスレッドはpaiの値がおかしくなります。

これは、共有変数が関係してくるのでしょうか？
どなた様かわかる方いらっしゃいましたらご教授願えないでしょうか。

#include<stdio.h>
#include<windows.h>

#define threads 2
#define loop 1000
#define width (1.0 / loop)

struct threadarg {
int begin;
int end;
double pai;
};

void Thread_func(void *targ)
{
threadarg* arg = (threadarg *)targ;
int i;
double x;
for( i = arg->begin; i < arg->end; i++){
x = (i + 0.5) * width;
arg->pai += 4.0 / (1.0 + x * x);
//printf("pai = %f\n", arg->pai);
}
}

int main(){
int cpu_num;
SYSTEM_INFO sysinfo;
GetSystemInfo(&sysinfo);
cpu_num=sysinfo.dwNumberOfProcessors;

HANDLE handle[100];
threadarg* arg = new threadarg[cpu_num];
int i;
　　　arg->pai=0.0;

for ( i = 0; i < cpu_num; i++)
{
arg[i].begin = i * loop / cpu_num;
arg[i].end = (i + 1) * loop / cpu_num;
handle[i]=CreateThread(NULL,0,(LPTHREAD_START_ROUTINE)
　　　　　　　　　Thread_func,(void *)&arg[i],0,NULL);
}

WaitForMultipleObjects(cpu_num,handle,TRUE,INFINITE);

// 計算結果の出力
arg->pai = arg->pai / loop;
printf("PAI = %f\n", arg->pai);

return 0;
}

コメント(16)

最初
全て
最新の40件

[1] mixiユーザー 02月11日 23:40

おそらく同じ変数に同時に書き込もうとして衝突しているのだと考えられます。
何らかの排他制御が必要で、今回の場合はクリティカルセクションが最も適しています。
クリティカルセクションでググッテ見てください。

[2] mixiユーザー 02月11日 23:42

LOCKとかUNLOCKって使わなかったっけ？？

[3] mixiユーザー 02月11日 23:46

実際に試していないので憶測で書きます。

Thread_funcの中で arg->pai に加算している部分。

arg は cpu_num の数だけ確保されているので、 ->pai はたくさんあるので、結果的にスレッドごとの計算結果の総和になっていません。

初期化の部分で、 arg->pai=0.0; としていますが、これだと arg[0].pai=0.0; と同じですから、
arg[1].pai=0.0; ～ arg[cpu_num-1].pai=0.0; の範囲が初期化されません。

修正の方針としては、 pai を1個だけにして共有変数とし、ミューテックスかクリティカルセクションで排他処理をする方法と、スレッドの数だけ pai を用意して、結果出力の直前で、各スレッドごとのpaiの総和を計算する方法があると思います。後者なら排他処理はいらないです。

[4] mixiユーザー 02月11日 23:52

いや、最初のarg->paiしか初期化してないからでしょ。

[5] mixiユーザー 02月11日 23:54

>003
すみません、かぶったｗ

[7] mixiユーザー 02月12日 00:51

>そらみみさん

CRITICALSECTIONを使ってみたのですが強制終了してしまいました。

struct threadargの中に、
CRITICAL_SECTION *cs;

thread_funcに
EnterCriticalSection(arg->cs);
for( i = arg->begin; i < arg->end; i++){
x = (i + 0.5) * width;
pai += 4.0 / (1.0 + x * x);
printf("pai = %f\n", pai);
}
LeaveCriticalSection(arg->cs);

main()のなかに、
CRITICAL_SECTION cs;

InitializeCriticalSection(&cs);

で最後に
DeleteCriticalSection(&cs);

と書いたのですが、どこか違うのでしょうか。

[8] mixiユーザー 02月12日 00:57

ポインタだけがあって中身がない状態になっていますよ

[9] mixiユーザー 02月12日 08:21

構造体に CRITICALSECTION *cs; を追加して、初期化のループで arg[i].cs = &cs; はやってますかね？
それよりか、 cs は main の外に出して、グローバル変数にした方が理解しやすいかと思います。
グローバル変数にしたくなければ、最終的にクラスを作る方向で。

あと、EnterCriticalSectionとLeaveCriticalSectionの中に、forループ全体を入れてしまうと、せっかくのマルチスレッドが効きませんから、 pai += の部分だけEnterとLeaveで囲むようにしましょう。

[10] mixiユーザー 02月12日 09:27

> そらみみさん

PAIもグローバル変数にした方がいいのでしょうか？

[11] mixiユーザー 02月12日 10:00

>>3に書いたとおり、二通りのやり方があると思います。

●paiを共有変数にする方法

paiをグローバル変数にして、
Thread_funcの中で

for( i = arg->begin; i < arg->end; i++){
x = (i + 0.5) * width;
x = 4.0 / (1.0 + x * x);
EnterCriticalSection(arg->cs);
pai += x;
LeaveCriticalSection(arg->cs);
}

●paiをスレッド毎に持たせる方法

mainの後半で、

WaitForMultipleObjects(cpu_num,handle,TRUE,INFINITE);

double pai = 0;
for ( i = 0; i < cpu_num; i++) {
pai += arg[i].pai;
}
pai /= loop;

// 計算結果の出力

[12] mixiユーザー 02月12日 10:41

何のためのマルチスレッドかを考えたら、paiを更新するたびに排他制御をする方式は　無し　でしょう。
x = (i + 0.5) * width;　x = 4.0 / (1.0 + x * x);の計算1回ごとに排他制御なんて、CPUパワーの大半が排他制御で消えてしまうのでは？

[13] mixiユーザー 02月12日 11:00

1) スレッド毎にローカル変数 PAI を用意する。
2) スレッドでの計算が終わったら、PAI の値を FIFO に送る。全スレッド同様。
3) 集計専用のスレッドを立ち上げておいて FIFO 待ち。送られてくる値を集計しまくって、規定数量来たら完了。

処理系毎の FIFO の詳細は、ググって。

[14] mixiユーザー 02月12日 11:06

PAI は小文字でかまいません…

[15] mixiユーザー 02月12日 11:53

double p = 0;
for( i = arg->begin; i < arg->end; i++){
x = (i + 0.5) * width;
x = 4.0 / (1.0 + x * x);
p += x;
}
EnterCriticalSection(arg->cs);
pai += p;
LeaveCriticalSection(arg->cs);

こんなところか。
まあ、これだと、スレッド毎にpaiを持たせるのと大差ないですけど。

[16] mixiユーザー 02月12日 14:51

> そらみみさん
ありがとうございます。できました。
結局スレッド毎にpaiを持たせることにしました。

ログインすると、残り1件のコメントが見れるよ