Skip to content

Commit

Permalink
EventPipe lock implementation (#82790)
Browse files Browse the repository at this point in the history 
* EventPipe lock implementation

* FB
  • Loading branch information
@LakshanF
LakshanF authored Mar 2, 2023
1 parent bac3bef commit cc8f02a
Show file tree
Hide file tree
Showing 5 changed files with 120 additions and 60 deletions.
2 changes: 2 additions & 0 deletions src/coreclr/nativeaot/Runtime/Crst.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ enum CrstType
CrstThreadStore,
CrstCastCache,
CrstYieldProcessorNormalized,
CrstEventPipe,
CrstEventPipeConfig,
};

enum CrstFlags
Expand Down
84 changes: 82 additions & 2 deletions src/coreclr/nativeaot/Runtime/eventpipe/ep-rt-aot.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
#include "holder.h"
#include "SpinLock.h"

// Uses _rt_aot_lock_internal_t that has CrstStatic as a field
// This is initialized at the beginning and EventPipe library requires the lock handle to be maintained by the runtime
ep_rt_lock_handle_t _ep_rt_aot_config_lock_handle;
CrstStatic _ep_rt_aot_config_lock;

Expand Down Expand Up @@ -147,7 +149,7 @@ ep_rt_aot_atomic_compare_exchange_size_t (volatile size_t *target, size_t expect
return static_cast<size_t>(PalInterlockedCompareExchange64 ((volatile int64_t *)target, (int64_t)value, (int64_t)expected));
#else
return static_cast<size_t>(PalInterlockedCompareExchange ((volatile int32_t *)target, (int32_t)value, (int32_t)expected));
#endif
#endif
}

ep_char8_t *
Expand Down Expand Up @@ -351,7 +353,12 @@ ep_rt_aot_spin_lock_alloc (ep_rt_spin_lock_handle_t *spin_lock)
{
STATIC_CONTRACT_NOTHROW;

spin_lock->lock = new (nothrow) SpinLock ();
// EventPipe library expects SpinLocks to be used but NativeAOT will use a lock and change as needed if performance is an issue
// Uses _rt_aot_lock_internal_t that has CrstStatic as a field
// EventPipe library will intialize using thread, EventPipeBufferManager instances and will maintain these on the EventPipe library side

spin_lock->lock = new (nothrow) CrstStatic ();
spin_lock->lock->InitNoThrow (CrstType::CrstEventPipe);
}

void
Expand Down Expand Up @@ -502,4 +509,77 @@ ep_rt_aot_volatile_store_ptr_without_barrier (
VolatileStoreWithoutBarrier<void *> ((void **)ptr, value);
}

void ep_rt_aot_init (void)
{
extern ep_rt_lock_handle_t _ep_rt_aot_config_lock_handle;
extern CrstStatic _ep_rt_aot_config_lock;

_ep_rt_aot_config_lock_handle.lock = &_ep_rt_aot_config_lock;
_ep_rt_aot_config_lock_handle.lock->InitNoThrow (CrstType::CrstEventPipeConfig);
}

bool ep_rt_aot_lock_acquire (ep_rt_lock_handle_t *lock)
{
if (lock) {
lock->lock->Enter();
return true;
}
return false;
}

bool ep_rt_aot_lock_release (ep_rt_lock_handle_t *lock)
{
if (lock) {
lock->lock->Leave();
return true;
}
return false;
}

bool ep_rt_aot_spin_lock_acquire (ep_rt_spin_lock_handle_t *spin_lock)
{
// In NativeAOT, we use a lock, instead of a SpinLock.
// The method signature matches the EventPipe library expectation of a SpinLock
if (spin_lock) {
spin_lock->lock->Enter();
return true;
}
return false;
}

bool ep_rt_aot_spin_lock_release (ep_rt_spin_lock_handle_t *spin_lock)
{
// In NativeAOT, we use a lock, instead of a SpinLock.
// The method signature matches the EventPipe library expectation of a SpinLock
if (spin_lock) {
spin_lock->lock->Leave();
return true;
}
return false;
}

#ifdef EP_CHECKED_BUILD

void ep_rt_aot_lock_requires_lock_held (const ep_rt_lock_handle_t *lock)
{
EP_ASSERT (((ep_rt_lock_handle_t *)lock)->lock->OwnedByCurrentThread ());
}

void ep_rt_aot_lock_requires_lock_not_held (const ep_rt_lock_handle_t *lock)
{
EP_ASSERT (lock->lock == NULL || !((ep_rt_lock_handle_t *)lock)->lock->OwnedByCurrentThread ());
}

void ep_rt_aot_spin_lock_requires_lock_held (const ep_rt_spin_lock_handle_t *spin_lock)
{
EP_ASSERT (ep_rt_spin_lock_is_valid (spin_lock));
EP_ASSERT (spin_lock->lock->OwnedByCurrentThread ());
}

void ep_rt_aot_spin_lock_requires_lock_not_held (const ep_rt_spin_lock_handle_t *spin_lock)
{
EP_ASSERT (spin_lock->lock == NULL || !spin_lock->lock->OwnedByCurrentThread ());
}

