I know, old question, but I felt it deserved a little attention.
I know, old question, but I felt it deserved a little attention.
About the only huge, performance-impacting change I can see that would be necessary is replacing the BitArray for a literal bool array.
This actually has a huge speed impact.
So, making that change, and inverting the related conditions causes us to have the following method:
public static IEnumerable<int> Primes(int upperLimit) { if (upperLimit < 2) { throw new ArgumentException("Upper Limit be must greater than or equal to 2."); } yield return 2; if (upperLimit == 2) { yield break; } // Check odd numbers for primality const int offset = 3; Func<int, int> ToNumber = delegate (int index) { return (2 * index) + offset; }; Func<int, int> ToIndex = delegate (int number) { return (number - offset) / 2; }; var bits = new bool[ToIndex(upperLimit) + 1]; var upperSqrtIndex = ToIndex((int)Math.Sqrt(upperLimit)); for (var i = 0; i <= upperSqrtIndex; i++) { // If this bit has already been turned off, then its associated number is composite. if (bits[i]) continue; var number = ToNumber(i); // The instant we have a known prime, immediately yield its value. yield return number; // Any multiples of number are composite and their respective bits should be turned off. for (var j = ToIndex(number * number); (j > i) && (j < bits.Length); j += number) { bits[j] = true; } } // Output remaining primes once bit array is fully resolved: for (var i = upperSqrtIndex + 1; i < bits.Length; i++) { if (!bits[i]) { yield return ToNumber(i); } } } You'll notice that I inverted all the booleans on the bits value, to make sure that requiring a default of false instead of true now would not create issues.
As far as speed goes, the difference seems to be performing in 63.3% of the time of your original method. (For a limit of 100,000,000 the modified version I have here is about 2486ms, yours is about 3917ms.)
Other than that, there is really not much you can do about performance overall.
As far as readability, I would never one-line an if statement without braces.
I'm speaking about:
if (bits[i]) continue;
Break that down into two-lines, it won't hurt anything. ;)
if (bits[i]) continue; You could also do for a little whitespace to break logical portions of your algorithm out.
public static IEnumerable<int> Primes(int upperLimit) { if (upperLimit < 2) { throw new ArgumentException("Upper Limit be must greater than or equal to 2."); } yield return 2; if (upperLimit == 2) { yield break; } // Check odd numbers for primality const int offset = 3; Func<int, int> ToNumber = delegate (int index) { return (2 * index) + offset; }; Func<int, int> ToIndex = delegate (int number) { return (number - offset) / 2; }; var bits = new bool[ToIndex(upperLimit) + 1]; var upperSqrtIndex = ToIndex((int)Math.Sqrt(upperLimit)); for (var i = 0; i <= upperSqrtIndex; i++) { // If this bit has already been turned off, then its associated number is composite. if (bits[i]) continue; // The instant we have a known prime, immediately yield its value. var number = ToNumber(i); yield return number; // Any multiples of number are composite and their respective bits should be turned off. for (var j = ToIndex(number * number); (j > i) && (j < bits.Length); j += number) { bits[j] = true; } } // Output remaining primes once bit array is fully resolved: for (var i = upperSqrtIndex + 1; i < bits.Length; i++) { if (!bits[i]) { yield return ToNumber(i); } } }