-
Notifications
You must be signed in to change notification settings - Fork 127
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
.NET 3.5 via MonoMod.Backports #540
Conversation
Backports has a few of these shims already, on |
Yes, I intend to only include MonoMod.Backports when absolutely necessary (i.e., only for the .NET 3.5 target). Using the existing shims for |
/// <summary> | ||
/// Gets a value indicating whether the current runtime runs on the Microsoft Windows operating system. | ||
/// </summary> | ||
public static bool IsRunningOnWindows | ||
{ | ||
get; | ||
} | ||
|
||
/// <summary> | ||
/// Gets a value indicating whether the current runtime runs on a Unix-based operating system. | ||
/// </summary> | ||
public static bool IsRunningOnUnix | ||
{ | ||
get; | ||
} |
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 think it's more efficient to not cache the value because then the JIT can optimize branching based on runtime information.
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.
As long as the static constructor has run when the JIT compiles a method using it, it will see this as a constant (because it gets emitted as a static readonly
field, through a very small method body).
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'm pretty sure it will also depend on whether the calls to into RuntimeInformation
are considered JIT intrinsics, which I am not sure about.
/// <summary> | ||
/// Obtains a singleton instance of an empty array of the provided type. | ||
/// </summary> | ||
/// <typeparam name="T">The type to get the empty array for.</typeparam> | ||
/// <returns>The empty array.</returns> | ||
#if !NET35 | ||
public static T[] Empty<T>() => Array.Empty<T>(); | ||
#else | ||
public static T[] Empty<T>() => ArrayHelper<T>.Empty; | ||
|
||
private static class ArrayHelper<T> | ||
{ | ||
public static readonly T[] Empty = new T[0]; | ||
} | ||
#endif |
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.
Collection expressions might be supported on NET35. If so, I think you could use []
everywhere instead of having a shim.
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 think that when Array.Empty<T>()
is not available, []
will always construct a new array. Probably worth double checking though.
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.
This is indeed the case. On .NET 3.5 the following C#
public static int[] ReturnEmpty() => [];
results in the following CIL, creating a new empty array on every call.
.method public static hidebysig int32[] ReturnEmpty() cil managed
{
.maxstack 8
IL_0000: ldc.i4.0
IL_0001: newarr valuetype [mscorlib] System.Int32
IL_0006: ret
}
MemoryMappedFile for .NET Framework 3.5
IInputFile::BaseAddress
MemoryMappedDataSource
, the new implementation usesUnmanagedDataSource
.Closes #536