2

to make it clear, does below code perform similarly or the latter might be faster? will the CLR cache the loop up result from a dictionary? I know we should use switch in this particular case, but would be good if I can know better what the CLR will do about dictionary look up result caching

            foreach (string columnName in csvReader.ReadFields())
            {
                if (FieldType.Int == fieldTypes[columnName])
                {
                    //do something
                }
                else if (FieldType.Double == fieldTypes[columnName])
                {
                    //do something
                }
                else if (FieldType.Datetime == fieldTypes[columnName])
                {
                    //do something
                }
                .....
                .....
                .....
            }

vs

            foreach (string columnName in csvReader.ReadFields())
            {
                FieldType fieldType = fieldTypes[columnName];
                if (FieldType.Int == fieldType  )
                {
                    //do something
                }
                else if (FieldType.Double == fieldType)
                {
                    //do something
                }
                else if (FieldType.Datetime == fieldType )
                {
                    //do something
                }
                .....
                .....
                .....
            }
John
  • 2,107
  • 3
  • 22
  • 39
  • Compiler will optimize the first one to the second one. – ikh Nov 07 '14 at 10:54
  • @ikh: any proof links? – Dennis Nov 07 '14 at 10:57
  • 1
    The compiler will definitely not do that. He has no idea what the indexer might do, or if it has any side-effects, so eliding 2 method calls is definitely not allowed. – dcastro Nov 07 '14 at 11:04
  • 1
    Common sub-expression elimination is one of the standard jitter optimizations. Even if it is misses the optimization for some reason, you'll never notice it since reading a CSV file takes much, *much* longer. Nanoseconds are not measurable when other code takes milliseconds. The perf of your program is completely dominated by I/O. Making it faster requires a better disk or network. – Hans Passant Nov 07 '14 at 11:06
  • @HansPassant What if the indexer has side-effects? Are you sure they can be eliminated? I know field reads/writes can, but I don't think method calls can... – dcastro Nov 07 '14 at 11:09
  • Inlining methods, especially indexers and property accessors, is another standard jitter optimization. But keep your eye on the ball, there's never a point to shave off nanoseconds when a program takes milliseconds to do its job. A million is a big number. – Hans Passant Nov 07 '14 at 11:12
  • @HansPassant Yeah I realize it really doesn't matter here, I'm just trying to understand what can and cannot happen, you know, for knowledge's sake :) – dcastro Nov 07 '14 at 11:20
  • If you really need to know which is faster, you should try both options and see if you can tell the difference. – Gabe Nov 07 '14 at 13:06

1 Answers1

2

Indexers are methods, and method calls cannot be elided. So no, the compiler/runtime won't merge 3 method calls into one. (Update: some indexers might be inlined and then elided if they're simple enough, but that's not likely to happen with Dictionary's indexer).

However, dictionaries perform random access in constant O(1) time, so performance really isn't an issue here (unless we're talking about micro-optimizations, where the constant factor might matter).

I would go for the most readable approach - which, IMO, is the second one.

dcastro
  • 66,540
  • 21
  • 145
  • 155
  • Removing method calls is a standard jitter optimizer feature. Note the MethodImplOption.NoInlining enumeration value, used to suppress the optimization. Almost any property or indexer method call will be inlined, unless it is too untrivial. The indexer for Dictionary was intentionally written to be inlined, note the use of the ThrowHelper class. – Hans Passant Nov 07 '14 at 11:19
  • @HansPassant I thought `ThrowHelper`'s purpose was [to reduce IL code size](http://stackoverflow.com/a/766179/857807). How does it help with inlining? – dcastro Nov 07 '14 at 11:25
  • Methods that throw an exception directly are not inlined. Since that makes it too difficult to diagnose the exception from the stack trace. – Hans Passant Nov 07 '14 at 11:28
  • @HansPassant Aaah, makes sense, inlining a method that throws an exception would ruin the stack trace. But say the indexer really was inlined (I think it's much too complex to be inlined, but let's assume it is). Wouldn't JIT be allowed to remove variable reads/stores *only* (as long as those reads/stores comply with the rules of the [memory model](http://joeduffyblog.com/2007/11/10/clr-20-memory-model/))? Removing entire methods seems too much to me... – dcastro Nov 07 '14 at 11:33
  • Sure, if the optimizer discovers that the inlined indexer/property has no useful side-effect then it eliminates it completely. And sure, won't happen for the Dictionary indexer since it contains calls to non-trivial internal methods. – Hans Passant Nov 07 '14 at 11:37
  • @HansPassant I see, thanks :) I've amended by answer. – dcastro Nov 07 '14 at 11:44