38

Is better do:

variable1Type foo; 
variable2Type baa; 

foreach(var val in list) 
{
    foo = new Foo( ... ); 
    foo.x = FormatValue(val); 

    baa = new Baa(); 
    baa.main = foo; 
    baa.Do();
}

Or:

foreach(var val in list) 
{
    variable1Type foo = new Foo( ... ); 
    foo.x = FormatValue(val); 

    variable2Type baa = new Baa(); 
    baa.main = foo; 
    baa.Do();
}

The question is: What is faster 1 case or 2 case? Is the difference is insignificant? Is it the same in real applications? This may be a optimization-micro, but I really want know which is better.

Jon Hanna
  • 110,372
  • 10
  • 146
  • 251
The Mask
  • 17,007
  • 37
  • 111
  • 185
  • 1
    There is no performance difference if you are not capturing the variable in a lambda. – Jon Dec 16 '11 at 14:53
  • 3
    You should value the semantic difference and not the performance difference, which in this case is none. By declaring inside the loop you automatically know it's only used inside the loop. – João Angelo Dec 16 '11 at 14:55
  • I hate to do it, but -1: your question does not indicate any research effort. You could have certainly tested this on your own. – Kiley Naro Dec 16 '11 at 14:56
  • 1
    Let me add: you are asking about *performance*, in order to know *the right way*. In this case (as also in many others) **the right way is to think about other things than performance**. – Jon Dec 16 '11 at 14:56
  • 1
    I would argue that later code is more readable – PUG Dec 16 '11 at 15:55

6 Answers6

57

Performance-wise, let's try concrete examples:

public void Method1()
{
  foreach(int i in Enumerable.Range(0, 10))
  {
    int x = i * i;
    StringBuilder sb = new StringBuilder();
    sb.Append(x);
    Console.WriteLine(sb);
  }
}
public void Method2()
{
  int x;
  StringBuilder sb;
  foreach(int i in Enumerable.Range(0, 10))
  {
    x = i * i;
    sb = new StringBuilder();
    sb.Append(x);
    Console.WriteLine(sb);
  }
}

I deliberately picked both a value-type and a reference-type in case that affects things. Now, the IL for them:

