Massimiliano Mantione's Blog

Chasing a ghost, and Microsoft JIT inlining behavior

Finally, the tree mover (and all the optimizations described here) is in svn, but that optimization set is not enabled by default.

The problem (which showed up as soon as I committed the patch) is that inlining breaks several pieces of code, most notably MonoDevelop.

All the issues are related to calls to MethodBase.GetCurrentMethod(), Assembly.GetExecutingAssembly(), Assembly.GetCallingAssembly() and any of the StackFrame constructors (thanks to lupus for this list!), and are related to the fact that inlining a method alters the chain of stack frames.

To fix this in a reliable way I wrote a small program to use as regression test, you can find it here. It must be split in two assemblies otherwise most of the tests don't make sense, but it is really easy to understand.
It also immediately shows the problems: with Mono from current svn, executing with "-O=-inline" prints five "PASSED" messages, which turn into five "FAILED" ones with "-O=inline".

So far, so good... Initially it seemed that a good solution would be checking if any of those methods was called, and if it was, refuse to inline the caller. However, after a few trials and errors, I found out that the proper fix is inlining only leaf methods (I mean, methods which don't call any other method), with the exception of internally generated wrappers, which can be inlined safely.
The rationale is the following: suppose you have a caller method CM, in assembly A, and a small called method SM, in assembly B. SM, then, calls another large method, LM (which for simplicity we think will never be inlined), which in turn calls for instance Assembly.GetCallingAssembly(). Now, if SM is inlined, it looks like if LM is called from CM (from assembly A), and not from SM (from assembly B): this makes so that the return value of Assembly.GetCallingAssembly() inside LM changes.
Note that inside SM there is nothing hinting for this, just a call to a method (LM). Finally note that this is true also if LM is a constructor, which means that also methods which call constructors cannot be inlined (which means any method that creates objects!).

To be fair, there is another reasoning we can follow which allows us to inline more methods: it is not the fact that a method merely contains a call or new CIL opcode that should prevent inlining. Instead, it is the fact that in the jitted code there's a call to potentially unknown code (either methods or constructors), which in turn could depend on the "correctness" of the chain of stack frames. This way of reasoning would allow us to inline nested calls, as long as none of them ends up calling a not-inlined method (or an inlined wrapper, because it will in the end call another piece of unknown code!).

But, regardless of the strategy we'll choose, fixing this is not hard, and in fact I did it quickly.
But even when in the regression test program I got all tests passing, the MonoDevelop splash screen was still not appearing, and scary warnings were logged every time I had inline enabled... To make a long story short, I wasted way too much time chasing a ghost: I was convinced that inline was doing something wrong (the problem was there only when inline was enabled), and I could not find what it was.
Then, trying to reproduce the problem with a smaller program, I got it: inline was doing nothing wrong, it was just exposing to the JIT the load of a static field a bit earlier. This static field was the SplashScreemForm singleton in MonoDevelop, and since the class is "beforefieldinit", the JIT initialized the field on the spot. SplashScreemForm is a Gtk.Window, so it got created before the GTK# initialization, and found itself without a display... bad thing.
However, the proper fix is just adding an empty static constructor to SplashScreemForm, so that it gets the strict field init semantics. The last line logged by the sample program I linked above shows exactly this kind of behavior.
And anybody making a GTK# Window a singleton should take note to remember to remove the "beforefieldinit" from the class, and access the singleton only after the GTK# Application has been initialized (I did the same mistake trying to reproduce the bug!).

So, in the end we'll have a JIT that always respects the structure of the call stack, but before you write code that needs this, think twice: the Microsoft runtime is not so strict ;-)
If you execute the test program on FX 1.1 or 2.0, you'll see that both of the Assembly.GetCallingAssembly() related tests fail (thanks to Sebastien for running the tests for me). The code pattern reproduced in the second one (in constructors) is sometimes used to load images from resources when building graphical objects, so code like this could work on Mono, and be broken on Windows...