Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.

Improve the performance of Environment.WorkingSet in Windows #26522

Merged
merged 13 commits into from
Sep 12, 2019
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// 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 partial class Interop
{
[StructLayout(LayoutKind.Sequential)]
internal struct ProcessMemoryCounters
sywhang marked this conversation as resolved.
Show resolved Hide resolved
{
public uint cb;
public uint PageFaultCount;
public ulong PeakWorkingSetSize;
sywhang marked this conversation as resolved.
Show resolved Hide resolved
public ulong WorkingSetSize;
public ulong QuotaPeakPagedPoolUsage;
public ulong QuotaPagedPoolUsage;
public ulong QuotaPeakNonPagedPoolUsage;
public ulong QuotaNonPagedPoolUsage;
public ulong PagefileUsage;
public ulong PeakPagefileUsage;
sywhang marked this conversation as resolved.
Show resolved Hide resolved
}

internal partial class Kernel32
{
[DllImport(Libraries.Kernel32, SetLastError = true)]
sywhang marked this conversation as resolved.
Show resolved Hide resolved
internal static extern bool K32GetProcessMemoryInfo(IntPtr handleProcess, out ProcessMemoryCounters pmCounter, uint cb);
sywhang marked this conversation as resolved.
Show resolved Hide resolved
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -1054,6 +1054,7 @@
<Compile Include="$(MSBuildThisFileDirectory)Interop\Windows\Kernel32\Interop.GetLongPathNameW.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Interop\Windows\Kernel32\Interop.GetLogicalDrives.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Interop\Windows\Kernel32\Interop.GetProcessTimes.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Interop\Windows\Kernel32\Interop.GetProcessMemoryInfo.cs" />
sywhang marked this conversation as resolved.
Show resolved Hide resolved
<Compile Include="$(MSBuildThisFileDirectory)Interop\Windows\Kernel32\Interop.GetSystemDirectoryW.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Interop\Windows\Kernel32\Interop.GetSystemInfo.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Interop\Windows\Kernel32\Interop.GetSystemTime.cs" />
Expand Down
19 changes: 19 additions & 0 deletions src/System.Private.CoreLib/shared/System/Environment.Unix.cs
Original file line number Diff line number Diff line change
Expand Up @@ -444,5 +444,24 @@ private static int CheckedSysConf(Interop.Sys.SysConfName name)
}
return (int)result;
}

public static long WorkingSet
{
get
{
Type? processType = Type.GetType("System.Diagnostics.Process, System.Diagnostics.Process, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a", throwOnError: false);
if (processType?.GetMethod("GetCurrentProcess")?.Invoke(null, BindingFlags.DoNotWrapExceptions, null, null, null) is IDisposable currentProcess)
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to fix the perf of this one too? How much more expensive is this compared to Windows? Does the cost of this show up in the profiles when you turn on performance counters that call this again and again?

Copy link
Author

Choose a reason for hiding this comment

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

Ideally, yes. But my benchmarks using BenchmarkDotNet suggested that Linux is at a better state than Windows (>10x) so for now I'm fixing Windows first.

Linux:

|     Method |     N |     Mean |    Error |   StdDev | Rank |
|----------- |------ |---------:|---------:|---------:|-----:|
| workingset | 10000 | 143.4 us | 2.520 us | 2.801 us |    1 |

Windows (before the fix):

|     Method |     N |     Mean |     Error |    StdDev | Rank |
|----------- |------ |---------:|----------:|----------:|-----:|
| workingset | 10000 | 1.620 ms | 0.0153 ms | 0.0127 ms |    1 |

In addition to this, the customer requesting this fix is Bing and they are running on Windows.

That being said, the Linux overhead is also huge compared to what we have on Windows after this fix so ideally we'd like to fix it on Linux too:

|     Method |     N |     Mean |     Error |    StdDev |   Median | Rank |
|----------- |------ |---------:|----------:|----------:|---------:|-----:|
| workingset | 10000 | 851.6 ns | 16.916 ns | 34.556 ns | 835.9 ns |    1 |

{
using (currentProcess)
{
object? result = processType!.GetMethod("get_WorkingSet64")?.Invoke(currentProcess, BindingFlags.DoNotWrapExceptions, null, null, null);
if (result is long) return (long)result;
}
}

// Could not get the current working set.
return 0;
}
}
}
}
15 changes: 15 additions & 0 deletions src/System.Private.CoreLib/shared/System/Environment.Windows.cs
Original file line number Diff line number Diff line change
Expand Up @@ -119,5 +119,20 @@ public static string SystemDirectory
return builder.ToString();
}
}

public static unsafe long WorkingSet
{
get
{
Interop.ProcessMemoryCounters memoryCounters;
memoryCounters.cb = (uint)(sizeof(Interop.ProcessMemoryCounters));

if (!Interop.Kernel32.K32GetProcessMemoryInfo(Interop.Kernel32.GetCurrentProcess(), out memoryCounters, memoryCounters.cb))
{
return 0;
}
return (long)memoryCounters.WorkingSetSize;
}
}
}
}
23 changes: 0 additions & 23 deletions src/System.Private.CoreLib/shared/System/Environment.cs
Original file line number Diff line number Diff line change
Expand Up @@ -153,29 +153,6 @@ public static Version Version
}
}

public static long WorkingSet
{
get
{
// Use reflection to access the implementation in System.Diagnostics.Process.dll. While far from ideal,
// we do this to avoid duplicating the Windows, Linux, macOS, and potentially other platform-specific implementations
// present in Process. If it proves important, we could look at separating that functionality out of Process into
// Common files which could also be included here.
Type? processType = Type.GetType("System.Diagnostics.Process, System.Diagnostics.Process, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a", throwOnError: false);
if (processType?.GetMethod("GetCurrentProcess")?.Invoke(null, BindingFlags.DoNotWrapExceptions, null, null, null) is IDisposable currentProcess)
{
using (currentProcess)
{
object? result = processType!.GetMethod("get_WorkingSet64")?.Invoke(currentProcess, BindingFlags.DoNotWrapExceptions, null, null, null);
if (result is long) return (long)result;
}
}

// Could not get the current working set.
return 0;
}
}

private static bool ValidateAndConvertRegistryTarget(EnvironmentVariableTarget target)
{
Debug.Assert(target != EnvironmentVariableTarget.Process);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,4 +73,4 @@ protected override void OnEventCommand(EventCommandEventArgs command)
}
}
}
}
}
sywhang marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.


namespace System.Diagnostics.Tracing
{
internal sealed class RuntimeEventSourceHelper
Expand Down