#endif /* EP_CHECKED_BUILD */
#endif /* ENABLE_PERFTRACING */
76 changes: 27 additions & 49 deletions src/coreclr/nativeaot/Runtime/eventpipe/ep-rt-aot.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1133,9 +1133,8 @@ inline
ep_rt_lock_handle_t *
ep_rt_aot_config_lock_get (void)
{
// shipping criteria: no EVENTPIPE-NATIVEAOT-TODO left in the codebase
// TODO: Implement EventPipe locking for NativeAOT
return nullptr;
extern ep_rt_lock_handle_t _ep_rt_aot_config_lock_handle;
return &_ep_rt_aot_config_lock_handle;
}

static
Expand Down Expand Up @@ -1320,8 +1319,8 @@ static
void
ep_rt_init (void)
{
// shipping criteria: no EVENTPIPE-NATIVEAOT-TODO left in the codebase
// TODO: Implement EventPipe locking for NativeAOT
extern void ep_rt_aot_init (void);
ep_rt_aot_init();
}

static
Expand All @@ -1345,19 +1344,15 @@ inline
bool
ep_rt_config_acquire (void)
{
// shipping criteria: no EVENTPIPE-NATIVEAOT-TODO left in the codebase
// TODO: Implement EventPipe locking for NativeAOT
return true;
return ep_rt_lock_acquire (ep_rt_aot_config_lock_get ());
}

static
inline
bool
ep_rt_config_release (void)
{
// shipping criteria: no EVENTPIPE-NATIVEAOT-TODO left in the codebase
// TODO: Implement EventPipe locking for NativeAOT
return true;
return ep_rt_lock_release (ep_rt_aot_config_lock_get ());
}

#ifdef EP_CHECKED_BUILD
Expand All @@ -1366,19 +1361,15 @@ inline
void
ep_rt_config_requires_lock_held (void)
{
// shipping criteria: no EVENTPIPE-NATIVEAOT-TODO left in the codebase
// TODO: Implement EventPipe locking for NativeAOT
return;
ep_rt_lock_requires_lock_held (ep_rt_aot_config_lock_get ());
}

static
inline
void
ep_rt_config_requires_lock_not_held (void)
{
// shipping criteria: no EVENTPIPE-NATIVEAOT-TODO left in the codebase
// TODO: Implement EventPipe locking for NativeAOT
return;
ep_rt_lock_requires_lock_not_held (ep_rt_aot_config_lock_get ());
}
#endif

Expand Down Expand Up @@ -2283,25 +2274,17 @@ bool
ep_rt_lock_acquire (ep_rt_lock_handle_t *lock)
{
STATIC_CONTRACT_NOTHROW;

bool result = true;
// shipping criteria: no EVENTPIPE-NATIVEAOT-TODO left in the codebase
// TODO: Implement EventPipe locking for NativeAOT

return result;
extern bool ep_rt_aot_lock_acquire (ep_rt_lock_handle_t *lock);
return ep_rt_aot_lock_acquire(lock);
}

static
bool
ep_rt_lock_release (ep_rt_lock_handle_t *lock)
{
STATIC_CONTRACT_NOTHROW;

bool result = true;
// shipping criteria: no EVENTPIPE-NATIVEAOT-TODO left in the codebase
// TODO: Implement EventPipe locking for NativeAOT

return result;
extern bool ep_rt_aot_lock_release (ep_rt_lock_handle_t *lock);
return ep_rt_aot_lock_release(lock);
}

#ifdef EP_CHECKED_BUILD
Expand All @@ -2312,7 +2295,8 @@ ep_rt_lock_requires_lock_held (const ep_rt_lock_handle_t *lock)
{

STATIC_CONTRACT_NOTHROW;
//EP_ASSERT (((ep_rt_lock_handle_t *)lock)->lock->OwnedByCurrentThread ());
extern void ep_rt_aot_lock_requires_lock_held (const ep_rt_lock_handle_t *lock);
ep_rt_aot_lock_requires_lock_held(lock);
}

static
Expand All @@ -2321,7 +2305,8 @@ void
ep_rt_lock_requires_lock_not_held (const ep_rt_lock_handle_t *lock)
{
STATIC_CONTRACT_NOTHROW;
//EP_ASSERT (lock->lock == NULL || !((ep_rt_lock_handle_t *)lock)->lock->OwnedByCurrentThread ());
extern void ep_rt_aot_lock_requires_lock_not_held (const ep_rt_lock_handle_t *lock);
ep_rt_aot_lock_requires_lock_not_held(lock);
}
#endif

Expand All @@ -2334,8 +2319,7 @@ void
ep_rt_spin_lock_alloc (ep_rt_spin_lock_handle_t *spin_lock)
{
STATIC_CONTRACT_NOTHROW;
extern void
ep_rt_aot_spin_lock_alloc (ep_rt_spin_lock_handle_t *spin_lock);
extern void ep_rt_aot_spin_lock_alloc (ep_rt_spin_lock_handle_t *spin_lock);
ep_rt_aot_spin_lock_alloc(spin_lock);
}

