Simplifying Pop() logic

Commit migrated from https://github.com/dotnet/coreclr/commit/7cc30ea964201729b79b7c74617fa7c3749a3b89

diff --git a/src/coreclr/src/vm/eventpipebuffer.cpp b/src/coreclr/src/vm/eventpipebuffer.cpp
index fc4c3c4..091e87e 100644 (file)
--- a/src/coreclr/src/vm/eventpipebuffer.cpp
+++ b/src/coreclr/src/vm/eventpipebuffer.cpp
@@ -229,7 +229,7 @@ EventPipeEventInstance* EventPipeBuffer::PeekNext(LARGE_INTEGER beforeTimeStamp)
     return GetNext(m_pLastPoppedEvent, beforeTimeStamp);
 }
 
-EventPipeEventInstance* EventPipeBuffer::PopNext(LARGE_INTEGER beforeTimeStamp)
+void EventPipeBuffer::PopNext(EventPipeEventInstance *pNext)
 {
     CONTRACTL
     {
@@ -241,14 +241,10 @@ EventPipeEventInstance* EventPipeBuffer::PopNext(LARGE_INTEGER beforeTimeStamp)
 
     _ASSERTE(m_state == READ_ONLY);
 
-    // Get the next event using the last popped event as a marker.
-    EventPipeEventInstance *pNext = PeekNext(beforeTimeStamp);
     if(pNext != NULL)
     {
         m_pLastPoppedEvent = pNext;
     }
-
-    return pNext;
 }
 
 EventPipeBufferState EventPipeBuffer::GetVolatileState()

diff --git a/src/coreclr/src/vm/eventpipebuffer.h b/src/coreclr/src/vm/eventpipebuffer.h
index 926a87c..ffa0ad6 100644 (file)
--- a/src/coreclr/src/vm/eventpipebuffer.h
+++ b/src/coreclr/src/vm/eventpipebuffer.h
@@ -149,8 +149,8 @@ public:
     // Get the next event from the buffer, but don't mark it read.
     EventPipeEventInstance* PeekNext(LARGE_INTEGER beforeTimeStamp);
 
-    // Get the next event from the buffer and mark it as read.
-    EventPipeEventInstance* PopNext(LARGE_INTEGER beforeTimeStamp);
+    // TODO
+    void PopNext(EventPipeEventInstance *pEvent);
 
     // Check the state of the buffer
     EventPipeBufferState GetVolatileState();

diff --git a/src/coreclr/src/vm/eventpipebuffermanager.cpp b/src/coreclr/src/vm/eventpipebuffermanager.cpp
index efe5bd1..a3387bd 100644 (file)
--- a/src/coreclr/src/vm/eventpipebuffermanager.cpp
+++ b/src/coreclr/src/vm/eventpipebuffermanager.cpp
@@ -542,7 +542,7 @@ void EventPipeBufferManager::WriteAllBuffersToFile(EventPipeFile *pFile, LARGE_I
         {
             SpinLockHolder _slh(&m_lock);
             // Pop the event from the buffer.
-            pOldestContainingList->PopNextEvent(stopTimeStamp);
+            pOldestContainingList->PopNextEvent(pOldestContainingBuffer, pOldestInstance);
         }
     }
 
@@ -628,7 +628,7 @@ EventPipeEventInstance* EventPipeBufferManager::GetNextEvent()
     {
         SpinLockHolder _slh(&m_lock);
         // Pop the event from the buffer.
-        pOldestContainingList->PopNextEvent(stopTimeStamp);
+        pOldestContainingList->PopNextEvent(pOldestContainingBuffer, pOldestInstance);
     }
     
     // Return the oldest event that hasn't yet been processed.
@@ -974,34 +974,7 @@ void EventPipeBufferList::ConvertBufferToReadOnly(EventPipeBuffer* pNewReadBuffe
     }
 }
 
