20

In the build settings panel of VS2010 Pro, there is a CheckBox with the label "optimize code"... of course, I want to check it... but being unusually cautious, I asked my brother about it and he said that it is unchecked for debugging and that in C++ it can potentially do things that would break or bug the code... but he doesn't know about C#.

So my question is, can I check this box for my release build without worrying about it breaking my code? Second, if it can break code, when and why? Links to explanations welcome.

The CheckBox I am talking about.

Nasreddine
  • 36,610
  • 17
  • 75
  • 94
Joshua W
  • 1,103
  • 12
  • 29
  • possible duplicate of [Performance differences between debug and release builds](http://stackoverflow.com/questions/4043821/performance-differences-between-debug-and-release-builds) – Hans Passant Dec 11 '11 at 20:10

9 Answers9

25

You would normally use this option in a release build. It's safe and mainstream to do so. There's no reason to be afraid of releasing code with optimizations enabled. Enabling optimization can interfere with debugging which is a good reason to disable it for debug builds.

David Heffernan
  • 601,492
  • 42
  • 1,072
  • 1,490
  • 2
    IIRC there are some edge conditions when variable removal can cause floating point calculations to give different values (due to not forcing it down from the native size) – Marc Gravell Dec 11 '11 at 20:12
  • 1
    @Marc It's common to see differences in floating point code with optimisers due to, for example, (a+b)+c being not equal to a+(b+c) and other such quirks of FP. Nothing to worry about. – David Heffernan Dec 11 '11 at 20:14
  • 3
    Its not that simple. Whether or not the jitter will enable the optimizer is determined primarily by whether or not a debugger is attached. Note Tools + Options, Debugging, General, "Suppress JIT optimization on module load" setting. Unticking it allows debugging optimized code. – Hans Passant Dec 11 '11 at 20:25
  • @hans ok, but that's a bit orthogonal to whether or not it is safe to use optimisations. – David Heffernan Dec 11 '11 at 20:30
  • Well, your answer was largely about debugging. The setting doesn't actually matter much. The only thing that doesn't work well is setting breakpoints on curly braces. – Hans Passant Dec 11 '11 at 20:40
  • I don't think the optimizer is allowed to change `(a+b)+c` into `a+(b+c)` – H H Dec 11 '11 at 20:57
  • @henk no I'm sure you are right. I guess I'm thinking of issues with non deterministic ordering of a threaded accumulator. But my main point is that floating point calculations are inherently imprecise. – David Heffernan Dec 11 '11 at 21:04
  • @Henk No idea about C# semantics, but every optimizing C++ compiler does have compiler options that influence whether it is allowed to reorder floats and similar things. Since that can noticeably speedup some computations (at a small cost of precision) it's not that absurd. – Voo Dec 11 '11 at 22:38
  • 4
    It isn't related to evaluation order. The problem is that the x86 FPU has a stack of registers with 80 bits of precision. The optimizer uses the stack to avoid storing back the result of calculations to memory. Much more efficient but the intermediate results don't get truncated back to 64 bits. Thus changing the calculation result. Not an issue for the x64 jitter, it uses the XMM registers instead which are 64 bits. It sounded like a good idea at the time :) – Hans Passant Dec 11 '11 at 22:50
16

The optimizations shouldn't really break your code. There's a post here by Eric Lippert which explains what happens when you turn that flag on. The performance gain will vary from application to application, so you'll need to test it with your project to see if there are any noticeable differences (in terms of performance).

keyboardP
  • 68,824
  • 13
  • 156
  • 205
7

It is possible that some bugs will occur when running in release mode that do not otherwise occur. The infamous "non-volatile flag" comes to mind:

flag = false;

Thread t = new Thread(
   o =>
   {
        while(!flag)
        {
           // do stuff
        }
   });
t.Start();

// main thread does some work

flag = true;
t.Join(); // will never return in release mode if flag is not volatile

This happens because of compiler optimizations, as the flag variable gets cached by the core of thread t and thus it cannot see the updated value of flag.

