Move eventpipe buffer to TLS

src/System.Private.CoreLib/src/System/Diagnostics/Eventing/EventPipePayloadDecoder.cs

@@ -110,16 +110,29 @@ internal static object[] DecodePayload(ref EventSource.EventMetadata metadata, R
                 }
                 else if (parameterType == typeof(string))
                 {
-                    ReadOnlySpan<char> charPayload = MemoryMarshal.Cast<byte, char>(payload);
-                    int charCount = charPayload.IndexOf('\0');
-                    if (charCount < 0)
+                    // Try to find null terminator (0x00) from the byte span
+                    // NOTE: we do this by hand instead of using IndexOf because payload may be unaligned due to 
+                    // mixture of different types being stored in the same buffer. (see eventpipe.cpp:CopyData)
+                    int byteCount = -1;
+                    for (int j = 1; j < payload.Length; j+=2)
                     {
+                        if (payload[j-1] == (byte)(0) && payload[j] == (byte)(0))
+                        {
+                            byteCount = j+1;
+                            break;
+                        }
+                    }
+
+                    ReadOnlySpan<char> charPayload;
+                    if (byteCount < 0)
+                    {
+                        charPayload = MemoryMarshal.Cast<byte, char>(payload);
                         payload = default;
                     }
                     else
                     {
-                        charPayload = charPayload.Slice(0, charCount);
-                        payload = payload.Slice((charCount + 1) * sizeof(char));
+                        charPayload = MemoryMarshal.Cast<byte, char>(payload.Slice(0, byteCount-2));
+                        payload = payload.Slice(byteCount);
                     }
                     decodedFields[i] = BitConverter.IsLittleEndian ? new string(charPayload) : Encoding.Unicode.GetString(MemoryMarshal.Cast<char, byte>(charPayload));
                 }

src/vm/eventpipe.cpp

