Finish (mostly) erradicating StringBuilder marshaling from corefx (#33780)
authorStephen Toub <stoub@microsoft.com>
Tue, 4 Dec 2018 01:39:02 +0000 (20:39 -0500)
committerAnirudh Agnihotry <anirudhagnihotry098@gmail.com>
Tue, 4 Dec 2018 03:51:17 +0000 (19:51 -0800)
* Finish (mostly) erradicating StringBuilder marshaling from corefx

This should finish my quest of removing StringBuilder marshaling from coreclr and corefx, which I've strived to do because it often adds unnecessary overhead and often is done incorrectly because of intricacies in how the marshaling works; while not using it often leads to `unsafe` code at call sites, there's much less magic, and it affords the ability to optimize more if desired.

There are still three remaining [Out] StringBuilder's in Interop.Odbc.cs, which I've not removed because tests are limited and the call graph to these is non-trivial with StringBuilders passed through.  There may also be a few straggling DllImports I missed while scouring interop that's not following our guidelines of being in src\Common\.

Other than those, the remaining StringBuilder usage I found in DllImports was for [In] only, and in those cases it's reasonable, as the call sites are building up StringBuilders and then passing them off to the call sites; converting those to use `char*` or similar could actually make them more expensive.  I did, however, ensure they were properly annotated as [In], in order to make the intent clear and avoid potential marshaling costs for the unnecessary [Out] direction.

Where I was touching a function in one of these stray interop files that was duplicated elsewhere, I also consolidated it to a centralized location, but I've not in this PR done the cleanup work for the rest of the files.

* Address PR feedback

* Address further feedback

Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
src/System.Private.CoreLib/shared/Interop/Windows/Kernel32/Interop.Globalization.cs
src/System.Private.CoreLib/shared/Interop/Windows/Kernel32/Interop.ResolveLocaleName.cs [new file with mode: 0644]
src/System.Private.CoreLib/shared/System.Private.CoreLib.Shared.projitems

index 2227d59..644230c 100644 (file)
@@ -9,7 +9,6 @@ internal static partial class Interop
 {
     internal static unsafe partial class Kernel32
     {
-        internal const int  LOCALE_NAME_MAX_LENGTH      = 85;
         internal const uint LOCALE_ALLOW_NEUTRAL_NAMES  = 0x08000000; // Flag to allow returning neutral names/lcids for name conversion
         internal const uint LOCALE_SUPPLEMENTAL         = 0x00000002;
         internal const uint LOCALE_REPLACEMENT          = 0x00000008;
@@ -99,9 +98,6 @@ internal static partial class Interop
         internal delegate BOOL EnumLocalesProcEx(char* lpLocaleString, uint dwFlags, void* lParam);
 
         [DllImport("kernel32.dll", CharSet = CharSet.Unicode)]
-        internal static extern int ResolveLocaleName(string lpNameToResolve, char* lpLocaleName, int cchLocaleName);
-
-        [DllImport("kernel32.dll", CharSet = CharSet.Unicode)]
         internal static extern bool EnumTimeFormatsEx(EnumTimeFormatsProcEx lpTimeFmtEnumProcEx, string lpLocaleName, uint dwFlags, void* lParam);
   
         internal delegate BOOL EnumTimeFormatsProcEx(char* lpTimeFormatString, void* lParam);
diff --git a/src/System.Private.CoreLib/shared/Interop/Windows/Kernel32/Interop.ResolveLocaleName.cs b/src/System.Private.CoreLib/shared/Interop/Windows/Kernel32/Interop.ResolveLocaleName.cs
new file mode 100644 (file)
index 0000000..2d6b6c3
--- /dev/null
@@ -0,0 +1,17 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+using System;
+using System.Runtime.InteropServices;
+
+internal static partial class Interop
+{
+    internal static unsafe partial class Kernel32
+    {
+        internal const int LOCALE_NAME_MAX_LENGTH = 85;
+
+        [DllImport("kernel32.dll", CharSet = CharSet.Unicode)]
+        internal static extern int ResolveLocaleName(string lpNameToResolve, char* lpLocaleName, int cchLocaleName);
+    }
+}
index 45d6e48..fc59dfd 100644 (file)
     <Compile Include="$(MSBuildThisFileDirectory)Interop\Windows\Kernel32\Interop.OutputDebugString.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Interop\Windows\Kernel32\Interop.ReadFile_SafeHandle_IntPtr.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Interop\Windows\Kernel32\Interop.ReadFile_SafeHandle_NativeOverlapped.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)Interop\Windows\Kernel32\Interop.ResolveLocaleName.cs" Condition="'$(EnableDummyGlobalizationImplementation)' != 'true'" />
     <Compile Include="$(MSBuildThisFileDirectory)Interop\Windows\Kernel32\Interop.SECURITY_ATTRIBUTES.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Interop\Windows\Kernel32\Interop.SecurityOptions.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Interop\Windows\Kernel32\Interop.SetEndOfFile.cs" />