Introducing Environment.ProcessorUserTime & ProcessorPrivilegedTime

[tarekgh]

We are exposing runtime metrics to report statistics for GC, thread pool, JIT, lock contention, timer, and process time. The implementation of these runtime metrics will reside in the System.Diagnostics.DiagnosticSource library, where the Metrics feature is housed. However, to report ProcessorUserTime and ProcessorPrivilegedTime, we need to call Process.GetCurrentProcess().ProcessorUserTime and Process.GetCurrentProcess().ProcessorPrivilegedTime, which requires adding a dependency on the System.Diagnostics.Process library. This dependency introduces two problems:

• Cyclic Dependency: System.Diagnostics.DiagnosticSource is a low-level library, and adding a dependency on System.Diagnostics.Process creates a cyclic dependency chain: System.IO.Pipes > System.Net.Sockets > System.Net.NameResolution > System.Diagnostics.DiagnosticSource > System.Diagnostics.Process > System.IO.Pipes.
• Size Regression: Adding more dependencies to this low-level library will increase the size of AOT builds.

To avoid these issues, we propose exposing new properties in the Environment class to report the same information, thus eliminating the need for the System.Diagnostics.Process dependency.

Proposal

static class Environment
{
    ...
    public static ProcessCpuUsage CpuUsage { get; }
    ...
}

public readonly struct ProcessCpuUsage
{
    public TimeSpan UserTime { get; } // report TimeSpans that are identical to calling Process.GetCurrentProcess().ProcessorUserTime
    public TimeSpan PrivilegedTime { get; } // report TimeSpans that are identical to Process.GetCurrentProcess().ProcessorPrivilegedTime
}

Alternative Design

static class Environment
{
    // identical tocalling Process.GetCurrentProcess().ProcessorUserTime
    public static TimeSpan ProcessorUserTime { get; }

    // identical tocalling Process.GetCurrentProcess().ProcessorPrivilegedTime
    public static TimeSpan  ProcessorPrivilegedTime { get; }
    ...
}

Example

     ProcessCpuUsage processCpuUsage = Environment.CpuUsage.
     Console.WriteLine($"ProcessorUserTime: {processCpuUsage.UserTime}  ... ProcessorPrivilegedTime: {processCpuUsage.PrivilegedTime} ");

     // Or the alternative design will be 
     Console.WriteLine($"ProcessorUserTime: {Environment.ProcessorUserTime}  ... ProcessorPrivilegedTime: {Environment.ProcessorPrivilegedTime} ");

[dotnet-policy-service[bot]]

Tagging subscribers to this area: @dotnet/area-system-diagnostics-process See info in area-owners.md if you want to be subscribed.

[dotnet-policy-service[bot]]

Tagging subscribers to this area: @dotnet/area-system-runtime See info in area-owners.md if you want to be subscribed.

[terrajobst]

Would the alternative design even work? Presumably these two values are linked so one would want to query them atomically, no?

[tarekgh]

@terrajobst

The alternative design is like what we are currently exposing from the Process class. But you have a good point as I expect more scenarios will need to snap the user and privileged time together.

https://learn.microsoft.com/en-us/dotnet/api/system.diagnostics.process.userprocessortime?view=net-8.0 https://learn.microsoft.com/en-us/dotnet/api/system.diagnostics.process.privilegedprocessortime?view=net-8.0

[MichalPetryka]

Is there a reason for separate type instead of a tuple?

[stephentoub]

Is there a reason for separate type instead of a tuple?

We could never add anything else to a tuple.

[terrajobst]

Video

• We'd prefer to have a nested type, as opposed to putting it in the System namespace
• Might as well also expose TotalTime

namespace System;

public partial class Environment
{
    [SupportedOSPlatform("maccatalyst")]
    [UnsupportedOSPlatform("ios")]
    [UnsupportedOSPlatform("tvos")]
    public static ProcessCpuUsage CpuUsage { get; }

    public readonly struct ProcessCpuUsage
    {
        public TimeSpan UserTime { get; }
        public TimeSpan PrivilegedTime { get; }
        public TimeSpan TotalTime => UserTime + PrivilegedTime;
    }
}

[terrajobst]

@bartonjs @stephentoub

@tarekgh asked me whether the struct members should be init too.

In general, I think we should avoid pure data holder types that can't be constructed as it makes testing harder, which means we either need to add a ctor or make the properties settable. For pure data holders I'd prefer settable props over a constructor because it means we don't need to add a new ctor whenever we add a new data member.

Now, since we don't like mutable structs we'd probably want init over set. Any objections to this?

namespace System;

public partial class Environment
{
    public readonly struct ProcessCpuUsage
    {
        public TimeSpan UserTime { get; init; }
        public TimeSpan PrivilegedTime { get; init; }
        public TimeSpan TotalTime => UserTime + PrivilegedTime;
    }
}

[jkotas]

We have number of types in BCL like this one that cannot be constructed. I do not think it makes the testing harder. These types are not exchange types and you cannot hook the api that produce them using DI.

I think we should stick with the approved surface. If we think that it makes sense to make types like this one constructable, it should be deeper discussion and the pattern applied consistently to all similar types.

[stephentoub]

I agree with Jan. If we want it to be constructable, we can later add a ctor or make the properties settable. I don't think init buys much here. We were also strongly considering using a tuple here, which is mutable, so immutability isn't a key requirement.

[tarekgh]

I will use internal init; for now and if we decide to make it public later, we can do it.

[terrajobst]

@jkotas @stephentoub

I'm surprised by this pushback. What flexibility do we gain by not having this struct be constructable? My understanding was that the reason we added this type in the first place was to return user and kernel time as a single unit; that seems to presuppose that our code merely populates this struct with the result, rather than doing anything clever.

To be clear, I don't particularly care for this struct, just trying to understand our rationale here.

[stephentoub]

@jkotas @stephentoub

I'm surprised by this pushback. What flexibility do we gain by not having this struct constructable? My understanding was that reason we added this type in the first hand is to return user and kernel time as a single unit; that seems to presuppose that our code merely constructs this struct with the result, rather than doing anything clever.

I'm fine with it being constructable. My pushback is on use of init here, which just seems unnecessarily "cute" (init presumes that the only valid way to create one of these is with object initializer syntax, which is constraining and forces a particular style, in this case unnecessarily). Either we don't want others to construct it, in which case the right shape is the one that was approved, or we do, in which case just make the properties settable (or add a constructor).

[jkotas]

My concern is about consistency. There are other similar types. GCMemoryInfo, GCGenerationInfo or ProcessModule are some examples that come to my mind. What should be the framework design rule for these types? Should we make all of them initiable with custom values just because of we can? Personally, I do not see the value of doing so.