.method public hidebysig instance void Method1() cil managed
{
    .maxstack 2
    .locals init (
        [0] int32 i,
        [1] int32 x,
        [2] class [mscorlib]System.Text.StringBuilder sb,
        [3] class [mscorlib]System.Collections.Generic.IEnumerator`1<int32> enumerator)
    L_0000: ldc.i4.0 
    L_0001: ldc.i4.s 10
    L_0003: call class [mscorlib]System.Collections.Generic.IEnumerable`1<int32> [System.Core]System.Linq.Enumerable::Range(int32, int32)
    L_0008: callvirt instance class [mscorlib]System.Collections.Generic.IEnumerator`1<!0> [mscorlib]System.Collections.Generic.IEnumerable`1<int32>::GetEnumerator()
    L_000d: stloc.3 
    L_000e: br.s L_002f
    L_0010: ldloc.3 
    L_0011: callvirt instance !0 [mscorlib]System.Collections.Generic.IEnumerator`1<int32>::get_Current()
    L_0016: stloc.0 
    L_0017: ldloc.0 
    L_0018: ldloc.0 
    L_0019: mul 
    L_001a: stloc.1 
    L_001b: newobj instance void [mscorlib]System.Text.StringBuilder::.ctor()
    L_0020: stloc.2 
    L_0021: ldloc.2 
    L_0022: ldloc.1 
    L_0023: callvirt instance class [mscorlib]System.Text.StringBuilder [mscorlib]System.Text.StringBuilder::Append(int32)
    L_0028: pop 
    L_0029: ldloc.2 
    L_002a: call void [mscorlib]System.Console::WriteLine(object)
    L_002f: ldloc.3 
    L_0030: callvirt instance bool [mscorlib]System.Collections.IEnumerator::MoveNext()
    L_0035: brtrue.s L_0010
    L_0037: leave.s L_0043
    L_0039: ldloc.3 
    L_003a: brfalse.s L_0042
    L_003c: ldloc.3 
    L_003d: callvirt instance void [mscorlib]System.IDisposable::Dispose()
    L_0042: endfinally 
    L_0043: ret 
    .try L_000e to L_0039 finally handler L_0039 to L_0043
}

.method public hidebysig instance void Method2() cil managed
{
    .maxstack 2
    .locals init (
        [0] int32 x,
        [1] class [mscorlib]System.Text.StringBuilder sb,
        [2] int32 i,
        [3] class [mscorlib]System.Collections.Generic.IEnumerator`1<int32> enumerator)
    L_0000: ldc.i4.0 
    L_0001: ldc.i4.s 10
    L_0003: call class [mscorlib]System.Collections.Generic.IEnumerable`1<int32> [System.Core]System.Linq.Enumerable::Range(int32, int32)
    L_0008: callvirt instance class [mscorlib]System.Collections.Generic.IEnumerator`1<!0> [mscorlib]System.Collections.Generic.IEnumerable`1<int32>::GetEnumerator()
    L_000d: stloc.3 
    L_000e: br.s L_002f
    L_0010: ldloc.3 
    L_0011: callvirt instance !0 [mscorlib]System.Collections.Generic.IEnumerator`1<int32>::get_Current()
    L_0016: stloc.2 
    L_0017: ldloc.2 
    L_0018: ldloc.2 
    L_0019: mul 
    L_001a: stloc.0 
    L_001b: newobj instance void [mscorlib]System.Text.StringBuilder::.ctor()
    L_0020: stloc.1 
    L_0021: ldloc.1 
    L_0022: ldloc.0 
    L_0023: callvirt instance class [mscorlib]System.Text.StringBuilder [mscorlib]System.Text.StringBuilder::Append(int32)
    L_0028: pop 
    L_0029: ldloc.1 
    L_002a: call void [mscorlib]System.Console::WriteLine(object)
    L_002f: ldloc.3 
    L_0030: callvirt instance bool [mscorlib]System.Collections.IEnumerator::MoveNext()
    L_0035: brtrue.s L_0010
    L_0037: leave.s L_0043
    L_0039: ldloc.3 
    L_003a: brfalse.s L_0042
    L_003c: ldloc.3 
    L_003d: callvirt instance void [mscorlib]System.IDisposable::Dispose()
    L_0042: endfinally 
    L_0043: ret 
    .try L_000e to L_0039 finally handler L_0039 to L_0043
}

As you can see, apart from the order on the stack the compiler happened to choose - which could just as well have been a different order - it had absolutely no effect. In turn, there really isn't anything that one is giving the jitter to make much use of that the other isn't giving it.

Other than that, there is one sort-of difference.

In my Method1(), x and sb are scoped to the foreach, and cannot be accessed either deliberately or accidentally outside of it.

In my Method2(), x and sb are not known at compile-time to be reliably assigned a value within the foreach (the compiler doesn't know the foreach will perform at least one loop), so use of it is forbidden.

So far, no real difference.

I can however assign and use x and/or sb outside of the foreach. As a rule I would say that this is probably poor scoping most of the time, so I'd favour Method1, but I might have some sensible reason to want to refer to them (more realistically if they weren't possibly unassigned), in which case I'd go for Method2.

Still, that's a matter of how the each code can be extended or not, not a difference of the code as written. Really, there's no difference.

Jon Hanna
  • 110,372
  • 10
  • 146
  • 251
  • 2
    +1 for creating a test case and using it to provide evidence to support your claims. Great work. – Kiley Naro Dec 16 '11 at 15:42
  • Your point is generally valid, and I would also check the IL if I weren't using Mac. However, the IL code still depends on the compiler, hence asserting that there would be no difference at IL level is not totally right in technical perspective (though I would say it's 99.9% right). Anyway I believe most sane and experienced programmer will choose to write like case 2 without any hesitating. – tia Dec 16 '11 at 15:43
  • 1
    @tia, Showing the IL is evidence that it could be the same, and proof that it is the same with current implementations. It is not proof that it *should* be the same, but since the c# code is equivalent any difference would be a minor flaw in the compiler (in producing less-efficient code for whichever was less efficient). These two things together (that it *should* be the same, and that it *is* the same) complete the answer. – Jon Hanna Dec 16 '11 at 15:47
