Skip to content

Commit

Permalink
Adjust test + code change
Browse files Browse the repository at this point in the history
  • Loading branch information
gleocadie committed Oct 14, 2024
1 parent 578cdb2 commit 0710771
Show file tree
Hide file tree
Showing 9 changed files with 204 additions and 136 deletions.
79 changes: 62 additions & 17 deletions profiler/src/Demos/Samples.Computer01/UnsafeToUnwind.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ internal class UnsafeToUnwind
private Thread _worker;
private Thread _workerUnsafe;
private Thread _workerException;
private Thread _workerContention;

public void Start()
{
Expand All @@ -25,23 +26,10 @@ public void Start()

_stopEvent = new ManualResetEvent(false);

_worker = new Thread(SafeToUnwind)
{
IsBackground = false // set to false to prevent the app from shutting down. The test will fail
};
_worker.Start();

_workerUnsafe = new Thread(Wrap_UnSafeToUnwind)
{
IsBackground = false // set to false to prevent the app from shutting down. The test will fail
};
_workerUnsafe.Start();

_workerException = new Thread(StartException)
{
IsBackground = false // set to false to prevent the app from shutting down. The test will fail
};
_workerException.Start();
_worker = CreateAndStartThread(SafeToUnwind);
_workerUnsafe = CreateAndStartThread(Wrap_UnSafeToUnwind);
_workerException = CreateAndStartThread(StartException);
_workerContention = CreateAndStartThread(StartContention);
}

public void Run()
Expand All @@ -63,10 +51,22 @@ public void Stop()
_worker.Join();
_workerUnsafe.Join();
_workerException.Join();
_workerContention.Join();
_stopEvent.Dispose();
_stopEvent = null;
}

private static Thread CreateAndStartThread(ThreadStart action)
{
var t = new Thread(action)
{
IsBackground = false // set to false to prevent the app from shutting down. The test will fail
};
t.Start();

return t;
}

[MethodImpl(MethodImplOptions.NoInlining)]
private static void Wrap_RaiseExceptionUnsafeUnwind()
{
Expand Down Expand Up @@ -124,5 +124,50 @@ private void StartException()
Thread.Sleep(TimeSpan.FromMilliseconds(250));
}
}

private void StartContention()
{
var lockObj = new object();
var otherThread = CreateAndStartThread(() =>
{
while (!_stopEvent.WaitOne(0))
{
Wrap_ContentionUnsafeToUnwind(lockObj);
}
});

while (!_stopEvent.WaitOne(0))
{
ContentionSafeToUnwind(lockObj);
}

otherThread.Join();
}

[MethodImpl(MethodImplOptions.NoInlining)]
private void Wrap_ContentionUnsafeToUnwind(object lockObj)
{
ContentionUnsafeToUnwind(lockObj);
}

[MethodImpl(MethodImplOptions.NoInlining)]
private void ContentionUnsafeToUnwind(object lockObj)
{
LockAndSleep(lockObj);
}

[MethodImpl(MethodImplOptions.NoInlining)]
private void ContentionSafeToUnwind(object lockObj)
{
LockAndSleep(lockObj);
}

