-
Notifications
You must be signed in to change notification settings - Fork 508
Get CoreRT Hello World selfcontained binary under 1 MB #7962
Conversation
@@ -31,8 +31,8 @@ internal static class CommonRuntimeTypes | |||
internal static Type IntPtr { get { return s_intPtr; } } | |||
internal static Type Single { get { return s_single; } } | |||
internal static Type Double { get { return s_double; } } | |||
internal static Type Decimal { get { return s_decimal; } } | |||
internal static Type DateTime { get { return s_datetime; } } | |||
internal static Type Decimal { get { return NotSoCommonTypes.s_decimal; } } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be better to deleted these and convert the few places that use it to use typeof
, e.g. if (type == typeof(Decimal))
.
Caching these in statics is actually a deoptimization.
The JIT has optimization for operator==
on System.Type that kicks in when one of the sides is typeof. It should compile this into a simple vtable compare.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be nice to port the fix for this deoptimization to CoreCLR as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I take it, the comment at the top of this class only applies for .NET Native?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I actually have a hard time finding the places where RyuJIT would be able to optimize the type comparison - most of these are used in deep reflection code where we have a System.Type
from who knows where that we want to compare to a well known one, or we want to grab a specific System.Type
to return.
I think typeof is still more expensive on CoreRT than on CoreCLR. It's at minimum a hashtable lookup. CoreCLR can do it faster for some reason.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CommonTypes.Decimal
is used in a single place in
corert/src/System.Private.CoreLib/src/Internal/Reflection/Augments/ReflectionAugments.cs
Line 90 in 09682e3
if (type.Equals(CommonRuntimeTypes.Decimal)) |
If you change this if (type == typeof(Decimal))
, the RyuJIT should optimize it to a simple cmp
instruction (look for CORINFO_INTRINSIC_TypeEQ
).
I am not suggestion to get rid of this class completely. I agree that it is not straightforward to get rid of this completely. I am just suggestion to get rid of the ones you are moving to NotSoCommonTypes
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The equivalent in CoreCLR that can use the same fix is here: https://github.com/dotnet/runtime/blob/master/src/libraries/System.Private.CoreLib/src/System/RuntimeType.cs#L192
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmm, the optimization is not as good as I though. It is not able to get rid of the materializing of the Type object.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmm, the optimization is not as good as I though. It is not able to get rid of the materializing of the Type object.
We could probably come up with a scheme for RyuJIT to be able optimize this.
typeof(X)
expands to a set of calls "load native TypeHandle
→ get RuntimeTypeHandle
from native TypeHandle
-> Type.GetTypeFromHandle
. RyuJIT optimizes this entire sequence into a single CORINFO_HELP_TYPEHANDLE_TO_RUNTIMETYPE
helper call that takes the native TypeHandle
as an argument.
RyuJIT can then optimize this to a simple compare if the other side of the comparison is a another typeof
or someObject.GetType()
. But that's not the case here.
If we could add a helper to compare native TypeHandle
with an arbitrary System.Type
, we could have RyuJIT call that in that case. That would help avoid materializing the System.Type
from the native TypeHandle
.
I could try to prototype that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dotnet/runtime#31686 is the JIT fix to optimize the pattern and make a lot of CommonRuntimeTypes
obsolete.
This has actually two things. The actual fix to make self-contained CoreRT Hello World less than 1 MB, and a bunch of tools that helped me find the fix. I’ve had the tools for a while but they don’t get that much use when they only exist on my machine.
The tools
I compiled an empty console app with CoreRT. I used the existing
<IlcGenerateMapFile>true</IlcGenerateMapFile>
property to create a map.xml file in the intermediate directory to lists all the things we compiled. I wondered whyDateTime
formatting is needed for an empty app.The existing map file is good to see what was generated, but not why it was generated.
The compiler builds a dependency graph internally that captures the whys. There has been a switch to dump the dependency graph into an XML file for a while - it just wasn’t publicly exposed. In this pull request I’m exposing it as
<IlcGenerateDgmlFile>true</ IlcGenerateDgmlFile>
. This generates up to two dgml.xml files in the intermediate directory (two if we’re running an optimized build with the scanner - in that case, the scan.dgml.xml file tends to be more useful).The next thing is interpreting the dependency graph. I wrote a tool for that some time ago, now I’m checking it in.
The input to the tool is the DGML file and name of a node of interest. The output is the list of reasons why that node was include.
It looks like this for the DateTime formatting case:
The reason boils down to pretty much this:
The
CommonRuntimeTypes
class has atypeof
reference toDateTime
. This means the compiler needs to assumeDateTime
was boxed (we don’t do precise analysis of what happened to thetypeof
and one might just pass the result of thetypeof
toFormatterServices.GetUninitializedObject
or reflection, which means an instance of the type could get allocated even without an explicit box anywhere in the code).Okay, so
DateTime
is considered boxed, what’s the big deal?The reason number 2 ("Secondary") listed in the dump is that
ISpanFormattable.TryFormat
is called somewhere. SinceDateTime
implementsISpanFormattable
, we need to generate the implementations for that because bad things would happen at runtime if someone calledISpanFormattable
methods on the boxedDateTime
and we don’t have the code for that.The fix
To break this kind of dependency (virtual/interface method call), we need to break one of the two reasons - either get rid of the object allocation, or get rid of the interface call.
Getting rid of the allocation is pretty straightforward - don’t reference
DateTime
from theCommonRuntimeTypes
class. That type is used in codepaths that are more rare.Makes the AOT compiled empty program go from 1,113,600 bytes to 962,560 bytes.