Expand All @@ -2345,8 +2329,7 @@ void
ep_rt_spin_lock_free (ep_rt_spin_lock_handle_t *spin_lock)
{
STATIC_CONTRACT_NOTHROW;
extern void
ep_rt_aot_spin_lock_free (ep_rt_spin_lock_handle_t *spin_lock);
extern void ep_rt_aot_spin_lock_free (ep_rt_spin_lock_handle_t *spin_lock);
ep_rt_aot_spin_lock_free(spin_lock);
}

Expand All @@ -2356,12 +2339,9 @@ bool
ep_rt_spin_lock_acquire (ep_rt_spin_lock_handle_t *spin_lock)
{
STATIC_CONTRACT_NOTHROW;
// EP_ASSERT (ep_rt_spin_lock_is_valid (spin_lock));

// shipping criteria: no EVENTPIPE-NATIVEAOT-TODO left in the codebase
// TODO: Implement locking (maybe by making the manual Lock and Unlock functions public)
// SpinLock::Lock (*(spin_lock->lock));
return true;
EP_ASSERT (ep_rt_spin_lock_is_valid (spin_lock));
extern bool ep_rt_aot_spin_lock_acquire (ep_rt_spin_lock_handle_t *spin_lock);
return ep_rt_aot_spin_lock_acquire(spin_lock);
}

static
Expand All @@ -2371,11 +2351,8 @@ ep_rt_spin_lock_release (ep_rt_spin_lock_handle_t *spin_lock)
{
STATIC_CONTRACT_NOTHROW;
EP_ASSERT (ep_rt_spin_lock_is_valid (spin_lock));

// shipping criteria: no EVENTPIPE-NATIVEAOT-TODO left in the codebase
// TODO: Implement locking (maybe by making the manual Lock and Unlock functions public)
// SpinLock::Unlock (*(spin_lock->lock));
return true;
extern bool ep_rt_aot_spin_lock_release (ep_rt_spin_lock_handle_t *spin_lock);
return ep_rt_aot_spin_lock_release(spin_lock);
}

#ifdef EP_CHECKED_BUILD
Expand All @@ -2385,8 +2362,8 @@ void
ep_rt_spin_lock_requires_lock_held (const ep_rt_spin_lock_handle_t *spin_lock)
{
STATIC_CONTRACT_NOTHROW;
EP_ASSERT (ep_rt_spin_lock_is_valid (spin_lock));

extern void ep_rt_aot_spin_lock_requires_lock_held (const ep_rt_spin_lock_handle_t *spin_lock);
ep_rt_aot_spin_lock_requires_lock_held(spin_lock);
}

static
Expand All @@ -2395,7 +2372,8 @@ void
ep_rt_spin_lock_requires_lock_not_held (const ep_rt_spin_lock_handle_t *spin_lock)
{
STATIC_CONTRACT_NOTHROW;

extern void ep_rt_aot_spin_lock_requires_lock_not_held (const ep_rt_spin_lock_handle_t *spin_lock);
ep_rt_aot_spin_lock_requires_lock_not_held(spin_lock);
}
#endif

Expand Down
8 changes: 2 additions & 6 deletions src/coreclr/nativeaot/Runtime/eventpipe/ep-rt-types-aot.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -138,11 +138,6 @@ struct _rt_aot_lock_internal_t {
CrstStatic *lock;
};

class SpinLock;
struct _rt_aot_spin_lock_internal_t {
SpinLock *lock;
};

/*
* EventPipeBuffer.
*/
Expand Down Expand Up @@ -303,7 +298,8 @@ typedef struct _rt_aot_event_internal_t ep_rt_wait_event_handle_t;
typedef struct _rt_aot_lock_internal_t ep_rt_lock_handle_t;

#undef ep_rt_spin_lock_handle_t
typedef _rt_aot_spin_lock_internal_t ep_rt_spin_lock_handle_t;
// NativeAOT will start with CrstStatic instead of a SpinLock and change as needed if performance is an issue
typedef struct _rt_aot_lock_internal_t ep_rt_spin_lock_handle_t;

/*
* Thread.
Expand Down
10 changes: 7 additions & 3 deletions src/tests/tracing/eventpipe/simpleprovidervalidation/simpleprovidervalidation.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,14 +44,18 @@ public static int Main()

private static Dictionary<string, ExpectedEventCount> _expectedEventCounts = new Dictionary<string, ExpectedEventCount>()
{
{ "MyEventSource", 1 },
{ "MyEventSource", 100_000 },
{ "Microsoft-DotNETCore-EventPipe", 1}
};

private static Action _eventGeneratingAction = () =>
{
Logger.logger.Log($"Firing an event...");
MyEventSource.Log.MyEvent();
for (int i = 0; i < 100_000; i++)
{
if (i % 10_000 == 0)
Logger.logger.Log($"Fired MyEvent {i:N0}/100,000 times...");
MyEventSource.Log.MyEvent();
}
};
}
}

0 comments on commit cc8f02a

Please sign in to comment.