45

I am updating some old code, and have found several instances where the same object is being cast repeatedly each time one of its properties or methods needs to be called. Example:

if (recDate != null && recDate > ((System.Windows.Forms.DateTimePicker)ctrl).MinDate)
{
    ((System.Windows.Forms.DateTimePicker)ctrl).CustomFormat = "MM/dd/yyyy";
    ((System.Windows.Forms.DateTimePicker)ctrl).Value = recDate;
}
else
{
    (System.Windows.Forms.DateTimePicker)ctrl).CustomFormat = " ";
}
((System.Windows.Forms.DateTimePicker)ctrl).Format = DateTimePickerFormat.Custom;

My inclination is to fix this monstrosity, but given my limited time I don't want to bother with anything that's not affecting functionality or performance.

So what I'm wondering is, are these redundant casts getting optimized away by the compiler? I tried figuring it out myself by using ildasm on a simplified example, but not being familiar with IL I only ended up more confused.

UPDATE

So far, the consensus seems to be that a)no, the casts are not optimized, but b)while there may possibly be some small performance hit as a result, it is not likely significant, and c)I should consider fixing them anyway. I have come down on the side of resolving to fix these someday, if I have time. Meanwhile, I won't worry about them.

Thanks everyone!

Cody Gray - on strike
  • 239,200
  • 50
  • 490
  • 574
allonym
  • 1,408
  • 10
  • 18
  • 1
    Interesting question, indeed: However, perform a performance analysis -- I highly doubt this use of casting noticeably affect the application performance -- even if not optimized out -- simply based on suspected invocation count (and thus removing the casts would be simply "fix[ing] this monstrosity", although I do like the nod to real-world constraints ;-) –  Mar 11 '11 at 21:47
  • 3
    Where is the check to see if ctrl is a DateTimePicker (or is it just assumed to be)? Generally with a situation like this you want to cast once (DateTimePicker dtp = ctrl as DateTimePicker), then check if the result is not null, and then just use the casted object from that point on. – Brandon Moretz Mar 11 '11 at 21:50
  • @Brandon - this is within a giant if/else if chain, e.g. if (ctrl is DateTimePicker). Not pretty, but at least not dangerous. What you propose is what I would like to do, but am reluctant because of the time constraint (there are hundreds of these). – allonym Mar 11 '11 at 21:52

4 Answers4

21

A spot check on the generated machine code in the Release build shows that the x86 jitter doesn't optimize the cast away.

You have to look at the big picture here though. You are assigning properties of a control. They have a ton of side-effects. In the case of DateTimePicker, the assignment results in a message being sent to the native Windows control. Which in turn crunches away at the message. The cost of the cast is negligible to the cost of the side effects. Rewriting the assignments is never going to make a noticeable difference in speed, you only made it a fraction of a percent faster.

Go ahead and do rewrite the code on a lazy Friday afternoon. But only because it is a blight to readability. That poorly readable C# code also produces poorly optimized machine code is not entirely a coincidence.

Hans Passant
  • 922,412
  • 146
  • 1,693
  • 2,536
  • If I could mark multiple answers, I would have accepted yours as well as Nathan's. Thanks! – allonym Mar 12 '11 at 00:52
  • What opcodes get generated for the exception? I'd expect nothing more than a check that it isn't null, then a check against the type -- maybe 10ns at most. – Gabe Mar 12 '11 at 08:03
  • @Gabe - there's a bunch of code generated, but it indeed first checks if the object is an exact match. If not, it uses a CLR helper function to check the type hierarchy. There's a possible branch misprediction, but the OP's specific case costs less than one nanosecond in most cases. – Hans Passant Mar 12 '11 at 13:27
  • No-one else has asked this I assume because they all know, but, how are you performing the "spot check on the generated machine code"? – J M Mar 13 '11 at 17:23
  • @J M: you can look at the generated machine code with the Go To Disassembly context menu command. To make this accurate, use Tools + Options, Debugging, General, untick "Suppress JIT optimization on module load". – Hans Passant Mar 13 '11 at 17:52
  • It's not completly valid imo. Cast from `T` to `T` is removed from the code. I needed this info while writing T4 template, and I had multiple warnings for redundant casts. Fortuly, they are removed from release code by csc.exe so nothing to be worried about. – Alex Zhukovskiy Jan 28 '16 at 16:26
19

It is not optimized away from IL in either debug or release builds.

simple C# test:

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;

namespace RedundantCastTest
{
    class Program
    {
        static object get()
        { return "asdf"; }

        static void Main(string[] args)
        {
            object obj = get();
            if ((string)obj == "asdf")
                Console.WriteLine("Equal: {0}, len: {1}", obj, ((string)obj).Length);
        }
    }
}

Corresponding IL (note the multiple castclass instructions):