Tudor
  • 61,523
  • 12
  • 102
  • 142
  • 3
    That code is just broken. It works in debug by chance. In release mode your luck runs out. – David Heffernan Dec 11 '11 at 20:20
  • 1
    @David Heffernan: Mmmm, I don't see it as being broken. Why do you think so? This is a well-known compiler/CPU reorder/caching problem. – Tudor Dec 11 '11 at 20:21
  • So you've provided a specific example of something that could go awry... this holds true for .NET 4? – Joshua W Dec 11 '11 at 20:23
  • @watkinsj: Yes. In fact this generally happens in all languages, including C, Java, etc. or it might even be caused by the CPU itself sometimes. – Tudor Dec 11 '11 at 20:24
  • 1
    @tudor are you suggesting that disabling optimisation guarantees correctness of this code and is a alternative to the appropriate use of volatile? – David Heffernan Dec 11 '11 at 20:28
  • 1
    @David Heffernan: I am not suggesting this. Without a doubt optimizations are always desirable. It was more a reply to the statement that "optimizing can't break your code". It can break the code if you're naive enough to think that instructions actually execute the way you think they do. – Tudor Dec 11 '11 at 20:31
  • 4
    The code is broken independent of any compiler flags. Even in debug mode this may cause problems (just as it may be perfectly fine in optimized code). Can enabling optimizations make some bugs more noticeable? Sure, but it won't break valid code. – Voo Dec 11 '11 at 22:08
  • 3
    I see it as broken precisely *because* it's a well-known compiler/CPU reorder/caching problem. There is no reason why that code should ever return without changing `flag` to being volatile or inserting `Thread.MemoryBarrier()`. Getting lucky with a debug build means a bug was hidden, not absent. – Jon Hanna Dec 11 '11 at 23:06
  • Interesting debate we have here. :) In my opinion, it's a "chicken or the egg problem", because volatile is used to counteract the effect of caching/reordering done by optimization. So in a sense, we are saying: my code should work how I'm thinking, but I need volatile to be sure optimizations don't mess it up. – Tudor Dec 12 '11 at 09:02
  • This debate implies the programmer is aware the code will be multi-threaded and that optimization can cache loop test variables. The latter is a lot less obvious and is definitely a new invention. Back in Delphi, the use of "Terminated" to check if TThread has ended was/is common (don't know if they added 'volatile' of their own nowadays though) - it was not considered 'broken', there was no 'volatile' keyword; the optimizer caching or whatever it did never broke code as far as I know. C# one does because you now have to use 'volatile' manually, and now optimizer can do more things. – TByte May 03 '22 at 09:36
  • @Voo you say this code is broken even with optimization off? How so? Why would modifying memory on one thread not show when another thread is reading it? The only issue here is surely the optimization feature that caches that particular loop test variable. It does not read like broken code, it is simply out-dated with regards to now needing the 'volatile' flag to deal with new optimizers. Back when 'volatile' was NOT available in languages, this sort of code would be just fine. I think your comment gives off a smell. Volatile was not here not that long ago. – TByte May 03 '22 at 09:47
  • @TByte "this code is broken even with optimization off? How so?" Because the C# specification says so. You have to learn to separate implementation details ("it works on this and this compiler with those settings") from what is guaranteed by the language ("10.5.3 guarantees XYZ"). Compilers do **not** break valid code (well not intentionally at least). Volatile has been available since C# 1.0, although C#'s memory model was rather badly specified back in the days. Whether delphi uses different semantics is irrelevant for C#. – Voo May 03 '22 at 10:01
  • @Voo Sorry, let me rephrase, you say "The code is broken independent of any compiler flags. Even in debug mode this may cause problems (just as it may be perfectly fine in optimized code)" - how in debug mode can that code be problematic? IME the only thing that breaks that code is turning on optimization. As I stated before, this code runs fine in other languages that don't have 'volatile' keyword/feature (if they broke such code by optimization being turned on, they'd become very unpopular). – TByte May 04 '22 at 15:45
  • @TByte I'm afraid there's too much to explain for in comments. But to reiterate: You have to analyse code according to the language specification and not some particular implementation. Implementations can change from one day to the next, your code may run on a different runtime in the future and so on. If you want to guarantee that your program will work correctly under all circumstances you must ensure that the intended behavior is guaranteed by the specification. – Voo May 04 '22 at 21:50
  • (For example the given code could break if you run it on an ISA with weaker memory model than x86 or x64, particularly if you were to use it on a non SMP machine - rven in debug). Also since you're harking on about delphi not having volatile: Writing multi threaded code that worked correctly on delphi required you to explicitly write the memory barriers. It's just that delphi code generally only runs on x86/x64 machines which hides many bugs and most multi threading concurrency bugs are subtle enough that you might never notice them. – Voo May 04 '22 at 21:55
