Skip to content
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.

Sign up for GitHub

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

Use GeneratedDllImport in System.Diagnostics.FileVersionInfo and System.Runtime.InteropServices.RuntimeInformation #52739

Merged
merged 3 commits into from
May 20, 2021
Merged

Use GeneratedDllImport in System.Diagnostics.FileVersionInfo and System.Runtime.InteropServices.RuntimeInformation #52739

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
30b04c0
Convert System.Diagnostics.FileVersionInfo
elinor-fung May 14, 2021
00b9f84
Convert System.Runtime.InteropServices.RuntimeInformation
elinor-fung May 14, 2021
4f060ff
Use SYSTEM_INFO* instead of GeneratedDllImport for GetSystemInfo/GetN…
elinor-fung May 20, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/libraries/Common/src/Interop/Unix/System.Native/Interop.GetUnixVersion.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ internal static partial class Interop
{
internal static partial class Sys
{
[DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_GetUnixVersion", CharSet = CharSet.Ansi, SetLastError = true)]
private static extern int GetUnixVersion(byte[] version, ref int capacity);
[GeneratedDllImport(Libraries.SystemNative, EntryPoint = "SystemNative_GetUnixVersion", CharSet = CharSet.Ansi, SetLastError = true)]
private static partial int GetUnixVersion(byte[] version, ref int capacity);

internal static string GetUnixVersion()
{
Expand Down
12 changes: 6 additions & 6 deletions src/libraries/Common/src/Interop/Unix/System.Native/Interop.Stat.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,13 +53,13 @@ internal enum FileStatusFlags
HasBirthTime = 1,
}

[DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_FStat", SetLastError = true)]
internal static extern int FStat(SafeHandle fd, out FileStatus output);
[GeneratedDllImport(Libraries.SystemNative, EntryPoint = "SystemNative_FStat", SetLastError = true)]
internal static partial int FStat(SafeHandle fd, out FileStatus output);

[DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_Stat", SetLastError = true)]
internal static extern int Stat(string path, out FileStatus output);
[GeneratedDllImport(Libraries.SystemNative, EntryPoint = "SystemNative_Stat", CharSet = CharSet.Ansi, SetLastError = true)]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is CharSet = CharSet.Ansi needed here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We had the p/invoke source generators avoid making any assumptions about CharSet if it isn't set, so it requires explicitly specifying either CharSet or MarshalAs when marshalling string/char.

internal static partial int Stat(string path, out FileStatus output);

[DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_LStat", SetLastError = true)]
internal static extern int LStat(string path, out FileStatus output);
[GeneratedDllImport(Libraries.SystemNative, EntryPoint = "SystemNative_LStat", CharSet = CharSet.Ansi, SetLastError = true)]
internal static partial int LStat(string path, out FileStatus output);
}
}
2 changes: 1 addition & 1 deletion src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetNativeSystemInfo.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,6 @@ internal static partial class Interop
internal static partial class Kernel32
{
[DllImport(Libraries.Kernel32)]
internal static extern void GetNativeSystemInfo(out SYSTEM_INFO lpSystemInfo);
internal static unsafe extern void GetNativeSystemInfo(SYSTEM_INFO* lpSystemInfo);
}
}
2 changes: 1 addition & 1 deletion src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetSystemInfo.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,6 @@ internal static partial class Interop
internal static partial class Kernel32
{
[DllImport(Libraries.Kernel32)]
internal static extern void GetSystemInfo(out SYSTEM_INFO lpSystemInfo);
internal static unsafe extern void GetSystemInfo(SYSTEM_INFO* lpSystemInfo);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is unsafe change necessary? (it is still using the old DllImport)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The parameter was switched to be a pointer - SYSTEM_INFO*

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes but since title is "Use GeneratedDllImport..". this change and changes due to this change looked unrelated.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The general goal of GeneratedDllImport (and this feature branch) is to remove p/invoke overhead - whether that is through using the DllImport source generator to generate the code or using it to find places where we could easily make the p/invoke inlinable.

}
}
4 changes: 2 additions & 2 deletions src/libraries/Common/src/Interop/Windows/Version/Interop.GetFileVersionInfoEx.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ internal static partial class Interop
{
internal static partial class Version
{
[DllImport(Libraries.Version, CharSet = CharSet.Unicode, EntryPoint = "GetFileVersionInfoExW")]
internal static extern bool GetFileVersionInfoEx(
[GeneratedDllImport(Libraries.Version, CharSet = CharSet.Unicode, EntryPoint = "GetFileVersionInfoExW")]
internal static partial bool GetFileVersionInfoEx(
uint dwFlags,
string lpwstrFilename,
uint dwHandle,
Expand Down
4 changes: 2 additions & 2 deletions src/libraries/Common/src/Interop/Windows/Version/Interop.GetFileVersionInfoSizeEx.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ internal static partial class Interop
{
internal static partial class Version
{
[DllImport(Libraries.Version, CharSet = CharSet.Unicode, EntryPoint = "GetFileVersionInfoSizeExW")]
internal static extern uint GetFileVersionInfoSizeEx(uint dwFlags, string lpwstrFilename, out uint lpdwHandle);
[GeneratedDllImport(Libraries.Version, CharSet = CharSet.Unicode, EntryPoint = "GetFileVersionInfoSizeExW")]
internal static partial uint GetFileVersionInfoSizeEx(uint dwFlags, string lpwstrFilename, out uint lpdwHandle);
}
}
4 changes: 2 additions & 2 deletions src/libraries/Common/src/Interop/Windows/Version/Interop.VerQueryValue.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ internal static partial class Interop
{
internal static partial class Version
{
[DllImport(Libraries.Version, CharSet = CharSet.Unicode, EntryPoint = "VerQueryValueW")]
internal static extern bool VerQueryValue(IntPtr pBlock, string lpSubBlock, out IntPtr lplpBuffer, out uint puLen);
[GeneratedDllImport(Libraries.Version, CharSet = CharSet.Unicode, EntryPoint = "VerQueryValueW")]
internal static partial bool VerQueryValue(IntPtr pBlock, string lpSubBlock, out IntPtr lplpBuffer, out uint puLen);
}
}
5 changes: 4 additions & 1 deletion ...s/System.IO.MemoryMappedFiles/src/System/IO/MemoryMappedFiles/MemoryMappedView.Windows.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,10 @@ public unsafe void Flush(UIntPtr capacity)
private static int GetSystemPageAllocationGranularity()
{
Interop.Kernel32.SYSTEM_INFO info;
Interop.Kernel32.GetSystemInfo(out info);
unsafe
{
Interop.Kernel32.GetSystemInfo(&info);
}

return (int)info.dwAllocationGranularity;
}
Expand Down
7 changes: 6 additions & 1 deletion src/libraries/System.Private.CoreLib/src/System/Environment.Windows.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,12 @@ public static int SystemPageSize
{
get
{
Interop.Kernel32.GetSystemInfo(out Interop.Kernel32.SYSTEM_INFO info);
Interop.Kernel32.SYSTEM_INFO info;
unsafe
{
Interop.Kernel32.GetSystemInfo(&info);
}

return info.dwPageSize;
}
}
Expand Down
7 changes: 6 additions & 1 deletion src/libraries/System.Private.CoreLib/src/System/Runtime/MemoryFailPoint.Windows.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,12 @@ public sealed partial class MemoryFailPoint
{
private static ulong GetTopOfMemory()
{
Interop.Kernel32.GetSystemInfo(out Interop.Kernel32.SYSTEM_INFO info);
Interop.Kernel32.SYSTEM_INFO info;
unsafe
{
Interop.Kernel32.GetSystemInfo(&info);
}

return (ulong)info.lpMaximumApplicationAddress;
}

Expand Down
1 change: 1 addition & 0 deletions ...pServices.RuntimeInformation/src/System.Runtime.InteropServices.RuntimeInformation.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
Link="Common\Interop\Windows\Kernel32\Interop.GetSystemInfo.cs" />
</ItemGroup>
<ItemGroup>
<Reference Include="System.Memory" />
<Reference Include="System.Reflection" />
<Reference Include="System.Reflection.Extensions" />
<Reference Include="System.Runtime" />
Expand Down
14 changes: 12 additions & 2 deletions ...ation/src/System/Runtime/InteropServices/RuntimeInformation/RuntimeInformation.Windows.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,12 @@ public static Architecture OSArchitecture

if (osArch == -1)
{
Interop.Kernel32.GetNativeSystemInfo(out Interop.Kernel32.SYSTEM_INFO sysInfo);
Interop.Kernel32.SYSTEM_INFO sysInfo;
unsafe
{
Interop.Kernel32.GetNativeSystemInfo(&sysInfo);
}

osArch = s_osArch = (int)Map((Interop.Kernel32.ProcessorArchitecture)sysInfo.wProcessorArchitecture);
}

Expand All @@ -59,7 +64,12 @@ public static Architecture ProcessArchitecture

if (processArch == -1)
{
Interop.Kernel32.GetSystemInfo(out Interop.Kernel32.SYSTEM_INFO sysInfo);
Interop.Kernel32.SYSTEM_INFO sysInfo;
unsafe
{
Interop.Kernel32.GetSystemInfo(&sysInfo);
}

processArch = s_processArch = (int)Map((Interop.Kernel32.ProcessorArchitecture)sysInfo.wProcessorArchitecture);
}

Expand Down