-EventPipeBuffer* EventPipeBufferList::TryGetReadBuffer(LARGE_INTEGER beforeTimeStamp, EventPipeBuffer* pNewReadBuffer)
-{
-    LIMITED_METHOD_CONTRACT;
-    _ASSERTE(EventPipe::IsBufferManagerLockOwnedByCurrentThread());
-
-    // is it possible that the read buffer has events that occur before 'beforeTimeStamp'?
-    if (pNewReadBuffer == NULL || pNewReadBuffer->GetCreationTimeStamp().QuadPart >= beforeTimeStamp.QuadPart)
-    {
-        return NULL;
-    }
-
-    // we can't read from a buffer while it is being simultaneously being written, we need to preempt the writer.
-    if (pNewReadBuffer->GetVolatileState() == EventPipeBufferState::WRITABLE)
-    {
-        {
-            SpinLockHolder _slh(m_pThread->GetLock());
-            if (m_pThread->GetWriteBuffer() == pNewReadBuffer)
-            {
-                m_pThread->SetWriteBuffer(nullptr);
-            }
-        }
-        _ASSERTE(pNewReadBuffer->GetVolatileState() == EventPipeBufferState::READ_ONLY);
-    }
-
-    return pNewReadBuffer;
-}
-
-EventPipeEventInstance* EventPipeBufferList::PeekNextEvent(LARGE_INTEGER beforeTimeStamp, EventPipeBuffer **pContainingBuffer)
+void EventPipeBufferList::PopNextEvent(EventPipeBuffer *pContainingBuffer, EventPipeEventInstance *pNext)
 {
     CONTRACTL
     {
@@ -1011,52 +984,6 @@ EventPipeEventInstance* EventPipeBufferList::PeekNextEvent(LARGE_INTEGER beforeT
     }
     CONTRACTL_END;
 
-    EventPipeBuffer* pReadBuffer = TryGetReadBuffer(beforeTimeStamp, m_pHeadBuffer);
-
-    // If the read buffer is still NULL, then this list contains no buffers with events in the time range we care about.
-    if(pReadBuffer == NULL)
-    {
-        return NULL;
-    }
-
-    // Get the next event in the buffer.
-    EventPipeEventInstance *pNext = pReadBuffer->PeekNext(beforeTimeStamp);
-
-    // If the next event is NULL, then go to the next buffer.
-    if(pNext == NULL)
-    {
-        pReadBuffer = TryGetReadBuffer(beforeTimeStamp, pReadBuffer->GetNext());
-        if(pReadBuffer != NULL)
-        {
-            pNext = pReadBuffer->PeekNext(beforeTimeStamp);
-        }
-    }
-
-    // Set the containing buffer.
-    if(pNext != NULL && pContainingBuffer != NULL)
-    {
-        *pContainingBuffer = pReadBuffer;
-    }
-
-    // Make sure pContainingBuffer is properly set.
-    _ASSERTE((pNext == NULL) || (pNext != NULL && pContainingBuffer == NULL) || (pNext != NULL && *pContainingBuffer == pReadBuffer));
-    return pNext;
-}
-
-EventPipeEventInstance* EventPipeBufferList::PopNextEvent(LARGE_INTEGER beforeTimeStamp)
-{
-    CONTRACTL
-    {
-        NOTHROW;
-        GC_NOTRIGGER;
-        MODE_ANY;
-    }
-    CONTRACTL_END;
-
-    // Get the next event.
-    EventPipeBuffer *pContainingBuffer = NULL;
-    EventPipeEventInstance *pNext = PeekNextEvent(beforeTimeStamp, &pContainingBuffer);
-
     // Check to see if we need to clean-up the buffer that contained the previously popped event.
     if(pContainingBuffer->GetPrevious() != NULL)
     {
@@ -1072,10 +999,8 @@ EventPipeEventInstance* EventPipeBufferList::PopNextEvent(LARGE_INTEGER beforeTi
     // If the event is non-NULL, pop it.
     if(pNext != NULL && pContainingBuffer != NULL)
     {
-        pContainingBuffer->PopNext(beforeTimeStamp);
+        pContainingBuffer->PopNext(pNext);
     }
-
-    return pNext;
 }
 
 EventPipeThread* EventPipeBufferList::GetThread()

diff --git a/src/coreclr/src/vm/eventpipebuffermanager.h b/src/coreclr/src/vm/eventpipebuffermanager.h
index b143448..9011c57 100644 (file)
--- a/src/coreclr/src/vm/eventpipebuffermanager.h
+++ b/src/coreclr/src/vm/eventpipebuffermanager.h
@@ -178,10 +178,6 @@ private:
     // The number of buffers in the list.
     unsigned int m_bufferCount;
 
-    // Check pNewReadBuffer to see if it has events in the right time-range and convert it to a readable
-    // state if needed
-    EventPipeBuffer* TryGetReadBuffer(LARGE_INTEGER beforeTimeStamp, EventPipeBuffer* pNewReadBuffer);
-
 public:
 
     EventPipeBufferList(EventPipeBufferManager *pManager, EventPipeThread* pThread);
@@ -201,11 +197,8 @@ public:
     // Get the count of buffers in the list.
     unsigned int GetCount() const;
 
-    // Get the next event as long as it is before the specified timestamp.
-    EventPipeEventInstance* PeekNextEvent(LARGE_INTEGER beforeTimeStamp, EventPipeBuffer **pContainingBuffer);
-
-    // Get the next event as long as it is before the specified timestamp, and also mark it as read.
-    EventPipeEventInstance* PopNextEvent(LARGE_INTEGER beforeTimeStamp);
+    // TODO
+    void PopNextEvent(EventPipeBuffer *pContainingBuffer, EventPipeEventInstance *pNext);
 
     // Get the thread associated with this list.
     EventPipeThread* GetThread();