HPCE / hpce-2017-cw6

2 stars 17 forks source link

why is unif01_Gen not thread safe? #33

Closed uarif1 closed 6 years ago

uarif1 commented 6 years ago

Hi,

The description says that unif01_gen is not thread safe. I didnt understand why as different tests dont seem to modify its params?

giuliojiang commented 6 years ago

because if you use the generator concurrently you break the assumption that each test should analyze random numbers in sequence

m8pple commented 6 years ago

The problem is less the data-structure unif01_Gen itself, it is on the functions and data-structures that it points to. The main thing that you want to do with an instance is to call something like

long x = unif01_StripB(gen, r, d); // Get some random bits from the generator

Internally that will contain a call which looks like:

return (gen->GetU01) (gen->param, gen->state);

The function pointer gen->GetU01 is provided by the work-load, as is the state gen->state. A very simple version might look like:

unsigned long myGetU01(void *p, void *state)
{
    uint32_t &x = *(uint32_t*)state;
    return x*12345 + 10000.
}

The problem them becomes if you have:

unif01_Gen *gen=workload_Create();
parallel_invoke(
  [](){   unif01_StripB(gen, 0, 32); },   // Task A
  [](){   unif01_StripB(gen, 0, 32); }    // Task B
);

Stripping away the levels of abstraction, this is equivalent to:

unif01_Gen *gen=workload_Create();
uint32_t &x = *(uint32_t*)gen->state; // This is the single state variable of the generator
parallel_invoke(
  [](){   x = x*12345 + 10000;   },   // Task A
  [](){   x = x*12345 + 10000;  }    // Task B
);

Now we can see the fundamental problem - there is one single state variable x associated with the generator instance, and two tasks are trying to do a read-modify-write update. So the two tasks might get the same value, they might get different values, or they might get some value that was completely impossible before.

Changing it to:

unif01_Gen *genA=workload_Create();
unif01_Gen *genB=workload_Clone(genA);
parallel_invoke(
  [](){   unif01_StripB(genA, 0, 32); },   // Task A
  [](){   unif01_StripB(genB, 0, 32); }    // Task B
);

completely removes this problem, and now it is guaranteed that both tasks will see exactly the same value.

So @giuliojiang is correct that it means that your tests won't see a contiguous stream of random samples, but it is worse than that because you may see numbers that weren't part of the original stream.