private void LockAndSleep(object lockObj)
{
lock (lockObj)
{
Thread.Sleep(TimeSpan.FromMilliseconds(250));
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ enum class ThreadMetaInfo : std::uint64_t
UnsafeToUnwind = 1
};

struct alignas(FieldAlignRequirement) TraceContextInfo
struct alignas(FieldAlignRequirement) TraceContext
{
public:
std::uint64_t _writeGuard;
Expand All @@ -35,14 +35,6 @@ struct alignas(FieldAlignRequirement) TraceContextInfo
std::uint64_t _threadMetaInfo;
};

struct TraceContext
{
public:
static constexpr std::uint8_t Version = 2;

TraceContextInfo _impl;
};

struct ManagedThreadInfo : public IThreadInfo
{
private:
Expand Down Expand Up @@ -94,7 +86,7 @@ struct ManagedThreadInfo : public IThreadInfo
inline void SetThreadDestroyed();
inline std::pair<uint64_t, shared::WSTRING> SetBlockingThread(uint64_t osThreadId, shared::WSTRING name);

inline TraceContextInfo* GetTraceContextPointer();
inline TraceContext* GetTraceContextPointer();
inline bool HasTraceContext() const;

inline std::string GetProfileThreadId() override;
Expand Down Expand Up @@ -431,14 +423,14 @@ inline std::pair<uint64_t, shared::WSTRING> ManagedThreadInfo::SetBlockingThread
return {oldId, oldName};
}

inline TraceContextInfo* ManagedThreadInfo::GetTraceContextPointer()
inline TraceContext* ManagedThreadInfo::GetTraceContextPointer()
{
return &_traceContext._impl;
return &_traceContext;
}

inline bool ManagedThreadInfo::CanReadTraceContext() const
{
bool canReadTraceContext = _traceContext._impl._writeGuard;
bool canReadTraceContext = _traceContext._writeGuard;

// As said in the doc, on x86 (x86_64 including) this is a compiler fence.
// In our case, it suffices. We have to make sure that reading this field is done
Expand Down Expand Up @@ -493,7 +485,7 @@ inline std::int32_t ManagedThreadInfo::GetTimerId() const

inline bool ManagedThreadInfo::IsSafeToUnwind() const
{
return (_traceContext._impl._threadMetaInfo & static_cast<std::uint64_t>(ThreadMetaInfo::UnsafeToUnwind)) == 0;
return (_traceContext._threadMetaInfo & static_cast<std::uint64_t>(ThreadMetaInfo::UnsafeToUnwind)) == 0;
}

inline AppDomainID ManagedThreadInfo::GetAppDomainId()
Expand All @@ -514,8 +506,8 @@ inline std::pair<std::uint64_t, std::uint64_t> ManagedThreadInfo::GetTracingCont

if (CanReadTraceContext())
{
localRootSpanId = _traceContext._impl._currentLocalRootSpanId;
spanId = _traceContext._impl._currentSpanId;
localRootSpanId = _traceContext._currentLocalRootSpanId;
spanId = _traceContext._currentSpanId;
}

return {localRootSpanId, spanId};
Expand Down
19 changes: 0 additions & 19 deletions profiler/src/ProfilerEngine/Datadog.Profiler.Native/PInvoke.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -189,22 +189,3 @@ extern "C" void __stdcall FlushProfile()
Log::Debug("FlushProfile called by Managed code");
profiler->GetSamplesCollector()->Export();
}

extern "C" int32_t __stdcall GetTraceContextVersion()
{
const auto profiler = CorProfilerCallback::GetInstance();

if (profiler == nullptr)
{
Log::Error("GetTraceContextVersion is called BEFORE CLR initialize");
return -1;
}

if (!profiler->GetClrLifetime()->IsRunning())
{
return -1;
}

return TraceContext::Version;

}
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,4 @@ extern "C" void* __stdcall GetPointerToNativeTraceContext();

extern "C" void __stdcall SetApplicationInfoForAppDomain(const char* runtimeId, const char* serviceName, const char* environment, const char* version);

extern "C" void __stdcall FlushProfile();

extern "C" int32_t __stdcall GetTraceContextVersion();
extern "C" void __stdcall FlushProfile();
Original file line number Diff line number Diff line change
Expand Up @@ -296,14 +296,14 @@ void StackSamplerLoop::CpuProfilingIteration()
// Note: it is not possible to get this information on Windows 32-bit or in some cases in 64-bit
// so isRunning should be true if this thread consumed some CPU since the last iteration
#if _WINDOWS
#if BIT64 // Windows 64-bit
#if BIT64 // Windows 64-bit
if (failure)
{
isRunning = (lastConsumption < currentConsumption);
}
#else // Windows 32-bit
#else // Windows 32-bit
isRunning = (lastConsumption < currentConsumption);
#endif
#endif
#else // nothing to do for Linux
#endif

Expand Down Expand Up @@ -450,10 +450,7 @@ void StackSamplerLoop::CollectOneThreadStackSample(
// Notify the loop manager that we are starting a stack collection, and set up a finalizer to notify the manager when we finsih it.
// This will enable the manager to monitor if this collection freezes due to a deadlock.

on_leave
{
_pManager->NotifyIterationFinished();
};
on_leave { _pManager->NotifyIterationFinished(); };

// On Windows, we will now suspend the target thread.
// On Linux, if we use signals, the suspension may be a no-op since signal handlers do not use explicit suspension.
Expand All @@ -480,10 +477,7 @@ void StackSamplerLoop::CollectOneThreadStackSample(
// Perform the stack walk according to bitness, OS, platform, sync/async stack-walking, etc..:
{
// We rely on RAII to call NotifyCollectionEnd when we get out this scope.
on_leave
{
_pManager->NotifyCollectionEnd();
};
on_leave { _pManager->NotifyCollectionEnd(); };

_pManager->NotifyCollectionStart();
pStackSnapshotResult = _pStackFramesCollector->CollectStackSample(pThreadInfo.get(), &hrCollectStack);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@
// This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2022 Datadog, Inc.
// </copyright>

using System.Text;
using System.Linq;
using Datadog.Profiler.IntegrationTests.Helpers;
using Xunit;
using FluentAssertions;
using Xunit.Abstractions;

namespace Datadog.Profiler.IntegrationTests.Bugs
Expand All @@ -23,25 +23,81 @@ public UnsafeToUnwindTest(ITestOutputHelper output)
public void AvoidUnwindingUnsafeExecution(string appName, string framework, string appAssembly)
{
var runner = new TestApplicationRunner(appName, framework, appAssembly, _output, commandLine: "--scenario 28", enableTracer: true);
// TODO add the env var here
runner.Environment.SetVariable("DD_PROFILER_SKIPPED_METHODS", "Samples.Computer01.UnsafeToUnwind[Wrap_UnSafeToUnwind];Samples.Computer01.UnsafeToUnwind[Wrap_RaiseExceptionUnsafeUnwind]");
runner.Environment.SetVariable(EnvironmentVariables.SkippedMethods, "Samples.Computer01.UnsafeToUnwind[Wrap_UnSafeToUnwind];Samples.Computer01.UnsafeToUnwind[Wrap_RaiseExceptionUnsafeUnwind];Samples.Computer01.UnsafeToUnwind[Wrap_ContentionUnsafeToUnwind]");

using var agent = MockDatadogAgent.CreateHttpAgent(_output);
runner.Run(agent);

var toSkipStack = new StackTrace(
new StackFrame("|lm:Samples.Computer01 |ns:Samples.Computer01 |ct:UnsafeToUnwind |cg: |fn:UnSafeToUnwind |fg: |sg:()"));

var exceptionToSkipStack = new StackTrace(
new StackFrame("|lm:Samples.Computer01 |ns:Samples.Computer01 |ct:UnsafeToUnwind |cg: |fn:RaiseExceptionUnsafeUnwind |fg: |sg:()"));

var exceptionToKeepStack = new StackTrace(
new StackFrame("|lm:Samples.Computer01 |ns:Samples.Computer01 |ct:UnsafeToUnwind |cg: |fn:RaiseExceptionSafeUnwind |fg: |sg:()"));

// Event based profiler
var contentionToSkipStack = new StackTrace(
new StackFrame("|lm:Samples.Computer01 |ns:Samples.Computer01 |ct:UnsafeToUnwind |cg: |fn:ContentionUnsafeToUnwind |fg: |sg:()"));

var contentionToKeepStack = new StackTrace(
new StackFrame("|lm:Samples.Computer01 |ns:Samples.Computer01 |ct:UnsafeToUnwind |cg: |fn:ContentionSafeToUnwind |fg: |sg:()"));

// Safe to unwind frame
var safeToUnwind = new StackTrace(
new StackFrame("|lm:Samples.Computer01 |ns:Samples.Computer01 |ct:UnsafeToUnwind |cg: |fn:SafeToUnwind |fg: |sg:()"));

var samples = SamplesHelper.GetSamples(runner.Environment.PprofDir);

samples.Should().NotBeNullOrEmpty("Not samples found");

samples.Where(s => s.StackTrace.EndWith(toSkipStack)).Should().BeNullOrEmpty();
samples.Where(s => s.StackTrace.EndWith(exceptionToSkipStack)).Should().BeNullOrEmpty();
samples.Where(s => s.StackTrace.EndWith(contentionToSkipStack)).Should().BeNullOrEmpty();

samples.Any(s => s.StackTrace.EndWith(safeToUnwind)).Should().BeTrue();
samples.Any(s => s.StackTrace.EndWith(exceptionToKeepStack)).Should().BeTrue();
samples.Any(s => s.StackTrace.EndWith(contentionToKeepStack)).Should().BeTrue();
}

[TestAppFact("Samples.Computer01")]
public void CheckIfEnvVarDoesNotExist(string appName, string framework, string appAssembly)
{
var runner = new TestApplicationRunner(appName, framework, appAssembly, _output, commandLine: "--scenario 28", enableTracer: true);

using var agent = MockDatadogAgent.CreateHttpAgent(_output);
runner.Run(agent);

var toSkipStack = new StackTrace(
new StackFrame("|lm:Samples.Computer01 |ns:Samples.Computer01 |ct:UnsafeToUnwind |cg: |fn:UnSafeToUnwind |fg: |sg:()"));

var exceptionToSkipStack = new StackTrace(
new StackFrame("|lm:Samples.Computer01 |ns:Samples.Computer01 |ct:UnsafeToUnwind |cg: |fn:RaiseExceptionUnsafeUnwind |fg: |sg:()"));

foreach (var sample in samples)
{
Assert.False(sample.StackTrace.EndWith(toSkipStack));
Assert.False(sample.StackTrace.EndWith(exceptionToSkipStack));
}
var exceptionToKeepStack = new StackTrace(
new StackFrame("|lm:Samples.Computer01 |ns:Samples.Computer01 |ct:UnsafeToUnwind |cg: |fn:RaiseExceptionSafeUnwind |fg: |sg:()"));

// Event based profiler
var contentionToSkipStack = new StackTrace(
new StackFrame("|lm:Samples.Computer01 |ns:Samples.Computer01 |ct:UnsafeToUnwind |cg: |fn:ContentionUnsafeToUnwind |fg: |sg:()"));

var contentionToKeepStack = new StackTrace(
new StackFrame("|lm:Samples.Computer01 |ns:Samples.Computer01 |ct:UnsafeToUnwind |cg: |fn:ContentionSafeToUnwind |fg: |sg:()"));

// Safe to unwind frame
var safeToUnwind = new StackTrace(
new StackFrame("|lm:Samples.Computer01 |ns:Samples.Computer01 |ct:UnsafeToUnwind |cg: |fn:SafeToUnwind |fg: |sg:()"));

var samples = SamplesHelper.GetSamples(runner.Environment.PprofDir);

samples.Where(s => s.StackTrace.EndWith(toSkipStack)).Should().NotBeNullOrEmpty("DD_PROFILER_SKIPPED_METHODS is not set and no CPU/Walltime samples contain the problematic function (UnSafeToUnwind).");
samples.Where(s => s.StackTrace.EndWith(exceptionToSkipStack)).Should().NotBeNullOrEmpty("DD_PROFILER_SKIPPED_METHODS is not set and no Exception samples contain the problematic function (RaiseExceptionUnsafeUnwind).");
samples.Where(s => s.StackTrace.EndWith(contentionToSkipStack)).Should().NotBeNullOrEmpty("DD_PROFILER_SKIPPED_METHODS is not set and no Contention samples contain the problematic function (ContentionSafeToUnwind).");

samples.Any(s => s.StackTrace.EndWith(safeToUnwind)).Should().BeTrue();
samples.Any(s => s.StackTrace.EndWith(exceptionToKeepStack)).Should().BeTrue();
samples.Any(s => s.StackTrace.EndWith(contentionToKeepStack)).Should().BeTrue();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -33,5 +33,6 @@ internal class EnvironmentVariables
public const string SsiShortLivedThreshold = "DD_INTERNAL_PROFILING_LONG_LIVED_THRESHOLD";
public const string TelemetryToDiskEnabled = "DD_INTERNAL_PROFILING_TELEMETRY_TO_DISK_ENABLED";
public const string SsiTelemetryEnabled = "DD_INTERNAL_PROFILING_SSI_TELEMETRY_ENABLED";
public const string SkippedMethods = "DD_PROFILER_SKIPPED_METHODS";
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,10 @@ public bool EndWith(StackTrace other)

for (int i = 0; i < other._stackFrames.Count; i++)
{
_stackFrames[i].ToString().Should().Be(other._stackFrames[i].ToString());
if (_stackFrames[i].ToString() != other._stackFrames[i].ToString())
{
return false;
}
}

return true;
Expand Down
Loading

0 comments on commit 0710771

Please sign in to comment.