4

Should optimisations introduce bugs? No.

Could optimisations introduce bugs? Maybe, nothing's perfect after all.

Could optimsations uncover bugs that were always in your code, but are hidden when they are turned off? Absolutely, happens quite a bit.

The important thing is to realise that it's a change. Just like you'd test if you'd done a lot of changes, you should test when you turn them off. If final-release will have them turned on, then final-test must have them turned on too.

Jon Hanna
  • 110,372
  • 10
  • 146
  • 251
1

Example wise i have a piece of code from some simulation parts of my master thesis. In which with the optimization flag turned on the code don't really break the program, but the pathfinder only performs one run and loops. (the recursive code traps itself in a loop on the pathfinder which it always breaks out of with the optimization flag turned off).

So yes it is possible for the optimization flag to make the software behave differently.

Thomas Andreè Wang
  • 3,379
  • 6
  • 37
  • 53
1

In C# the optimization should NEVER break your code.

Instead, with optimizations turned on the compiler produces more compact CIL when translating between C# and CIL.

I observed (and frankly it's interesting!) that the C# compilers from .NET < 2.0 (1.0 and 1.1) produced as good CIL WITHOUT optimizations as later C# compilers (2.0 and later) produce WITH optimizations.

Wiktor Zychla
  • 47,367
  • 6
  • 74
  • 106
  • 1
    Does anyone else believe the quality of codegen of the C# compiler has regressed? – David Heffernan Dec 11 '11 at 20:31
  • 1
    anything specific in your CIL discussion (last line)? – Marc Gravell Dec 11 '11 at 20:33
  • 3
    Can you give an example for the lower CIL quality? How do you even define "good CIL"? – CodesInChaos Dec 11 '11 at 20:46
  • David: There are couple of new translation schemes. For example, the compiler now stores most of intermediate calculations in auxiliary local variables. Then, the control flow contains more jumps, sometimes hard to explain (like br to next instruction). I could provide examples but it's easy to craft a simple method, just one/two 'ifs' and compare the output of the two compilers. – Wiktor Zychla Dec 11 '11 at 20:50
  • Are you judging quality based on ease of understanding for you or runtime performance? – David Heffernan Dec 11 '11 at 20:53
  • David: neither. I think you missed my last comment. – Wiktor Zychla Dec 11 '11 at 20:58
  • No, that was in response to your comment. Quality of codegen surely has to be judged by correctness first and timings second. – David Heffernan Dec 11 '11 at 21:12
  • It takes 10 minutes to see that the 1.1 compiler produces comparable code with/without optimizations where the 2.0 compiler introduces at least a) an auxiliary local variable for each expression b) a "single ret" even if it means the br from the previous instruction. The C# 1.1 without optimizations will then often beat C# 2.0 without optimizations in the performance because of the bloated code produced by the latter. Is your point that you don't believe in facts I talk about or you would like to point out I am wrong in some other sense? – Wiktor Zychla Dec 11 '11 at 21:39
  • 2
    @Wiktor: Judging performance on what the IL looks like is crazy. It's the jitted code that actually gets executed, not IL. Have you considered the possibility that the "bloated" code that you describe, with extra locals etc, might be *easier* for the jitter to work with and might actually result in native code that performs *better*? – LukeH Dec 11 '11 at 21:47
  • LukeH: I don't know much about JIT optimizations but when the optimization is turned off (i.e. if you run in debug mode) JIT translates CIL very naively. It means that all the bloated code is still there. I don't think that what you write is true (bloated CIL could be easier to JIT) because it would question all the optimizations at the CIL level applied when you compile C# with optimizations turned on. – Wiktor Zychla Dec 11 '11 at 21:56
  • **Nobody** cares - or at least should care - about the performance of non-JITed code. Instead of looking at the IL which is irrelevant, it's much simpler to just measure the differences - even looking at the generated x86 assembly won't necessarily tell us much (e.g.: Which is faster: `xor eax, eax` or `sub eax, eax` to set a register to 0 and why?) - modern CPUs are tremendously difficult to analyze. – Voo Dec 11 '11 at 22:13
  • There are no relevant facts here. Relevant facts would be realistic benchmarks with repeatable timings. – David Heffernan Dec 12 '11 at 07:13
  • I don't blame the C# compiler for doing anything wrong. And yes, it is relevant to compare the quality of generated CIL, because if there isn't then why should be care about ANY optimizations done by the C# compiler during C#-CIL translation? Also, I am perfectly aware of the rationale behind these new translation schemes, I've studied it for few weeks as a part of larger project. To summarize - I wrote "good CIL" as an indication that it's bloated while you guys insist that the only possible criteria is the performance. – Wiktor Zychla Dec 12 '11 at 10:02
