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

Move AsyncCausality to shared partition #22062

Merged
merged 3 commits into from
Jan 18, 2019
Merged

Move AsyncCausality to shared partition #22062

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0fab23c
Move AsyncCausality to shared partition
marek-safar Jan 17, 2019
672c99e
Review feedback
marek-safar Jan 18, 2019
2dffb91
Set FeatureAsyncCausalityTracer property
marek-safar Jan 18, 2019
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
3 changes: 1 addition & 2 deletions src/System.Private.CoreLib/System.Private.CoreLib.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,6 @@
<Compile Include="$(BclSourcesRoot)\Internal\Resources\WindowsRuntimeResourceManagerBase.cs" Condition="'$(FeatureAppX)' == 'true'" />
<Compile Include="$(BclSourcesRoot)\Internal\Runtime\Augments\EnvironmentAugments.cs" />
<Compile Include="$(BclSourcesRoot)\Internal\Runtime\Augments\RuntimeThread.cs" />
<Compile Include="$(BclSourcesRoot)\Internal\Threading\Tasks\AsyncCausalitySupport.cs" />
<Compile Include="$(BclSourcesRoot)\Microsoft\Win32\UnsafeNativeMethods.cs" />
<Compile Include="$(BclSourcesRoot)\Microsoft\Win32\Win32Native.cs" />
<Compile Include="$(BclSourcesRoot)\System\__Canon.cs" />
Expand Down Expand Up @@ -279,7 +278,6 @@
<Compile Include="$(BclSourcesRoot)\System\Threading\Monitor.cs" />
<Compile Include="$(BclSourcesRoot)\System\Threading\Overlapped.cs" />
<Compile Include="$(BclSourcesRoot)\System\Threading\SynchronizationContext.cs" />
<Compile Include="$(BclSourcesRoot)\System\Threading\Tasks\AsyncCausalityTracer.cs" />
<Compile Include="$(BclSourcesRoot)\System\Threading\Tasks\FutureFactory.cs" />
<Compile Include="$(BclSourcesRoot)\System\Threading\Tasks\Task.cs" />
<Compile Include="$(BclSourcesRoot)\System\Threading\Tasks\TaskContinuation.cs" />
Expand Down Expand Up @@ -367,6 +365,7 @@
<Compile Include="$(BclSourcesRoot)\System\Runtime\InteropServices\WindowsRuntime\WindowsRuntimeMarshal.cs" />
<Compile Include="$(BclSourcesRoot)\System\Runtime\InteropServices\WindowsRuntime\WindowsRuntimeMetadata.cs" />
<Compile Include="$(BclSourcesRoot)\System\Threading\Tasks\IAsyncCausalityTracerStatics.cs" />
<Compile Include="$(BclSourcesRoot)\System\Threading\Tasks\AsyncCausalityTracer.cs" />
<Compile Include="$(BclSourcesRoot)\System\Variant.cs" />
</ItemGroup>
<ItemGroup Condition="'$(TargetsUnix)' == 'true'">
Expand Down
5 changes: 0 additions & 5 deletions .../Threading/Tasks/AsyncCausalitySupport.cs → .../Threading/Tasks/AsyncCausalitySupport.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,8 @@
// 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.Diagnostics;
using System.Threading.Tasks;
using System.Collections.Generic;
using System.Runtime.CompilerServices;
using AsyncCausalityStatus = System.Threading.Tasks.AsyncCausalityStatus;
using CausalityTraceLevel = System.Threading.Tasks.CausalityTraceLevel;