.method private hidebysig static void Main(string[] args) cil managed
{
    .entrypoint
    .maxstack 3
    .locals init (
        [0] object obj,
        [1] bool CS$4$0000)
    L_0000: nop 
    L_0001: call object RedundantCastTest.Program::get()
    L_0006: stloc.0 
    L_0007: ldloc.0 
    L_0008: castclass string
    L_000d: ldstr "asdf"
    L_0012: call bool [mscorlib]System.String::op_Equality(string, string)
    L_0017: ldc.i4.0 
    L_0018: ceq 
    L_001a: stloc.1 
    L_001b: ldloc.1 
    L_001c: brtrue.s L_003a
    L_001e: ldstr "Equal: {0}, len: {1}"
    L_0023: ldloc.0 
    L_0024: ldloc.0 
    L_0025: castclass string
    L_002a: callvirt instance int32 [mscorlib]System.String::get_Length()
    L_002f: box int32
    L_0034: call void [mscorlib]System.Console::WriteLine(string, object, object)
    L_0039: nop 
    L_003a: ret 
}

Neither is it optimized from the IL in the release build:

.method private hidebysig static void Main(string[] args) cil managed
{
    .entrypoint
    .maxstack 3
    .locals init (
        [0] object obj)
    L_0000: call object RedundantCastTest.Program::get()
    L_0005: stloc.0 
    L_0006: ldloc.0 
    L_0007: castclass string
    L_000c: ldstr "asdf"
    L_0011: call bool [mscorlib]System.String::op_Equality(string, string)
    L_0016: brfalse.s L_0033
    L_0018: ldstr "Equal: {0}, len: {1}"
    L_001d: ldloc.0 
    L_001e: ldloc.0 
    L_001f: castclass string
    L_0024: callvirt instance int32 [mscorlib]System.String::get_Length()
    L_0029: box int32
    L_002e: call void [mscorlib]System.Console::WriteLine(string, object, object)
    L_0033: ret 
}

Neither case means that the casts don't get optimized when native code is generated - you'd need to look at the actual machine assembly there. i.e. by running ngen and disassembling. I'd be greatly surprised if it wasn't optimized away.

Regardless, I'll cite The Pragmatic Programmer and the broken window theorem: When you see a broken window, fix it.

ChrisF
  • 134,786
  • 31
  • 255
  • 325
Nathan Ernst
  • 4,540
  • 25
  • 38
  • Thanks for the answer. With all due deference to The Pragmatic Programmer, this window may not be truly broken. It might just have ugly window dressing:p – allonym Mar 11 '11 at 22:04
  • Even if it's not optimized out of the IL, that doesn't mean that it slows down execution. – Gabe Mar 11 '11 at 22:33
  • Conventional wisdom (as I learned it) is that casts are expensive. Am I living in the past? (also, note that I do not consider performance as limited to speed. memory is a concern) – allonym Mar 11 '11 at 22:36
  • @allonym - Casts are moderately expensive. Not as bad as dynamic dispatching, though. And in the grand scheme of things it's probably not anything close to being the bottleneck in your application. Still, I'd be tempted to fix it. – Steve Wortham Mar 11 '11 at 22:59
  • What about memory? Notice that boxing of integer type? Now with c# 6.0 we have string interpolation and calling `ToString()` method of int type which is redundant do escapes this boxing and unboxing or am I wrong? – kuskmen Dec 22 '16 at 10:32
6

No; FxCop flags this as a performance warning. See info here: http://msdn.microsoft.com/en-us/library/ms182271.aspx

I'd recommend running that over your code if you want to find things to fix.

Mark Sowul
  • 10,244
  • 1
  • 45
  • 51
1

I have never heard of or seen redundant cast optimizations on the CLR. Lets try a contrived example

object number = 5;
int iterations = 10000000;
int[] storage = new int[iterations];

var sw = Stopwatch.StartNew();
for (int i = 0; i < iterations; i++) {
    storage[i] = ((int)number) + 1;
    storage[i] = ((int)number) + 2;
    storage[i] = ((int)number) + 3;
}
Console.WriteLine(sw.ElapsedTicks);

storage = new int[iterations];

sw = Stopwatch.StartNew();
for (int i = 0; i < iterations; i++) {
    var j = (int)number;
    storage[i] = j + 1;
    storage[i] = j + 2;
    storage[i] = j + 3;
}
Console.WriteLine(sw.ElapsedTicks);
Console.ReadLine();

On my machine, running under release, I am consistantly getting about 350k ticks for redundant redundancy and 280k ticks for self optimzation. So no, it looks like the CLR does not optimize for this.

Bob
  • 97,670
  • 29
  • 122
  • 130
  • that's interesting. it doesn't comport with StackOverflowException's observation, though. – allonym Mar 11 '11 at 21:58
  • 6
    Your example is too contrived. You are converting from an `object` to an `int` which performs an `unbox` operation. The OP's example used a `castclass` operation, which is completely different. – Gabe Mar 11 '11 at 22:35
  • It was admittedly contrived but the point still stands, redundant casts are not optimized for any types. I also tested reference types and saw the same result, just with a smaller difference. The point still stands. – Bob Mar 14 '11 at 12:51