0

.net compiler optimization could cause bugs. happend to me today. took me a few hours to nail it. the code is:

for (int i = 0; i < list.Count-1; i++) {
  list[i+1].DoSomeThing();
  //some code
  if (someCondition) {
    list.insert(i+1, new Item());
    i++;
  }
}

at some point, the list[i+1] is addressed as list[i], as if both both point to the same item. this bug was so wierd. the code ran well at debug mode, and at release mode, but when I ran it out side visual studio, ex. from the .exe file, the code crashed. only turning off the compiler optimization fixed it.

Yomi1984
  • 165
  • 2
  • 10
0

In my case when I had the optimizations flag turned on it would not complete all the operations so there were measuring points missing in the final result so I simply turned the optimization flag off to fix the bug:

using System.Threading.Tasks;

                Parallel.Invoke(
                    async () => await ProcessPartialArrayOperationAssets(operationAssets, 0, operationAssets.Count / 2,
                        operations, inspection1),
                    async () => await ProcessPartialArrayOperationAssets(operationAssets, operationAssets.Count / 2,
                        operationAssets.Count, operations, inspection1)
                );

private async Task ProcessPartialArrayInspectionOperations(IList<InspectionOperation> operations,
    int begin,
    int end,
    Inspection inspection,
    InspectionAsset inspectionAsset)
{
    await Task.Run(() =>
    {
        // create one new operation measuring point for each measuring point in the operation's equipment
        int itemCounter = begin + 1;

        for (int i = begin; i < end; i++)
        {
            lock (_thisLock)
            {
                InspectionOperation operation = operations[i];
                int itemNumber = 1;

                // get the asset
                InspectionAsset operationAsset = operation.OperationAsset;
                if (operationAsset != null)
                {
                    // get the measuring points
                    string ABAPTrue = Abap.ABAP_TRUE;

                    lock (_thisLock)
                    {
                        IList<MeasuringPoint> measuringPoints = DbContext.MeasuringPoints.Where(x =>
                                x.AssetID == operationAsset.AssetID && x.InactiveFlag != ABAPTrue)
                            .ToList();

                        if (measuringPoints != null)
                        {
                            //Debug.WriteLine("measuringPoints.Count = " + measuringPoints.Count);

                            // create the operation measuring points
                            foreach (MeasuringPoint measuringPoint in measuringPoints)
                            {
                                OperationMeasuringPoint operationMeasuringPoint =
                                    new OperationMeasuringPoint
                                    {
                                        InspectionID = inspection.InspectionID,
                                        OperationNumber = operation.OperationNumber,
                                        SubActivity = "",
                                        RoutingNo = "",
                                        ItemNumber = itemNumber.ToString("D4"),
                                        // e.g. "0001", "0002" and so on
                                        ItemCounter = itemCounter.ToString("D8"),
                                        // e.g. "00000001", "00000002" and so on
                                        MeasuringPointID = measuringPoint.MeasuringPointID,
                                        MeasuringPointDescription = measuringPoint.Description,
                                        Equipment = inspectionAsset.AssetID,
                                        Category = "P"
                                    };
                                DbContext.Entry(operationMeasuringPoint).State = EntityState.Added;
                                itemNumber++;
                                itemCounter++;
                            }
                        }
                    }
                }
            }
        }
    });
}

