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.

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

[AOT] Refactor Logger function to improve performance and mark managedCommon as AOT compatible #36327

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion src/common/ManagedCommon/LanguageHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public static string LoadLanguage()
inputStream.Close();
reader.Dispose();

return JsonSerializer.Deserialize<OutGoingLanguageSettings>(data).LanguageTag;
return JsonSerializer.Deserialize<OutGoingLanguageSettings>(data, ManagedCommonJsonSerializerContext.Default.OutGoingLanguageSettings).LanguageTag;
}
catch (Exception)
{
Expand Down
90 changes: 43 additions & 47 deletions src/common/ManagedCommon/Logger.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,14 @@ namespace ManagedCommon
{
public static class Logger
{
private static readonly Assembly Assembly = Assembly.GetExecutingAssembly();
private static readonly string Version = FileVersionInfo.GetVersionInfo(Assembly.Location).ProductVersion;

private static readonly string Error = "Error";
private static readonly string Warning = "Warning";
private static readonly string Info = "Info";
private static readonly string Debug = "Debug";
private static readonly string TraceFlag = "Trace";

private static readonly string Version = GetVersion();

/// <summary>
/// Initializes the logger and sets the path for logging.
/// </summary>
Expand Down Expand Up @@ -53,18 +52,37 @@ public static void InitializeLogger(string applicationLogPath, bool isLocalLow =
Trace.AutoFlush = true;
}

public static string GetVersion()

Choose a reason for hiding this comment

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

Can we just ask the caller to pass in its own class type?
From Github copilot, the following code should be AOT compatible:
var versionInfo = FileVersionInfo.GetVersionInfo(typeof(Program).Assembly.Location);
Console.WriteLine($"Version: {versionInfo.FileVersion}

{
string applicationName = "PowerToys.exe";
try
{
var versionInfo = FileVersionInfo.GetVersionInfo(Path.Combine(System.AppContext.BaseDirectory, applicationName));
if (versionInfo != null)
{
return versionInfo.ProductVersion;
}
}
catch (Exception)
{
return "0.0.0.1";
}

return "0.0.0.1";
}

[MethodImpl(MethodImplOptions.NoInlining)]
public static void LogError(string message)
public static void LogError(string message, [System.Runtime.CompilerServices.CallerMemberName] string memberName = "", [System.Runtime.CompilerServices.CallerFilePath] string sourceFilePath = "", [System.Runtime.CompilerServices.CallerLineNumber] int sourceLineNumber = 0)
{
Log(message, Error);
Log(message, Error, memberName, sourceFilePath, sourceLineNumber);
}

[MethodImpl(MethodImplOptions.NoInlining)]
public static void LogError(string message, Exception ex)
public static void LogError(string message, Exception ex, [System.Runtime.CompilerServices.CallerMemberName] string memberName = "", [System.Runtime.CompilerServices.CallerFilePath] string sourceFilePath = "", [System.Runtime.CompilerServices.CallerLineNumber] int sourceLineNumber = 0)
{
if (ex == null)
{
Log(message, Error);
Log(message, Error, memberName, sourceFilePath, sourceLineNumber);
}
else
{
Expand All @@ -83,38 +101,38 @@ public static void LogError(string message, Exception ex)
"Stack trace: " + Environment.NewLine +
ex.StackTrace;

Log(exMessage, Error);
Log(exMessage, Error, memberName, sourceFilePath, sourceLineNumber);
}
}

[MethodImpl(MethodImplOptions.NoInlining)]
public static void LogWarning(string message)
public static void LogWarning(string message, [System.Runtime.CompilerServices.CallerMemberName] string memberName = "", [System.Runtime.CompilerServices.CallerFilePath] string sourceFilePath = "", [System.Runtime.CompilerServices.CallerLineNumber] int sourceLineNumber = 0)
{
Log(message, Warning);
Log(message, Warning, memberName, sourceFilePath, sourceLineNumber);
}

[MethodImpl(MethodImplOptions.NoInlining)]
public static void LogInfo(string message)
public static void LogInfo(string message, [System.Runtime.CompilerServices.CallerMemberName] string memberName = "", [System.Runtime.CompilerServices.CallerFilePath] string sourceFilePath = "", [System.Runtime.CompilerServices.CallerLineNumber] int sourceLineNumber = 0)
{
Log(message, Info);
Log(message, Info, memberName, sourceFilePath, sourceLineNumber);
}

[MethodImpl(MethodImplOptions.NoInlining)]
public static void LogDebug(string message)
public static void LogDebug(string message, [System.Runtime.CompilerServices.CallerMemberName] string memberName = "", [System.Runtime.CompilerServices.CallerFilePath] string sourceFilePath = "", [System.Runtime.CompilerServices.CallerLineNumber] int sourceLineNumber = 0)
{
Log(message, Debug);
Log(message, Debug, memberName, sourceFilePath, sourceLineNumber);
}

[MethodImpl(MethodImplOptions.NoInlining)]
public static void LogTrace()
public static void LogTrace([System.Runtime.CompilerServices.CallerMemberName] string memberName = "", [System.Runtime.CompilerServices.CallerFilePath] string sourceFilePath = "", [System.Runtime.CompilerServices.CallerLineNumber] int sourceLineNumber = 0)
{
Log(string.Empty, TraceFlag);
Log(string.Empty, TraceFlag, memberName, sourceFilePath, sourceLineNumber);
}

[MethodImpl(MethodImplOptions.NoInlining)]

Choose a reason for hiding this comment

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

May be we don't need this one anymore.

private static void Log(string message, string type)
private static void Log(string message, string type, string memberName, string sourceFilePath, int sourceLineNumber)
{
Trace.WriteLine("[" + DateTime.Now.TimeOfDay + "] [" + type + "] " + GetCallerInfo());
Trace.WriteLine("[" + DateTime.Now.TimeOfDay + "] [" + type + "] " + GetCallerInfo(memberName, sourceFilePath, sourceLineNumber));
Trace.Indent();
if (message != string.Empty)
{
Expand All @@ -124,49 +142,27 @@ private static void Log(string message, string type)
Trace.Unindent();
}

[MethodImpl(MethodImplOptions.NoInlining)]
private static string GetCallerInfo()
{
StackTrace stackTrace = new();

var callerMethod = GetCallerMethod(stackTrace);

return $"{callerMethod?.DeclaringType?.Name}::{callerMethod.Name}";
}

private static MethodBase GetCallerMethod(StackTrace stackTrace)
private static string GetCallerInfo(string memberName, string sourceFilePath, int sourceLineNumber)

Choose a reason for hiding this comment

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

For sourceLineNumber.
Given originally they don't have, we need to ask for PMs, that any parser of the log it will break this.
If Yes, we should still keep the original "TypeName::MethodName" format first.

Last but not least, do we know how to view PowerToys log? If we can parrse the log and show a summary of before after, it will give confidence of this changes.

{
const int topFrame = 3;

var topMethod = stackTrace.GetFrame(topFrame)?.GetMethod();
string callerFileName = "Unknown";

try
{
if (topMethod?.Name == nameof(IAsyncStateMachine.MoveNext) && typeof(IAsyncStateMachine).IsAssignableFrom(topMethod?.DeclaringType))
string fileNameWithoutEx = Path.GetFileNameWithoutExtension(sourceFilePath);
if (!string.IsNullOrEmpty(fileNameWithoutEx))
{
// Async method; return actual method as determined by heuristic:
// "Nearest method on stack to async state-machine's MoveNext() in same namespace but in a different type".
// There are tighter ways of determining the actual method, but this is good enough and probably faster.
for (int deepFrame = topFrame + 1; deepFrame < stackTrace.FrameCount; deepFrame++)
{
var deepMethod = stackTrace.GetFrame(deepFrame)?.GetMethod();

if (deepMethod?.DeclaringType != topMethod?.DeclaringType && deepMethod?.DeclaringType?.Namespace == topMethod?.DeclaringType?.Namespace)
{
return deepMethod;
}
}
callerFileName = fileNameWithoutEx;
}
}
catch (Exception)
{
// Ignore exceptions in Release. The code above won't throw, but if it does, we don't want to crash the app.
callerFileName = "Unknown";
#if DEBUG
throw;
#endif
}

return topMethod;
return $"{callerFileName}::{memberName}";
}
}
}
1 change: 1 addition & 0 deletions src/common/ManagedCommon/ManagedCommon.csproj
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
<Project Sdk="Microsoft.NET.Sdk">
<!-- Look at Directory.Build.props in root for common stuff as well -->
<Import Project="..\..\Common.Dotnet.CsWinRT.props" />
<Import Project="..\..\Common.Dotnet.AotCompatibility.props" />

<PropertyGroup>
<Description>PowerToys ManagedCommon</Description>
Expand Down
13 changes: 13 additions & 0 deletions src/common/ManagedCommon/ManagedCommonJsonSerializerContext.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// Copyright (c) Microsoft Corporation
// The Microsoft Corporation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System.Text.Json.Serialization;
using static ManagedCommon.LanguageHelper;

namespace ManagedCommon;

[JsonSerializable(typeof(OutGoingLanguageSettings))]
internal sealed partial class ManagedCommonJsonSerializerContext : JsonSerializerContext
{
}
2 changes: 1 addition & 1 deletion src/common/ManagedCommon/NativeMethods.cs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public struct INPUT

internal static int Size
{
get { return Marshal.SizeOf(typeof(INPUT)); }
get { return Marshal.SizeOf<INPUT>(); }
}
}

Expand Down
7 changes: 3 additions & 4 deletions src/common/ManagedCommon/RunnerHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,11 @@ namespace ManagedCommon
{
public static class RunnerHelper
{
public static void WaitForPowerToysRunner(int powerToysPID, Action act)
public static void WaitForPowerToysRunner(int powerToysPID, Action act, [System.Runtime.CompilerServices.CallerMemberName] string memberName = "")
{
var stackTrace = new StackTrace();
var assembly = Assembly.GetCallingAssembly().GetName();
var callingMethod = stackTrace.GetFrame(1).GetMethod().Name;
PowerToysTelemetry.Log.WriteEvent(new DebugEvent() { Message = $"[{assembly}][{callingMethod}]WaitForPowerToysRunner waiting for Event powerToysPID={powerToysPID}" });
PowerToysTelemetry.Log.WriteEvent(new DebugEvent() { Message = $"[{assembly}][{memberName}]WaitForPowerToysRunner waiting for Event powerToysPID={powerToysPID}" });
Task.Run(() =>
{
const uint INFINITE = 0xFFFFFFFF;
Expand All @@ -29,7 +28,7 @@ public static void WaitForPowerToysRunner(int powerToysPID, Action act)
IntPtr powerToysProcHandle = NativeMethods.OpenProcess(SYNCHRONIZE, false, powerToysPID);
if (NativeMethods.WaitForSingleObject(powerToysProcHandle, INFINITE) == WAIT_OBJECT_0)
{
PowerToysTelemetry.Log.WriteEvent(new DebugEvent() { Message = $"[{assembly}][{callingMethod}]WaitForPowerToysRunner Event Notified powerToysPID={powerToysPID}" });
PowerToysTelemetry.Log.WriteEvent(new DebugEvent() { Message = $"[{assembly}][{memberName}]WaitForPowerToysRunner Event Notified powerToysPID={powerToysPID}" });
act.Invoke();
}
});
Expand Down
2 changes: 1 addition & 1 deletion src/common/ManagedCommon/ThemeListener.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ namespace ManagedCommon
/// <param name="sender">Sender ThemeListener</param>
public delegate void ThemeChangedEvent(ThemeListener sender);

public class ThemeListener : IDisposable
public partial class ThemeListener : IDisposable
{
/// <summary>
/// Gets the App Theme.
Expand Down
Loading