2

I'm new to C# and am basically trying to generate large Integers using the BigInteger class in C#, by feeding it a byte[] array of randomly filled values. I have another method, CheckForPrime(BigIngeger b) that checks for a prime number and simply returns true if the number is prime. So to run everything, I'm using a Parallel ForEach loop which loops infinitely until a condition is false. Here is my code:

private static IEnumerable<bool> IterateUntilFalse(bool condition)
{
    while (condition) yield return true;
}

and here is my attempt at a Parallel.ForEach loop that runs till the condition is false:

public void PrintOutPrimes(int numPrimes)
{
    byte[] makeBytes = new byte[512];
    BigInteger makePrime;
    RandomNumberGenerator r = RandomNumberGenerator.Create();
    ConcurrentBag<BigInteger> bbag = new ConcurrentBag<BigInteger>();

    int trackNum = 0;
    bool condition = (trackNum <= numPrimes);

    Parallel.ForEach(IterateUntilFalse(condition), (ignored, state) =>
    {
        if (trackNum > numPrimes) state.Stop();
        r.GetNonZeroBytes(makeBytes);
        makePrime = BigInteger.Abs(new BigInteger(makeBytes));
        if (CheckForPrime(makePrime))
        {
            bbag.Add(makePrime);
            if(bbag.TryPeek(out makePrime))
            {
                Console.WriteLine(makePrime);
            }
            Interlocked.Increment(ref trackNum);
        }
    });
}

I want to generate large prime numbers using the performance of Parallel.ForEach, and then print them out as it finds them. The problem seems like the code is "bypassing" the if statement in the loop and adding non-prime numbers to the concurrent bag too. Is there a way to avoid this and guarantee that only the prime numbers be added to the bag and then printed sequentially?

ThomasArdal
  • 4,999
  • 4
  • 33
  • 73
kodak67
  • 21
  • 1
  • 1
    You probably ought to be using variables local to the `ForEach` call. Because your variables are declared outside that call, you only have one instance of them for all threads. That means that one thread might check whether a value is prime and find that it is, then another thread change that value, then the first thread add the new value to the list. Any data that is specific to one calculation should be using variables declared within the scope of that one calculation. – John Jun 24 '22 at 06:49
  • 1
    Consider this: Thread 1 (T1) populates `makeBytes` and then sets `makePrime`. It then checks if it is prime, and it decides that it is. Meanwhile, T2 populates `makePrime` with a new value and starts its own prime check. T1 now adds the new `makePrime` value produced by T2 into the bag. You could be using variables local to the `ForEach` method, not shared by each thread in the `ForEach` method, or otherwise you'll get inconsistent results due to multiple threads accessing the resources at the same time. – ProgrammingLlama Jun 24 '22 at 06:51
  • 1
    Related: [What's the best way of achieving a parallel infinite Loop?](https://stackoverflow.com/questions/8671771/whats-the-best-way-of-achieving-a-parallel-infinite-loop) – Theodor Zoulias Jun 24 '22 at 20:34

1 Answers1

3

bool condition is not a condition, it's the result of evaluating the condition once. Basically it is always true and will never change. Therefore, your iterator will yield true forever.

To solve that, you need a real condition that can be evaluated several times, like a Lambda expression. But even with a lambda this won't work well due to race conditions in your local variables.

Actually I don't really see why you use Parallel.Foreach at all. If you want 500 primes, then use Parallel.For, not Parallel.Foreach.

Instead of generating a huge amount of random numbers and checking them for primality, choose a random number and increment it until you find a prime number. That way it's guaranteed that you get one prime number for every random number input.

Concept:

Parallel.For(0, numPrimes, (i, state)=>
{
    byte[] makeBytes = new byte[512];
    r.GetNonZeroBytes(makeBytes);
    BigInteger makePrime = BigInteger.Abs(new BigInteger(makeBytes));
    while (!IsPrime(makePrime))
       makePrime += 1;       // at some point, it will become prime
    bbag.Add(makePrime);
});

You probably want to start with an odd number and increment by 2.

You definitely want one RNG per thread instead of accessing a single global RNG, except you have a thread-safe RNG.

You also want the byte[] to be part of the state, so that it does not get recreated per loop and messes with garbage collection.

Thomas Weller
  • 55,411
  • 20
  • 125
  • 222