From 4b8dad55879eeb4b5c5baedc194c94f4250016d9 Mon Sep 17 00:00:00 2001 From: Haipz Date: Tue, 12 Nov 2024 15:46:47 +0800 Subject: [PATCH 1/3] Cache current process object to avoid performance hit (#5597) * Read working set from Environment in ProcessInfo since it has better performance. * Add unit test for ProcessInfo. * Remove OSSkipCondition tag from process info unit test since it's cross-platform. * Use Environment.WorkingSet in GetMemoryUsageInBytes. --- .../Windows/Interop/ProcessInfo.cs | 4 ++-- .../Windows/WindowsSnapshotProvider.cs | 3 +-- .../Windows/ProcessInfoTests.cs | 23 +++++++++++++++++++ 3 files changed, 26 insertions(+), 4 deletions(-) create mode 100644 test/Libraries/Microsoft.Extensions.Diagnostics.ResourceMonitoring.Tests/Windows/ProcessInfoTests.cs diff --git a/src/Libraries/Microsoft.Extensions.Diagnostics.ResourceMonitoring/Windows/Interop/ProcessInfo.cs b/src/Libraries/Microsoft.Extensions.Diagnostics.ResourceMonitoring/Windows/Interop/ProcessInfo.cs index cb5febeff5..fb5223f3d0 100644 --- a/src/Libraries/Microsoft.Extensions.Diagnostics.ResourceMonitoring/Windows/Interop/ProcessInfo.cs +++ b/src/Libraries/Microsoft.Extensions.Diagnostics.ResourceMonitoring/Windows/Interop/ProcessInfo.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System; using System.Diagnostics; using System.Diagnostics.CodeAnalysis; @@ -41,7 +42,6 @@ internal sealed class ProcessInfo : IProcessInfo public ulong GetCurrentProcessMemoryUsage() { - using Process process = Process.GetCurrentProcess(); - return (ulong)process.WorkingSet64; + return (ulong)Environment.WorkingSet; } } diff --git a/src/Libraries/Microsoft.Extensions.Diagnostics.ResourceMonitoring/Windows/WindowsSnapshotProvider.cs b/src/Libraries/Microsoft.Extensions.Diagnostics.ResourceMonitoring/Windows/WindowsSnapshotProvider.cs index 7197499afd..da828a2d06 100644 --- a/src/Libraries/Microsoft.Extensions.Diagnostics.ResourceMonitoring/Windows/WindowsSnapshotProvider.cs +++ b/src/Libraries/Microsoft.Extensions.Diagnostics.ResourceMonitoring/Windows/WindowsSnapshotProvider.cs @@ -109,8 +109,7 @@ internal sealed class WindowsSnapshotProvider : ISnapshotProvider internal static long GetMemoryUsageInBytes() { - using var process = Process.GetCurrentProcess(); - return process.WorkingSet64; + return Environment.WorkingSet; } internal static ulong GetTotalMemoryInBytes() diff --git a/test/Libraries/Microsoft.Extensions.Diagnostics.ResourceMonitoring.Tests/Windows/ProcessInfoTests.cs b/test/Libraries/Microsoft.Extensions.Diagnostics.ResourceMonitoring.Tests/Windows/ProcessInfoTests.cs new file mode 100644 index 0000000000..ab83f2677d --- /dev/null +++ b/test/Libraries/Microsoft.Extensions.Diagnostics.ResourceMonitoring.Tests/Windows/ProcessInfoTests.cs @@ -0,0 +1,23 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Microsoft.Extensions.Diagnostics.ResourceMonitoring.Windows.Interop; +using Microsoft.TestUtilities; +using Xunit; + +namespace Microsoft.Extensions.Diagnostics.ResourceMonitoring.Windows.Test; + +/// +/// Process Info Interop Tests. +/// +/// These tests are added for coverage reasons, but the code doesn't have +/// the necessary environment predictability to really test it. +public sealed class ProcessInfoTests +{ + [ConditionalFact] + public void GetCurrentProcessMemoryUsage() + { + var workingSet64 = new ProcessInfo().GetCurrentProcessMemoryUsage(); + Assert.True(workingSet64 > 0); + } +} From c77e368808f613cca0f94606a37c847571277797 Mon Sep 17 00:00:00 2001 From: Stephen Toub Date: Tue, 12 Nov 2024 11:02:32 -0500 Subject: [PATCH 2/3] Fix namespace for IServiceCollection extensions (#5620) We generally put extension methods into the same namespace as the thing they're extending. --- .../ChatClientBuilderServiceCollectionExtensions.cs | 4 ++-- .../EmbeddingGeneratorBuilderServiceCollectionExtensions.cs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Libraries/Microsoft.Extensions.AI/ChatCompletion/ChatClientBuilderServiceCollectionExtensions.cs b/src/Libraries/Microsoft.Extensions.AI/ChatCompletion/ChatClientBuilderServiceCollectionExtensions.cs index 246ac7f368..9d419f434a 100644 --- a/src/Libraries/Microsoft.Extensions.AI/ChatCompletion/ChatClientBuilderServiceCollectionExtensions.cs +++ b/src/Libraries/Microsoft.Extensions.AI/ChatCompletion/ChatClientBuilderServiceCollectionExtensions.cs @@ -2,10 +2,10 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; -using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.AI; using Microsoft.Shared.Diagnostics; -namespace Microsoft.Extensions.AI; +namespace Microsoft.Extensions.DependencyInjection; /// Provides extension methods for registering with a . public static class ChatClientBuilderServiceCollectionExtensions diff --git a/src/Libraries/Microsoft.Extensions.AI/Embeddings/EmbeddingGeneratorBuilderServiceCollectionExtensions.cs b/src/Libraries/Microsoft.Extensions.AI/Embeddings/EmbeddingGeneratorBuilderServiceCollectionExtensions.cs index 369de130e7..4f2eddf6b1 100644 --- a/src/Libraries/Microsoft.Extensions.AI/Embeddings/EmbeddingGeneratorBuilderServiceCollectionExtensions.cs +++ b/src/Libraries/Microsoft.Extensions.AI/Embeddings/EmbeddingGeneratorBuilderServiceCollectionExtensions.cs @@ -2,10 +2,10 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; -using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.AI; using Microsoft.Shared.Diagnostics; -namespace Microsoft.Extensions.AI; +namespace Microsoft.Extensions.DependencyInjection; /// Provides extension methods for registering with a . public static class EmbeddingGeneratorBuilderServiceCollectionExtensions From 6487428046a2d7a5c8bb6034e97c38d6e0b7affa Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Tue, 12 Nov 2024 16:37:57 +0000 Subject: [PATCH 3/3] Fix linker warning. (#5627) --- .../Utilities/AIJsonUtilities.Schema.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Libraries/Microsoft.Extensions.AI.Abstractions/Utilities/AIJsonUtilities.Schema.cs b/src/Libraries/Microsoft.Extensions.AI.Abstractions/Utilities/AIJsonUtilities.Schema.cs index 195cf062eb..4e3f90aa47 100644 --- a/src/Libraries/Microsoft.Extensions.AI.Abstractions/Utilities/AIJsonUtilities.Schema.cs +++ b/src/Libraries/Microsoft.Extensions.AI.Abstractions/Utilities/AIJsonUtilities.Schema.cs @@ -302,7 +302,7 @@ public static partial class AIJsonUtilities _ = objSchema.TryGetPropertyValue(RequiredPropertyName, out JsonNode? required); if (required is not JsonArray { } requiredArray || requiredArray.Count != propertiesObj.Count) { - requiredArray = [.. propertiesObj.Select(prop => prop.Key)]; + requiredArray = [.. propertiesObj.Select(prop => (JsonNode)prop.Key)]; objSchema[RequiredPropertyName] = requiredArray; } }