2
\$\begingroup\$

Please review the two code snippets below and let me know if the second snippet is better than the first one.

In code snippet 2 I have added an inline function. According to my peers, using inline functions can help improve performance, since context switch will occur less often.

But I am not able to verify this claim. Is there any perfomance difference in the two code snippets?

Snippet 1

foreach (var item in requestPayLoad.UserData)
{
    UserItem userItem = UserItemHelper.CreateItem(item, jobID);
    if (userItem != null)
    {
        lstUserItems.Add(userItem);
    }
}

Snippet 2

var GetUserItem = new Func<UserItem , UserJob , UserItemStruct>((custItemStruct, syncJob) =>
                  {
                      return UserItemHelper.CreateItem(custItemStruct, syncJob);
                  });

foreach (var item in requestPayLoad.UserData)
{
    UserItem userItem = GetUserItem(item,job);
    if (userItem != null)
    {
        lstUserItems.Add(userItem);
    }
}
\$\endgroup\$
  • 2
    \$\begingroup\$ Have you tried profiling these code snippets to find any performance differences? \$\endgroup\$ – dreza Jul 2 '12 at 8:11
  • 1
    \$\begingroup\$ The jit compiler will inline for you when it can. Don't write your code in such a way to hinder its functionality. \$\endgroup\$ – Jeff Mercado Jul 2 '12 at 9:09
12
\$\begingroup\$

You seem to be confusing several concepts, some completely unrelated.

First, the second snippet is less readable, because it adds quite a lot of complexity that's not necessary. Unless you have a good reason to do something like this (for example, significant measurable performance improvements), don't do it.

Now to the confusions: what you're doing could be called an inline function. But the performance improvements your peers were most likely talking about are from inlining functions. Inlining functions is an optimization made by the jitter, which sometimes inlines the machine code of one function into another function. This optimization is completely unrelated to creating a local delegate from lambda. And actually, by doing this you are preventing that optimization. Also, invoking a delegate has some small cost, so your second snippet will be actually slower.

To sum up, the second snippet is less readable and less efficient: I don't see any reason why should you ever write code like that. In any case, trying to optimize code like this is pointless: optimize only the code that's worth optimizing (identified by profiling) and then use empirical data to see which version is actually faster. Don't guess, measure.

You also seem to be confused about what context switch is. Context switch happens when your CPU is executing code from one thread, and then switches to executing code from another thread. This is completely unrelated to your question.

\$\endgroup\$
10
\$\begingroup\$

For what it's worth, you could re-write this as

lstUserItems.AddRange(requestPayLoad.UserData
    .Select(item => UserItemHelper.CreateItem(item, jobID))
    .Where(userItem => userItem != null));
\$\endgroup\$
0
\$\begingroup\$

The second code snippet is a really dirty one becouse you loose the control over your code (reuseability, unit test (for this you should rethink the concept of UserItemHelper), readability).

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.