HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Keeping Linq's readability (e.g. over goto statements) without sacrificing performance

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
keepingwithoutlinqsacrificingreadabilitystatementsgotoperformanceover

Problem

I'm doing Project Euler problem 5 as a kata, focussing on TDD and code readability. The challenge is:


What is the smallest positive number that is evenly divisible by all of the numbers from 1 to 20?

For reference, a "less-readable" solution that reeks of goto was something along these lines:

public int Solution_GotoVersion()
{
    int solution = 1;
    outer: while (solution++ < int.MaxValue)
    {
        for (int j = 1; j <= 20; j++)
        {
            if (solution % j != 0)
                goto outer;
        }
        return solution;
    }
    throw new Exception("Solution not found");
}


This takes about 4 seconds to run, relative to the other solutions.

Of course I was trying to avoid such code (in light of readability, for one), I wrote it specifically to compare it to my first solution, that uses an alternative along the lines of this SO answer. So, focussing on readability, here's something a little nicer:

public int Solution_LinqVersion()
{
    var dividers = Enumerable.Range(1, 20);

    int solution = Enumerable
                    .Range(1, int.MaxValue)
                    .FirstOrDefault(candidate
                      => dividers.All(divider => candidate.IsDivisibleBy(divider)));

    return solution;
}


This performs much worse, taking about 25 seconds, relatively. I don't mind loosing some performance when traded for readability, but a factor 8 is a bit much.

I've tried an intermediate solution, like this:

public int Solution_LittleOfBothWorlds()
{
    var dividers = Enumerable.Range(1, 20).ToArray();
    int solution = 1;
    while (solution++  solution % divider == 0))
            return solution;
    }
    throw new Exception("Solution not found");
}


This is in the middle, performance-wise, taking about 14 seconds, relatively.

What can I do to keep relative performance close to the first example, with code that utilizes Linq's readability?

Note: I'm aware there are of many other optimizations possible,

Solution

Focus on optimizing the innermost loop. That's where the performance is lost. In particular try to minimize the number of indirect calls (virtual method, interface and delegate calls).

Your LINQ code has ~60 of those. My variant 1 reduces them to ~20, and my variant 2 to only a handful.

Variant 1:

Very close to linq, but using Array.TrueForAll. About twice as fast are pure linq:

public int Solution_ArrayLinqVersion()
{
    var dividers = Enumerable.Range(1, 20).ToArray();

    int solution = Enumerable
                    .Range(1, int.MaxValue)
                    .First(candidate
                      => Array.TrueForAll(dividers, divider => candidate%divider==0));

    return solution;
}


Variant 2:

Use a loop for the division testing(inner loop) and linq for the outer loop. Almost as fast as no LINQ:

int LinqOutLoopInner()
{
    return Enumerable
        .Range(1, int.MaxValue)
        .First(IsDivisibleBy1to20);
}

static bool IsDivisibleBy1to20(int candidate)
{
    for (int j = 1; j <= 20; j++)
    {
        if (candidate % j != 0)
            return false;
    }
    return true;
}

Code Snippets

public int Solution_ArrayLinqVersion()
{
    var dividers = Enumerable.Range(1, 20).ToArray();

    int solution = Enumerable
                    .Range(1, int.MaxValue)
                    .First(candidate
                      => Array.TrueForAll(dividers, divider => candidate%divider==0));

    return solution;
}
int LinqOutLoopInner()
{
    return Enumerable
        .Range(1, int.MaxValue)
        .First(IsDivisibleBy1to20);
}

static bool IsDivisibleBy1to20(int candidate)
{
    for (int j = 1; j <= 20; j++)
    {
        if (candidate % j != 0)
            return false;
    }
    return true;
}

Context

StackExchange Code Review Q#18743, answer score: 7

Revisions (0)

No revisions yet.