1

Below is the code i am using

private void TestFunction()
{
  foreach (MySampleClass c in dictSampleClass)
  {
    String sText = c.VAR1 + c.VAR2 + c.VAR3
    PerformSomeTask(sText,c.VAR4);
  }
}

My friend has suggesed to change to (to improve performance. dictSampleClass is a dictionary. It has 10K objects)

private void TestFunction()
{
  String sText="";
  foreach (MySampleClass c in dictSampleClass)
  {
    sText = c.VAR1 + c.VAR2 + c.VAR3
    PerformSomeTask(sText,c.VAR4);
  }
}

My Question is, "Does above change improve performance? if yes, how?"

WOW thats more than expected response. Most guys said "C# compiler would take care of that". So what about c compiler??

Manjoor
  • 4,091
  • 10
  • 43
  • 67
  • 1
    I'd be very surprised, if this improved performance. But it hurts readability. – Henrik Jul 02 '10 at 08:56
  • 1
    There is a (negligible) performance HIT as you assign the empty string only to reassign immediately within the foreach. – Hans Kesting Jul 02 '10 at 08:59
  • 1
    Dupe: http://stackoverflow.com/questions/1884906/ – Nathan Baulch Jul 02 '10 at 09:13
  • 1
    You can avoid to set the sText to empty, since it is initialized anyway at every iteration. As others said, the compiler will produce the same code, but I find that declaring the variable outside the loop is cleaner. That said, note that concatenating 3 strings that way, produces 2 more strings than expected (tmp1=c.Var1+c.Var2 and tmp2=tmp1+c.Var3), so it could be worth (for the sake of using as few memory as needed) using a StringBuilder whose size is pre-initialized to (c.VAR1.Length+c.VAR2.Length+c.VAR3.Length). – BladeWise Jul 02 '10 at 09:50
  • Sorry, but why would the C compiler have anything to do with it? – Adam Houldsworth Jul 02 '10 at 10:20
  • @Nathan Baulch: Thanks for pointing out. I am sorry, i could not find that before asking :( – Manjoor Jul 02 '10 at 10:33
  • @Adam: Does it means any language compiler would react same ? – Manjoor Jul 02 '10 at 10:35
  • @Manjoor no its specific to the implementation of the compiler; however it is very likely that good compilers for popular languages have optimisations such as these. – Adam Houldsworth Jul 02 '10 at 10:52
  • 2
    You can answer this question yourself. **Try it both ways and see if the performance is different.** If you can't tell a difference, then *performance is not different*. The whole point of better performance is that it is observably better. – Eric Lippert Jul 02 '10 at 14:20

5 Answers5

7

The compiler has intelligence to move variable declarations into/out of loops where required. In your example however, you are using a string, which is immutable. By declaring it outside I believe you are trying to "create once, use many", however strings are created each time they are modified so you can't achieve that.

Not to sound harse, but this is a premature optimisation, and likely a failing one - due to string immutability.

If the collection is large, go down the route of appending many strings into a StringBuilder - separated by a delimiter. Then split the string on this delimiter and iterate the array to add them, instead of concatenating them and adding them in one loop.

StringBuilder sb = new StringBuilder();

foreach (MySampleClass c in dictSampleClass)
{
    sb.Append(c.VAR1);
    sb.Append(c.VAR2);
    sb.Append(c.VAR3);
    sb.Append("|");
}

string[] results = sb.ToString().Split('|');

for (int i = 0; i < dictSampleClass.Count; i++)
{
    string s = results[i];
    MySampleClass c = dictSampleClass[i];
    PerformSomeTask(s,c.VAR4); 
}

I infer no benefits to using this code - most likely doesn't even work!

UPDATE: in light of the fast performance of string creation from multiple strings, if PerformSomeTask is your bottleneck, try to break the iteration of the strings into multiple threads - it won't improve the efficiency of the code but you'll be able to utilise multiple cores.

Adam Houldsworth
  • 63,413
  • 11
  • 150
  • 187
2

Run the two functions through reflector and have a look at the generated assembly. It might give some insights but at best, the performance improval would be minimal.

cristobalito
  • 4,192
  • 1
  • 29
  • 41
1

I'd go for this instead:

private void TestFunction() 
{ 
  foreach (MySampleClass c in dictSampleClass) 
  { 
    PerformSomeTask(c.VAR1 + c.VAR2 + c.VAR3, c.VAR4); 
  } 
} 

There's still probably no real performance benefit, but it removes creating a variable that you don't really need.

cjk
  • 45,739
  • 9
  • 81
  • 112
0

No, it does not. Things like these are handled by the C# compiler which is very inteligent nad you basicaly do not need to care about these details.

Aways use code that promotes readability.

You can check this by disassembling the program.

0

Ii will not improve performance. But on another note: Assumed improvements are error prone. You have to measure you application and only optimise the slow part if needed. I don't believe that loop is your appliacations's bottleneck.

aggsol
  • 2,343
  • 1
  • 32
  • 49
  • Not safe to assume that either, 10k items with string concatenations in a tight loop - could be memory hungry. – Adam Houldsworth Jul 02 '10 at 09:17
  • 1
    @Adam: It's not really a string concatenation in a loop, its a string initialisation from multiple sources. Its not the same as `String str = c.VAR1; str += c.VAR2; str += c.VAR3` which would cause performance problems. – cjk Jul 02 '10 at 10:22
  • @ck true, I've never considered the performance differences of that. – Adam Houldsworth Jul 02 '10 at 10:25