Making adding to a dictionary more efficient?

Question

I have the following code and it's taking quite some time to run. I'm getting an Out of Memory Exception due to the high volume of answers there are. Is there any way I could speed it up?

var controlStrings = answerControlStrings.Where(a => a.ControlN.Substring(a.ControlN.IndexOf('_') + 1, a.ControlN.Length - a.ControlN.IndexOf('_') - 1).Equals(newAnswer.QVar, StringComparison.InvariantCultureIgnoreCase));
if (controlStrings.Count() > 0)
{
    newAnswer.CatiControlStrings.Add("en", controlStrings.Where(c => c.CultureN.Contains("cati") && c.CultureN.Contains("en")).ToList());
    newAnswer.WebControlStrings.Add("en", controlStrings.Where(c => c.CultureN.Contains("web") && c.CultureN.Contains("en")).ToList());
    newAnswer.CatiControlStrings.Add("es", controlStrings.Where(c => c.CultureN.Contains("cati") && c.CultureN.Contains("es")).ToList());
    newAnswer.WebControlStrings.Add("es", controlStrings.Where(c => c.CultureN.Contains("web") && c.CultureN.Contains("es")).ToList());
}

Answer 1

You're iterating through controlStrings 4 times (because you have 4 Where clauses).

It might be better to rewrite this as a for loop:

foreach (var c in controlStrings)
{
    var cultureN = c.CultureN;

    if (cultureN.Contains("cati") && cultureN.Contains("en"))
        ... add c to CatiControlStrings;

    if (cultureN.Contains("web") && cultureN.Contains("en"))
        ... add c to WebControlStrings;

    ... and the same for Spanish.
}

You can also call Contains fewer times:

foreach (var c in controlStrings)
{
    var cultureN = c.CultureN;

    bool cati = cultureN.Contains("cati");
    bool web = cultureN.Contains("web");
    if (!cati && !web)
        continue;
    bool en = cultureN.Contains("en");
    bool es = cultureN.Contains("es");

    if (cati && en)
        ... add c to CatiControlStrings;

    if (web && en)
        ... add c to WebControlStrings;

    ... and the same for Spanish.
}

Also your Count() statement may be expensive and unnecessary.

You're calculating IndexOf twice for every string.

And you're creating a new Substring for every string you test. Instead of creating a substring, use one of the overloads of String.Compare which lets you compare substrings.

Alternatively something like this might be better (because it doesn't use Substring and is simpler than using IndexOf with Compare:

string compareWith = "_" + newAnswer.QVar;
var controlStrings = answerControlStrings.Where(a => a.ControlN.EndsWith(compareWith,
    StringComparison.InvariantCultureIgnoreCase));

Garbage, garbage, everywhere says that using a Where will generate a lot of temporary objects. I don't understand why you're getting "out of memory" sometimes, but rewriting even your first statement to use less Linq might help (or at least make the cause your "out of memory" exception clearer).

foreach (var a in controlStrings)
{
    if (a.ControlN.EndsWith(compareWith, StringComparison.InvariantCultureIgnoreCase))
    {
        // the add-to-dictionary code above
    }
}

Reading C# Does Lambda => generate garbage? (including comments to the accepted answer), Linq generates temporary garbage.

I have a suspicion, after reading Garbage Collection and Performance, that maybe the framework doesn't garbage-collect before throwing 'out of memory'; my guess is that:

• Garbage collection runs when it feels like it
• Garbage collection runs on a separate thread, without pausing/blocking your threads
• Unlike Java, C# will/may not stop to garbage collect immediately before/instead of throwing 'out of memory exception'
• Therefore if your thread generates too much garbage too fast (i.e. faster than the garbage is collected), it may throw.

An alternative theory is that garbage remains uncollectable inside the lambda instances until your var controlStrings goes out of scope.

Simplifying your code to use less Linq and no Substring instances should produce less garbage, which might help to avoid throwing the exception.