Thus I replaced the Parallel.Invoke call with this as well. FYI, this problem occurred using .NET Framework 4.7.

await ProcessPartialArrayOperationAssets(operationAssets, 0, operationAssets.Count, operations, inspection1);

UPDATE:

OK, I've found that I was able to re-enable the optimization flag and use Parallel.Invoke if I remove the async Task from the method signature:

    private void ProcessPartialArrayInspectionOperations(IList<InspectionOperation> operations,
        int begin,
        int end,
        Inspection inspection,
        InspectionAsset inspectionAsset)
    {
        // create one new operation measuring point for each measuring point in the operation's equipment
        int itemCounter = begin + 1;

        for (int i = begin; i < end; i++)
        {

            InspectionOperation operation = operations[i];
            int itemNumber = 1;

            // get the asset
            InspectionAsset operationAsset = operation.OperationAsset;
            if (operationAsset != null)
            {
                // get the measuring points
                string ABAPTrue = Abap.ABAP_TRUE;

                lock (_thisLock)
                {
                    IList<MeasuringPoint> measuringPoints = DbContext.MeasuringPoints.Where(x =>
                            x.AssetID == operationAsset.AssetID && x.InactiveFlag != ABAPTrue)
                        .ToList();

                    if (measuringPoints != null)
                    {
                        //Debug.WriteLine("measuringPoints.Count = " + measuringPoints.Count);

                        // create the operation measuring points
                        foreach (MeasuringPoint measuringPoint in measuringPoints)
                        {
                            OperationMeasuringPoint operationMeasuringPoint =
                                new OperationMeasuringPoint
                                {
                                    InspectionID = inspection.InspectionID,
                                    OperationNumber = operation.OperationNumber,
                                    SubActivity = "",
                                    RoutingNo = "",
                                    ItemNumber = itemNumber.ToString("D4"),
                                    // e.g. "0001", "0002" and so on
                                    ItemCounter = itemCounter.ToString("D8"),
                                    // e.g. "00000001", "00000002" and so on
                                    MeasuringPointID = measuringPoint.MeasuringPointID,
                                    MeasuringPointDescription = measuringPoint.Description,
                                    Equipment = inspectionAsset.AssetID,
                                    Category = "P"
                                };
                            DbContext.Entry(operationMeasuringPoint).State = EntityState.Added;
                            itemNumber++;
                            itemCounter++;
                        }
                    }
                }
            }
        }
    }

                        Parallel.Invoke(
                            () => ProcessPartialArrayInspectionOperations(operations, 0, operations.Count / 2,
                                inspection1, inspectionAsset),
                            () => ProcessPartialArrayInspectionOperations(operations, operations.Count / 2,
                                operations.Count, inspection1, inspectionAsset)
                        );

Alternatively, I think I could use Task.Run for each and then a await Task.WhenAll(t1, t2, t3); as explained here, but in this case I am not making explicit database calls so I don't think it applies to use Task.Run instead of Parallel.Invoke though this page does explain why my Parallel.Invoke was not completing: Parallel.Invoke does not wait for async methods to complete

For details, please see "Concurrency in C#" https://stephencleary.com/book/

user8128167
  • 6,929
  • 6
  • 66
  • 79
0

I saw most that most upvoted answers claim that turning on optimization will not break your code. I disagree. I initially wrote this as a comment to DH's answer, but I would like to repeat it here.

On recent work on a very messy C# project, I advocated to turn on optimization. Things did break. Optimization broke loop code like this while (!someBool), where the variable was being set by another thread or background worker. On one hand I blame the code (not so much!), on the other, I blame the optimizer for this decision. I would not have expected this behaviour from say, a Delphi optimizer, which is considered 'safe'. The solution was to add the volatile keyword against someBool to ensure it was not cached (into a register presumably) and never read again by the loop. volatile ensures the code reads the contents of someBool every iteration.

This unfortunate incident eroded any trust my boss had in the idea of turning on optimization, and it put enough doubt in my mind such that I can't guarantee that turning on optimization will not break any other piece of code! Thus, that project still has its optimization off. Better safe than sorry.

TByte
  • 121
  • 1
  • 5