4

It doesn't matter, it has no effect on performance whatsoever.

but I really want know to do right way.

Most will tell you inside-the-loop makes the most sense.

jason
  • 236,483
  • 35
  • 423
  • 525
3

It's just a matter of scope. In this case, where foo and baa are only used within the for loop, it's best practice to declare them inside the loop. It's safer too.

Ed Manet
  • 3,118
  • 3
  • 20
  • 23
1

OK, I answered this without noticing where the original poster was creating a new object every time going through the loop and not just 'using' the object. So, no, there should not really be a negligible difference when it comes to performance. With this, I would go with the second method and declare the object within the loop. That way it will be cleaned up during the next GC pass and the maintains the object within the scope.

--- I will leave up my orignal answer, just because I typed it all, and it might help someone else who searches this message later on. I promise that in the future, I will pay more attention before I try and answer the next question.

Tim

Actually, I think there is a difference. If I recall correctly, every time you create a new object = new foo() you will get that object added to the memory heap. So, by creating the objects in the loop you are going to be adding to the overhead of the system. If you know that the loop is going to be small, it's not a problem.

So, if you end up in a loop with say 1000 objects in it, you are going to be creating 1000 variables that will not be disposed of until the next garbage collection. Now hit a database where you want to do something and you have 20,000+ rows to work with... You can create quite a system demand depending on what object type you are creating.

This should be easy to test... Create an app that creates 10,000 items with a time stamp when you enter the loop and when you exit. The first time you do it, declare the variable before the loop and the next time during the loop. You might have to bump that count up much higher than 10K to see a real difference in speed though.

As well, there is the scope issue. If created in the loop, it's gone once you exit the loop, so you can't access it again. But this also helps with the clean up as the garbage collection will eventually dispose of it once it exit the loop.

Tim

Tim Sapp
  • 86
  • 5
  • Variables don't cause memory in the heap, objects do. They are creating the same number of objects each time. – Jon Hanna Dec 16 '11 at 15:08
  • 1
    Is a variable not an object? I figured that a string variable is a Class system.string object. So when you do something such as "VariableType1 foo = new VariableType1();" are you not creating that as a object which has to be placed in memory? – Tim Sapp Dec 16 '11 at 15:16
  • No. `string x;` variable on stack for object to be assigned to, but no actual object on heap. `x = new string('a', 23);` now there's an object on heap. `string a = x; string b = x; string c = x; string d = x;` 4 more variables, but only one object on heap. `a = b = c = d = x = new string('b', 2);` one or two objects on heap as the first string will be GC'd at some point, but we don't know exactly when. In the question, both forms call `new Foo()` and `new Baa()` the same number of times. It's that which uses heap memory. – Jon Hanna Dec 16 '11 at 15:24
  • Ahh now I see it, I was thinking more along the lines of creating the object before the loop and then USING it, not creating a new object every time. Dhurrr... I guess I need to pay more attention before I jump in there to posting an answer. – Tim Sapp Dec 16 '11 at 16:04
0

Both are completely valid, not sure there's a 'right way' to do this.

Your first case is more memory-efficient (at least in the short term). Declaring your variables inside the loop will force more reallocation of memory; however, with .NETs garbage collector, as those variables go out of scope they will be cleaned up periodically, but not necessarily immediately. The difference in speed is arguably negligible.

The second case is indeed a bit safer, as limiting the scope of your variables as much as possible is usually good practice.

Adam
  • 1,202
  • 11
  • 25
  • Your description of the GC is wrong. In either case, if variable1Type and variable2Type are no longer used, then the objects they point to will be eligible for collection either when the method returns or when another local variable uses their slot on the stack is used for another variable. The two are identical as far as GC goes. Scope has nothing to do with garbage collection. Scope affects what you *can* do with a variable, the compiler will use stack memory depending on what you *did* do, and the GC can claim anything without a live reference to it. – Jon Hanna Dec 16 '11 at 15:36
0

In JS the memory allocation is whole eachtime, In C#, there is no such difference usually, but if the local variable is captured by an anonymous method like lambda expression it will matter.

Muhammad Atif Agha
  • 1,535
  • 3
  • 33
  • 74