@@ -751,11 +751,6 @@ void EventPipe::WriteEventInternal(EventPipeEvent &event, EventPipeEventPayload
 
     // Get the current thread;
     Thread *pThread = GetThread();
-    if(pThread == NULL)
-    {
-        // We can't write an event without the thread object.
-        return;
-    }
 
     if(s_pConfig == NULL)
     {
@@ -764,8 +759,9 @@ void EventPipe::WriteEventInternal(EventPipeEvent &event, EventPipeEventPayload
     }
     _ASSERTE(s_pSession != NULL);
 
-    // If the activity id isn't specified, pull it from the current thread.
-    if(pActivityId == NULL)
+    // If the activity id isn't specified AND we are in a managed thread, pull it from the current thread.
+    // If pThread is NULL (we aren't in writing from a managed thread) then pActivityId can be NULL 
+    if(pActivityId == NULL && pThread != NULL)
     {
         pActivityId = pThread->GetActivityId();
     }
@@ -780,7 +776,7 @@ void EventPipe::WriteEventInternal(EventPipeEvent &event, EventPipeEventPayload
         // We're not interested in these events and they can cause corrupted trace files because rundown
         // events are written synchronously and not under lock.
         // If we encounter an event that did not originate on the thread that is doing rundown, ignore it.
-        if(!s_pConfig->IsRundownThread(pThread))
+        if(pThread == NULL || !s_pConfig->IsRundownThread(pThread))
         {
             return;
         }

src/vm/eventpipebuffer.cpp

@@ -54,14 +54,13 @@ bool EventPipeBuffer::WriteEvent(Thread *pThread, EventPipeSession &session, Eve
         NOTHROW;
         GC_NOTRIGGER;
         MODE_ANY;
-        PRECONDITION(pThread != NULL);
         PRECONDITION(((size_t)m_pCurrent % AlignmentSize) == 0);
     }
     CONTRACTL_END;
 
     // Calculate the size of the event.
     unsigned int eventSize = sizeof(EventPipeEventInstance) + payload.GetSize();
-
+    
     // Make sure we have enough space to write the event.
     if(m_pCurrent + eventSize >= m_pLimit)
     {
@@ -75,14 +74,16 @@ bool EventPipeBuffer::WriteEvent(Thread *pThread, EventPipeSession &session, Eve
     EX_TRY
     {
         // Placement-new the EventPipeEventInstance.
+        // if pthread is NULL, it's likely we are running in something like a GC thread which is not a Thread object, so it can't have an activity ID set anyway
         EventPipeEventInstance *pInstance = new (m_pCurrent) EventPipeEventInstance(
             session,
             event,
-            pThread->GetOSThreadId(),
+            (pThread == NULL) ? ::GetCurrentThreadId() : pThread->GetOSThreadId(),
             pDataDest,
             payload.GetSize(),
-            pActivityId,
+            (pThread == NULL) ? NULL : pActivityId,
             pRelatedActivityId);
+
 
         // Copy the stack if a separate stack trace was provided.
         if(pStack != NULL)

src/vm/eventpipebuffermanager.cpp

@@ -10,6 +10,12 @@
 
 #ifdef FEATURE_PERFTRACING
 
+#ifndef __llvm__
+__declspec(thread) ThreadEventBufferList ThreadEventBufferList::gCurrentThreadEventBufferList;
+#else // !__llvm__
+thread_local ThreadEventBufferList ThreadEventBufferList::gCurrentThreadEventBufferList;
+#endif // !__llvm__
+
 EventPipeBufferManager::EventPipeBufferManager()
 {
     CONTRACTL
@@ -54,16 +60,6 @@ EventPipeBufferManager::~EventPipeBufferManager()
             EventPipeBufferList *pThreadBufferList = pCurElem->GetValue();
             if (!pThreadBufferList->OwnedByThread())
             {
-                Thread *pThread = NULL;
-                while ((pThread = ThreadStore::GetThreadList(pThread)) != NULL)
-                {
-                    if (pThread->GetEventPipeBufferList() == pThreadBufferList)
-                    {
-                        pThread->SetEventPipeBufferList(NULL);
-                        break;
-                    }
-                }
-
                 // We don't delete buffers themself because they can be in-use
                 delete(pThreadBufferList);
             }
@@ -77,14 +73,13 @@ EventPipeBufferManager::~EventPipeBufferManager()
     }
 }
 
-EventPipeBuffer* EventPipeBufferManager::AllocateBufferForThread(EventPipeSession &session, Thread *pThread, unsigned int requestSize)
+EventPipeBuffer* EventPipeBufferManager::AllocateBufferForThread(EventPipeSession &session, unsigned int requestSize)
 {
     CONTRACTL
     {
         NOTHROW;
         GC_NOTRIGGER;
         MODE_ANY;
-        PRECONDITION(pThread != NULL);
         PRECONDITION(requestSize > 0);
     }
     CONTRACTL_END;
@@ -95,7 +90,8 @@ EventPipeBuffer* EventPipeBufferManager::AllocateBufferForThread(EventPipeSessio
     // Determine if the requesting thread has at least one buffer.
     // If not, we guarantee that each thread gets at least one (to prevent thrashing when the circular buffer size is too small).
     bool allocateNewBuffer = false;
-    EventPipeBufferList *pThreadBufferList = pThread->GetEventPipeBufferList();
+    EventPipeBufferList *pThreadBufferList = ThreadEventBufferList::Get();
+
     if(pThreadBufferList == NULL)
     {
         pThreadBufferList = new (nothrow) EventPipeBufferList(this);
@@ -111,7 +107,7 @@ EventPipeBuffer* EventPipeBufferManager::AllocateBufferForThread(EventPipeSessio
         }
 
         m_pPerThreadBufferList->InsertTail(pElem);
-        pThread->SetEventPipeBufferList(pThreadBufferList);
+        ThreadEventBufferList::Set(pThreadBufferList);
         allocateNewBuffer = true;
     }
 
@@ -326,9 +322,6 @@ bool EventPipeBufferManager::WriteEvent(Thread *pThread, EventPipeSession &sessi
         return false;
     }
 
-    // The event is still enabled.  Mark that the thread is now writing an event.
-    pThread->SetEventWriteInProgress(true);
-
     // Check one more time to make sure that the event is still enabled.
     // We do this because we might be trying to disable tracing and free buffers, so we
     // must make sure that the event is enabled after we mark that we're writing to avoid
@@ -341,7 +334,9 @@ bool EventPipeBufferManager::WriteEvent(Thread *pThread, EventPipeSession &sessi
     // See if the thread already has a buffer to try.
     bool allocNewBuffer = false;
     EventPipeBuffer *pBuffer = NULL;
-    EventPipeBufferList *pThreadBufferList = pThread->GetEventPipeBufferList();
+
+    EventPipeBufferList *pThreadBufferList = ThreadEventBufferList::Get();
+
     if(pThreadBufferList == NULL)
     {
         allocNewBuffer = true;
@@ -358,8 +353,14 @@ bool EventPipeBufferManager::WriteEvent(Thread *pThread, EventPipeSession &sessi
         }
         else
         {
+            // The event is still enabled.  Mark that the thread is now writing an event.
+            pThreadBufferList->SetThreadEventWriteInProgress(true);
+
             // Attempt to write the event to the buffer.  If this fails, we should allocate a new buffer.
             allocNewBuffer = !pBuffer->WriteEvent(pEventThread, session, event, payload, pActivityId, pRelatedActivityId, pStack);
+
+            // Mark that the thread is no longer writing an event.
+            pThreadBufferList->SetThreadEventWriteInProgress(false);
         }
     }
 
@@ -374,19 +375,24 @@ bool EventPipeBufferManager::WriteEvent(Thread *pThread, EventPipeSession &sessi
         // to switch to preemptive mode here.
 
         unsigned int requestSize = sizeof(EventPipeEventInstance) + payload.GetSize();
-        pBuffer = AllocateBufferForThread(session, pThread, requestSize);
+        pBuffer = AllocateBufferForThread(session, requestSize);
     }
 
     // Try to write the event after we allocated (or stole) a buffer.
     // This is the first time if the thread had no buffers before the call to this function.
     // This is the second time if this thread did have one or more buffers, but they were full.
     if(allocNewBuffer && pBuffer != NULL)
     {
+        // By this point, a new buffer list has been allocated so we should fetch it again before using it.
+        pThreadBufferList = ThreadEventBufferList::Get();
+        // The event is still enabled.  Mark that the thread is now writing an event.
+        pThreadBufferList->SetThreadEventWriteInProgress(true);
         allocNewBuffer = !pBuffer->WriteEvent(pEventThread, session, event, payload, pActivityId, pRelatedActivityId, pStack);
+
+        // Mark that the thread is no longer writing an event.
+        pThreadBufferList->SetThreadEventWriteInProgress(false);
     }
 
-    // Mark that the thread is no longer writing an event.
-     pThread->SetEventWriteInProgress(false);
 
 #ifdef _DEBUG
     if(!allocNewBuffer)
@@ -543,75 +549,6 @@ void EventPipeBufferManager::DeAllocateBuffers()
 
     _ASSERTE(EnsureConsistency());
 
-    // Take the thread store lock because we're going to iterate through the thread list.
-    {
-        ThreadStoreLockHolder tsl;
-
-        // Take the buffer manager manipulation lock.
-        SpinLockHolder _slh(&m_lock);
-
-        Thread *pThread = NULL;
-        while ((pThread = ThreadStore::GetThreadList(pThread)) != NULL)
-        {
-            // Get the thread's buffer list.
-            EventPipeBufferList *pBufferList = pThread->GetEventPipeBufferList();
-            if(pBufferList != NULL)
-            {
-                // Attempt to free the buffer list.
-                // If the thread is using its buffer list skip it.
-                // This means we will leak a single buffer, but if tracing is re-enabled, that buffer can be used again.
-                if(!pThread->GetEventWriteInProgress())
-                {
-                    EventPipeBuffer *pBuffer = pBufferList->GetAndRemoveHead();
-                    while(pBuffer != NULL)
-                    {
-                        DeAllocateBuffer(pBuffer);
-                        pBuffer = pBufferList->GetAndRemoveHead();
-                    }
-
-                    // Remove the list entry from the per thread buffer list.
-                    SListElem<EventPipeBufferList*> *pElem = m_pPerThreadBufferList->GetHead();
-                    while(pElem != NULL)
-                    {
-                        EventPipeBufferList* pEntry = pElem->GetValue();
-                        if(pEntry == pBufferList)
-                        {
-                            pElem = m_pPerThreadBufferList->FindAndRemove(pElem);
-
-                            // In DEBUG, make sure that the element was found and removed.
-                            _ASSERTE(pElem != NULL);
-
-                            SListElem<EventPipeBufferList*> *pCurElem = pElem;
-                            pElem = m_pPerThreadBufferList->GetNext(pElem);
-                            delete(pCurElem);
-                        }
-                        else
-                        {
-                            pElem = m_pPerThreadBufferList->GetNext(pElem);
-                        }
-                    }
-
-                    // Remove the list reference from the thread.
-                    pThread->SetEventPipeBufferList(NULL);
-
-                    // Now that all of the list elements have been freed, free the list itself.
-                    delete(pBufferList);
-                    pBufferList = NULL;
-                }
-#ifdef _DEBUG
-                else
-                {
-                    // We can't deallocate the buffers.
-                    m_numBuffersLeaked += pBufferList->GetCount();
-                }
-#endif // _DEBUG            
-            }
-        }
-    }
-
-    // Now that we've walked through all of the threads, let's see if there are any other buffers
-    // that belonged to threads that died during tracing.  We can free these now.
-
     // Take the buffer manager manipulation lock
     SpinLockHolder _slh(&m_lock);
 
@@ -646,9 +583,10 @@ void EventPipeBufferManager::DeAllocateBuffers()
         {
             pElem = m_pPerThreadBufferList->GetNext(pElem);
         }
-    } 
+    }
 }
 
+
 #ifdef _DEBUG
 bool EventPipeBufferManager::EnsureConsistency()
 {
@@ -678,6 +616,7 @@ EventPipeBufferList::EventPipeBufferList(EventPipeBufferManager *pManager)
     m_bufferCount = 0;
     m_pReadBuffer = NULL;
     m_ownedByThread = true;
+    m_threadEventWriteInProgress = false;
 
 #ifdef _DEBUG
     m_pCreatingThread = GetThread();
@@ -871,18 +810,6 @@ EventPipeEventInstance* EventPipeBufferList::PopNextEvent(LARGE_INTEGER beforeTi
     return pNext;
 }
 
-bool EventPipeBufferList::OwnedByThread()
-{
-    LIMITED_METHOD_CONTRACT;
-    return m_ownedByThread;
-}
-
-void EventPipeBufferList::SetOwnedByThread(bool value)
-{
-    LIMITED_METHOD_CONTRACT;
-    m_ownedByThread = value;
-}
-
 #ifdef _DEBUG
 Thread* EventPipeBufferList::GetThread()
 {
@@ -960,4 +887,13 @@ bool EventPipeBufferList::EnsureConsistency()
 }
 #endif // _DEBUG
 
+ThreadEventBufferList::~ThreadEventBufferList()
+{
+    // Before the thread dies, mark its buffers as no longer owned
+    // so that they can be cleaned up after the thread dies.
+    // EventPipeBufferList *pList = GetThreadEventBufferList();
+    if (m_pThreadEventBufferList != NULL)
+        m_pThreadEventBufferList->SetOwnedByThread(false);
+}
+
 #endif // FEATURE_PERFTRACING