views:

886

answers:

6

Please ignore code readability in this question.

In terms of performance, should the following code be written like this:

int maxResults = criteria.MaxResults;

if (maxResults > 0)
{
    while (accounts.Count > maxResults)
        accounts.RemoveAt(maxResults);
}

or like this:

if (criteria.MaxResults > 0)
{
    while (accounts.Count > criteria.MaxResults)
        accounts.RemoveAt(criteria.MaxResults);
}

?

Edit: criteria is a class, and MaxResults is a simple integer property (i.e., public int MaxResults { get { return _maxResults; } }.

Does the C# compiler treat MaxResults as a black box and evaluate it every time? Or is it smart enough to figure out that I've got 3 calls to the same property with no modification of that property between the calls? What if MaxResults was a field?

One of the laws of optimization is precalculation, so I instinctively wrote this code like the first listing, but I'm curious if this kind of thing is being done for me automatically (again, ignore code readability).

(Note: I'm not interested in hearing the 'micro-optimization' argument, which may be valid in the specific case I've posted. I'd just like some theory behind what's going on or not going on.)

+6  A: 

If MaxResults is a property then no, it will not optimize it, because the getter may have complex logic, say:

private int _maxResults;
public int MaxReuslts {
  get { return _maxResults++; }
  set { _maxResults = value; }
}

See how the behavior would change if it in-lines your code?

If there's no logic...either method you wrote is fine, it's a very minute difference and all about how readable it is TO YOU (or your team)...you're the one looking at it.

Nick Craver
Well even if it was a property it could be inlined but that would be done by the jit. That value could also be changed by another thread, so it may not be safe to optimize away.
Matthew Whited
btw, there's a typo in your [s]etter.
Vinzz
@Vinzz: Thanks!
Nick Craver
+1  A: 

why not test it?

just set up 2 console apps make it look 10 million times and compare the results ... remember to run them as properly released apps that have been installed properly or else you cannot gurantee that you are not just running the msil.

Really you are probably going to get about 5 answers saying 'you shouldn't worry about optimisation'. they clearly do not write routines that need to be as fast as possible before being readable (eg games).

If this piece of code is part of a loop that is executed billions of times then this optimisation could be worthwhile. For instance max results could be an overridden method and so you may need to discuss virtual method calls.

Really the ONLY way to answer any of these questions is to figure out is this is a piece of code that will benefit from optimisation. Then you need to know the kinds of things that are increasing the time to execute. Really us mere mortals cannot do this a priori and so have to simply try 2-3 different versions of the code and then test it.

John Nicholas
boom and sure enough i anticipated the readability comment ... he even used bold ;p
John Nicholas
A: 

If criteria is a class type, I doubt it would be optimized, because another thread could always change that value in the meantime. For structs I'm not sure, but my gut feeling is that it won't be optimized, but I think it wouldn't make much difference in performance in that case anyhow.

herzmeister der welten
There's no reason that a struct could not be modified in another thread in pretty much the same way a class can. structs aren't guaranteed to be immutable, and they can be passed by reference.
Thorarin
That's very true. Mutable structs are so evil, I never use them. Thus I almost forgot about that case. So my gut feeling finally becomes a veritable wisdom. ;-)
herzmeister der welten
+4  A: 

It will be called and evaluated every time. The compiler has no way of determining if a method (or getter) is deterministic and pure (no side effects).

Note that actual evaluation of the property may be inlined by the JIT compiler, making it effectively as fast as a simple field.

It's good practise to make property evaluation an inexpensive operation. If you do some heavy calculation in the getter, consider caching the result manually, or changing it to a method.

Thorarin
+6  A: 

Your two code samples are only guaranteed to have the same result in single-threaded environments, which .Net isn't, and if MaxResults is a field (not a property). The compiler can't assume, unless you use the synchronization features, that criteria.MaxResults won't change during the course of your loop. If it's a property, it can't assume that using the property doesn't have side effects.

Eric Lippert points out quite correctly that it depends a lot on what you mean by "the compiler". The C# -> IL compiler? Or the IL -> machine code (JIT) compiler? And he's right to point out that the JIT may well be able to optimize the property getter, since it has all of the information (whereas the C# -> IL compiler doesn't, necessarily). It won't change the situation with multiple threads, but it's a good point nonetheless.

T.J. Crowder
+35  A: 

First off, the only way to actually answer performance questions is to actually try it both ways and test the results in realistic conditions.

That said, the other answers which say that "the compiler" does not do this optimization because the property might have side effects are both right and wrong. The problem with the question (aside from the fundamental problem that it simply cannot be answered without actually trying it and measuring the result) is that "the compiler" is actually two compilers: the C# compiler, which compiles to MSIL, and the JIT compiler, which compiles IL to machine code.

The C# compiler never ever does this sort of optimization; as noted, doing so would require that the compiler peer into the code being called and verify that the result it computes does not change over the lifetime of the callee's code. The C# compiler does not do so.

The JIT compiler might. No reason why it couldn't. It has all the code sitting right there. It is completely free to inline the property getter, and if the jitter determines that the inlined property getter returns a value that can be cached in a register and re-used, then it is free to do so. (If you don't want it to do so because the value could be modified on another thread then you already have a race condition bug; fix the bug before you worry about performance.)

Whether the jitter actually does inline the property fetch and then enregister the value, I have no idea. I know practically nothing about the jitter. But it is allowed to do so if it sees fit. If you are curious about whether it does so or not, you can either (1) ask someone who is on the team that wrote the jitter, or (2) examine the jitted code in the debugger.

And finally, let me take this opportunity to note that computing results once, storing the result and re-using it is not always an optimization. This is a surprisingly complicated question. There are all kinds of things to optimize for:

  • execution time

  • executable code size -- this has a major effect on executable time because big code takes longer to load, increases the working set size, puts pressure on processor caches, RAM and the page file. Small slow code is often in the long run faster than big fast code in important metrics like startup time and cache locality.

  • register allocation -- this also has a major effect on execution time, particularly in architectures like x86 which have a small number of available registers. Enregistering a value for fast re-use can mean that there are fewer registers available for other operations that need optimization; perhaps optimizing those operations instead would be a net win.

  • and so on. It get real complicated real fast.

In short, you cannot possibly know whether writing the code to cache the result rather than recomputing it is actually (1) faster, or (2) better performing. Better performance does not always mean making execution of a particular routine faster. Better performance is about figuring out what resources are important to the user -- execution time, memory, working set, startup time, and so on -- and optimizing for those things. You cannot do that without (1) talking to your customers to find out what they care about, and (2) actually measuring to see if your changes are having a measurable effect in the desired direction.

Eric Lippert
+1 I was hoping to get your attention on this question. Thank you for a great answer.
Jon Seigel
Big +1 for pointing out that "the compiler" isn't really a meaningful term in .Net (or Java, for that matter, or any other environment that compiles to an intermediate format and then applies [may apply] JIT).
T.J. Crowder
@Eric: Your answers are always great. Without even seeing you name at the bottom of this answer I knew it had to be you. Thanks sharing the details of the inner workings of .Net and C# will all of us mortals.
Matthew Whited