1

The following code:

#include <ppl.h>
int i;
vector<int> val(10),summ(10,0);
for(i=0;i<10;i++) val[i]=i;

parallel_for(0, 10, [&] (int y){
    vector<int> vett(1000);
    double vall=val[y];

    for(i=0;i<vett.size();i++)
        vett[i]=vall;

    for(i=0;i<vett.size();i++)
        summ[y]+=vett[i];
 });

for(i=0;i<10;i++)
cout<<summ[i]<<endl;

Produces random output like: 0 1000 1468 204 3600 25 5898 7000 7456 1395

I should use "combinable" I guess, but the documentation I found about it is not very good. Do you know how to make this code work correctly? What if vett is a 2d vector?

Since I'd like to learn parallel computing, is it worthy learning this new microsoft library or there are better alternatives?

MarcoS
  • 160
  • 7
  • What documentation are you using? I've found ["Concurrency Runtime"](http://msdn.microsoft.com/en-us/library/dd504870.aspx) and ["Parallel Programming with Microsoft Visual C++"](http://msdn.microsoft.com/en-us/library/gg675934.aspx) very useful. – Blastfurnace Feb 14 '12 at 14:48

2 Answers2

2

The main problem with your code is the i subscript variable. It is being used by multiple parallel tasks at the same time. Chaos ensues and is the reason for the wonky results. The simplest fix is to declare the loops in your lambda like this:

for(int i=0;i<vett.size();i++)
    vett[i]=vall;

for(int i=0;i<vett.size();i++)
    summ[y]+=vett[i];

Note that each loop index is declared in the for loop initialization. This is more idiomatic usage and can be used in all your loops, unless you really need the final value of i after the loop is finished.

Blastfurnace
  • 18,411
  • 56
  • 55
  • 70
  • to clarify, all variables declared in the parallel_for are shared between treads? so by declaring `i` in the `for` initialization you avoid it. – MarcoS Feb 14 '12 at 16:25
  • 1
    @MarcoS: The lambda in your code captured `i` in an outer scope _by reference_. This meant multiple tasks shared that single variable. If you declare the variable inside the lambda then each task gets its own instance of `i`. By declaring `i` in the for loop initialization the lifetime is even more restricted to just the loop body. – Blastfurnace Feb 14 '12 at 16:31
  • Thanks, just declared another int inside the for_loop and results are no longer random. And I know why. Nice – MarcoS Feb 14 '12 at 16:41
1

A good rule of thumb for parallel code is to minimize the number of shared variables.

As a consequence, you should never use [&] as the lambda capture for parallel_for (or std::thread). Capture only the variables that need to be shared.

If you did this, you would have discovered the problem @Blastfurnace pointed out, namely that i was shared between all workers.

Ben Voigt
  • 277,958
  • 43
  • 419
  • 720
  • I don't know too much about lambda functions so I don't understand what you mean with `[&]`. You mean the index of the 'parallel_for'? I found that as standard example, can that be changed? Seems like lambda functions come first when trying to learn ppl – MarcoS Feb 14 '12 at 16:29
  • @MarcoS: It's not an index. It's a lambda expression which creates a functor object. And yes, you should read about lambda expressions before using them with PPL. For example: http://blogs.msdn.com/b/vcblog/archive/2008/10/28/lambdas-auto-and-static-assert-c-0x-features-in-vc10-part-1.aspx – Ben Voigt Feb 14 '12 at 16:40