namespace Internal.Threading.Tasks
{
Expand Down
2 changes: 2 additions & 0 deletions src/System.Private.CoreLib/shared/System.Private.CoreLib.Shared.projitems
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
<Compile Include="$(MSBuildThisFileDirectory)Internal\IO\File.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Internal\Padding.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Internal\Runtime\CompilerServices\Unsafe.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Internal\Threading\Tasks\AsyncCausalitySupport.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Microsoft\Win32\SafeHandles\CriticalHandleMinusOneIsInvalid.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Microsoft\Win32\SafeHandles\CriticalHandleZeroOrMinusOneIsInvalid.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Microsoft\Win32\SafeHandles\SafeHandleMinusOneIsInvalid.cs" />
Expand Down Expand Up @@ -770,6 +771,7 @@
<Compile Include="$(MSBuildThisFileDirectory)System\Threading\SpinWait.cs" />
<Compile Include="$(MSBuildThisFileDirectory)System\Threading\SynchronizationLockException.cs" />
<Compile Include="$(MSBuildThisFileDirectory)System\Threading\ThreadLocal.cs" />
<Compile Include="$(MSBuildThisFileDirectory)System\Threading\Tasks\AsyncCausalityTracer.cs" />
<Compile Include="$(MSBuildThisFileDirectory)System\Threading\Tasks\ConcurrentExclusiveSchedulerPair.cs" />
<Compile Include="$(MSBuildThisFileDirectory)System\Threading\Tasks\Future.cs" />
<Compile Include="$(MSBuildThisFileDirectory)System\Threading\Tasks\ProducerConsumerQueues.cs" />
Expand Down
81 changes: 81 additions & 0 deletions src/System.Private.CoreLib/shared/System/Threading/Tasks/AsyncCausalityTracer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
// 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.

#if !FEATURE_COMINTEROP
Copy link
Member

Choose a reason for hiding this comment

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

We prefer conditions in the .csproj files to end-to-end ifdefs.

Copy link
Author

Choose a reason for hiding this comment

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

Yep, I was not sure how to best indicate this file is for FEATURE_COMINTEROP only

Copy link
Member

Choose a reason for hiding this comment

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

The condition for the file can be '$(FeatureAsyncCausalityTracer)'!='true' so that each runtime can have its own idea whether and when to use the noop implementation.

Copy link
Author

Choose a reason for hiding this comment

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

@jkotas Where would I set that flag for CoreCLR FEATURE_COMINTEROP ?

Copy link
Member

Choose a reason for hiding this comment

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

Adding this to System.Private.CoreLib.csproj in CoreCLR should do the trick:

  <PropertyGroup>
    <FeatureAsyncCausalityTracer Condition="'$(FeatureCominterop)' == 'true'">true</FeatureAsyncCausalityTracer>
  </PropertyGroup>

Copy link
Author

Choose a reason for hiding this comment

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

ok


using System.Diagnostics;

namespace System.Threading.Tasks
{
internal enum CausalityTraceLevel
{
Required = 0,
Important = 1,
Verbose = 2,
}

internal enum AsyncCausalityStatus
{
Started = 0,
Completed = 1,
Canceled = 2,
Error = 3,
}

internal enum CausalityRelation
{
AssignDelegate = 0,
Join = 1,
Choice = 2,
Cancel = 3,
Error = 4,
}

internal enum CausalitySynchronousWork
{
CompletionNotification = 0,
ProgressNotification = 1,
Execution = 2,
}

//
// Empty implementation of AsyncCausality events
//
internal static class AsyncCausalityTracer
{
public static readonly bool LoggingOn = false;
Copy link
Member

Choose a reason for hiding this comment

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

This can be const.

Copy link
Author

@marek-safar marek-safar Jan 18, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

In theory yes but we end up with a lot of C# Unreachable Code warnings, is that ok?

Copy link
Member

Choose a reason for hiding this comment

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

Ok, we do not like warnings. Maybe property returning false would work better - the codegen can always see that it is false, without triggering ctors, etc.

Copy link
Author

Choose a reason for hiding this comment

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

Why property? this should not generate static ctor as it's the default value and codegen and linker can recognize this pattern easier without evaluating the property

Copy link
Member

Choose a reason for hiding this comment

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

If you make it a property, linker can recognize property returning constant too and it is easier to do right.

Properties returning constant cannot be overridden by reflection or other means. It makes them easy for optimizers to reason about.

Static fields are much harder for optimizers to reason about because of reflection, cctor triggers, static fields set by debugger, etc.


[Conditional("NOOP_ASYNCCASUALITYTRACER")]
Copy link
Member

Choose a reason for hiding this comment

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

May call this file AsyncCausalityTracer.Noop.cs ?

Copy link
Member

Choose a reason for hiding this comment

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

.Noop or .Dummy (current CoreRT convention)?

Copy link
Author

Choose a reason for hiding this comment

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

I don't like dummy, will go with .Noop.cs

public static void EnableToETW(bool enabled)
{
}

[Conditional("NOOP_ASYNCCASUALITYTRACER")]
public static void TraceOperationCreation(CausalityTraceLevel traceLevel, int taskId, string operationName, ulong relatedContext)
{
}

[Conditional("NOOP_ASYNCCASUALITYTRACER")]
public static void TraceOperationCompletion(CausalityTraceLevel traceLevel, int taskId, AsyncCausalityStatus status)
{
}

[Conditional("NOOP_ASYNCCASUALITYTRACER")]
public static void TraceOperationRelation(CausalityTraceLevel traceLevel, int taskId, CausalityRelation relation)
{
}

[Conditional("NOOP_ASYNCCASUALITYTRACER")]
public static void TraceSynchronousWorkStart(CausalityTraceLevel traceLevel, int taskId, CausalitySynchronousWork work)
{
}

[Conditional("NOOP_ASYNCCASUALITYTRACER")]
public static void TraceSynchronousWorkCompletion(CausalityTraceLevel traceLevel, CausalitySynchronousWork work)
{
}
}
}

#endif
55 changes: 3 additions & 52 deletions src/System.Private.CoreLib/src/System/Threading/Tasks/AsyncCausalityTracer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,107 +2,69 @@
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

//
//
#if FEATURE_COMINTEROP
Copy link
Member

Choose a reason for hiding this comment

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

Change this to condition in the project file

Copy link
Author

Choose a reason for hiding this comment

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

Done that already just forgot to remove the #if


using System;
using System.Security;
using System.Diagnostics;
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;

#if FEATURE_COMINTEROP
using System.Runtime.InteropServices.WindowsRuntime;

using WFD = Windows.Foundation.Diagnostics;
#endif


namespace System.Threading.Tasks
{
internal enum CausalityTraceLevel
{
#if FEATURE_COMINTEROP
Required = WFD.CausalityTraceLevel.Required,
Important = WFD.CausalityTraceLevel.Important,
Verbose = WFD.CausalityTraceLevel.Verbose
#else
Required,
Important,
Verbose
#endif
}

internal enum AsyncCausalityStatus
{
#if FEATURE_COMINTEROP
Canceled = WFD.AsyncCausalityStatus.Canceled,
Completed = WFD.AsyncCausalityStatus.Completed,
Error = WFD.AsyncCausalityStatus.Error,
Started = WFD.AsyncCausalityStatus.Started
#else
Started,
Completed,
Canceled,
Error
#endif
}

internal enum CausalityRelation
{
#if FEATURE_COMINTEROP
AssignDelegate = WFD.CausalityRelation.AssignDelegate,
Join = WFD.CausalityRelation.Join,
Choice = WFD.CausalityRelation.Choice,
Cancel = WFD.CausalityRelation.Cancel,
Error = WFD.CausalityRelation.Error
#else
AssignDelegate,
Join,
Choice,
Cancel,
Error
#endif
}

internal enum CausalitySynchronousWork
{
#if FEATURE_COMINTEROP
CompletionNotification = WFD.CausalitySynchronousWork.CompletionNotification,
ProgressNotification = WFD.CausalitySynchronousWork.ProgressNotification,
Execution = WFD.CausalitySynchronousWork.Execution
#else
CompletionNotification,
ProgressNotification,
Execution
#endif
}

internal static class AsyncCausalityTracer
{
internal static void EnableToETW(bool enabled)
{
#if FEATURE_COMINTEROP
if (enabled)
f_LoggingOn |= Loggers.ETW;
else
f_LoggingOn &= ~Loggers.ETW;
#endif
}

internal static bool LoggingOn
{
get
{
#if FEATURE_COMINTEROP
return f_LoggingOn != 0;
#else
return false;
#endif
}
}

#if FEATURE_COMINTEROP
//s_PlatformId = {4B0171A6-F3D0-41A0-9B33-02550652B995}
private static readonly Guid s_PlatformId = new Guid(0x4B0171A6, 0xF3D0, 0x41A0, 0x9B, 0x33, 0x02, 0x55, 0x06, 0x52, 0xB9, 0x95);

Expand Down Expand Up @@ -164,15 +126,13 @@ private static void TracingStatusChangedHandler(object sender, WFD.TracingStatus
else
f_LoggingOn &= ~Loggers.CausalityTracer;
}
#endif

//
// The TraceXXX methods should be called only if LoggingOn property returned true
//
[MethodImplAttribute(MethodImplOptions.NoInlining)] // Tracking is slow path. Disable inlining for it.
internal static void TraceOperationCreation(CausalityTraceLevel traceLevel, int taskId, string operationName, ulong relatedContext)
{
#if FEATURE_COMINTEROP
try
{
if ((f_LoggingOn & Loggers.ETW) != 0)
Expand All @@ -185,13 +145,11 @@ internal static void TraceOperationCreation(CausalityTraceLevel traceLevel, int
//view function comment
LogAndDisable(ex);
}
#endif
}

[MethodImplAttribute(MethodImplOptions.NoInlining)]
internal static void TraceOperationCompletion(CausalityTraceLevel traceLevel, int taskId, AsyncCausalityStatus status)
{
#if FEATURE_COMINTEROP
try
{
if ((f_LoggingOn & Loggers.ETW) != 0)
Expand All @@ -204,13 +162,11 @@ internal static void TraceOperationCompletion(CausalityTraceLevel traceLevel, in
//view function comment
LogAndDisable(ex);
}
#endif
}

[MethodImplAttribute(MethodImplOptions.NoInlining)]
internal static void TraceOperationRelation(CausalityTraceLevel traceLevel, int taskId, CausalityRelation relation)
{
#if FEATURE_COMINTEROP
try
{
if ((f_LoggingOn & Loggers.ETW) != 0)
Expand All @@ -223,13 +179,11 @@ internal static void TraceOperationRelation(CausalityTraceLevel traceLevel, int
//view function comment
LogAndDisable(ex);
}
#endif
}

[MethodImplAttribute(MethodImplOptions.NoInlining)]
internal static void TraceSynchronousWorkStart(CausalityTraceLevel traceLevel, int taskId, CausalitySynchronousWork work)
{
#if FEATURE_COMINTEROP
try
{
if ((f_LoggingOn & Loggers.ETW) != 0)
Expand All @@ -242,13 +196,11 @@ internal static void TraceSynchronousWorkStart(CausalityTraceLevel traceLevel, i
//view function comment
LogAndDisable(ex);
}
#endif
}

[MethodImplAttribute(MethodImplOptions.NoInlining)]
internal static void TraceSynchronousWorkCompletion(CausalityTraceLevel traceLevel, CausalitySynchronousWork work)
{
#if FEATURE_COMINTEROP
try
{
if ((f_LoggingOn & Loggers.ETW) != 0)
Expand All @@ -261,22 +213,21 @@ internal static void TraceSynchronousWorkCompletion(CausalityTraceLevel traceLev
//view function comment
LogAndDisable(ex);
}
#endif
}

#if FEATURE_COMINTEROP
//fix for 796185: leaking internal exceptions to customers,
//we should catch and log exceptions but never propagate them.
private static void LogAndDisable(Exception ex)
{
f_LoggingOn = 0;
Debugger.Log(0, "AsyncCausalityTracer", ex.ToString());
}
#endif

private static ulong GetOperationId(uint taskId)
{
return (((ulong)Thread.GetDomainID()) << 32) + taskId;
}
}
}

#endif