From aea810cf1328d1fa55b48b974c84e5430f4272d3 Mon Sep 17 00:00:00 2001 From: Sung Yoon Whang Date: Thu, 7 May 2020 09:21:30 -0700 Subject: [PATCH] Fix EventPipeBuffer to use ClrVirtualAlloc instead of malloc --- src/vm/eventpipebuffer.cpp | 6 ++++-- src/vm/eventpipebuffermanager.cpp | 6 ++++++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/vm/eventpipebuffer.cpp b/src/vm/eventpipebuffer.cpp index 69eec1e26875..6392fb78fc0d 100644 --- a/src/vm/eventpipebuffer.cpp +++ b/src/vm/eventpipebuffer.cpp @@ -23,7 +23,9 @@ EventPipeBuffer::EventPipeBuffer(unsigned int bufferSize, EventPipeThread* pWrit m_state = EventPipeBufferState::WRITABLE; m_pWriterThread = pWriterThread; m_eventSequenceNumber = eventSequenceNumber; - m_pBuffer = new BYTE[bufferSize]; + // Use ClrVirtualAlloc instead of malloc to allocate buffer to avoid potential internal fragmentation in the native CRT heap. + // (See https://github.com/dotnet/runtime/pull/35924 and https://github.com/microsoft/ApplicationInsights-dotnet/issues/1678 for more details) + m_pBuffer = (BYTE*)ClrVirtualAlloc(NULL, bufferSize, MEM_COMMIT, PAGE_READWRITE); memset(m_pBuffer, 0, bufferSize); m_pLimit = m_pBuffer + bufferSize; m_pCurrent = GetNextAlignedAddress(m_pBuffer); @@ -47,7 +49,7 @@ EventPipeBuffer::~EventPipeBuffer() } CONTRACTL_END; - delete[] m_pBuffer; + ClrVirtualFree(m_pBuffer, 0, MEM_RELEASE); } bool EventPipeBuffer::WriteEvent(Thread *pThread, EventPipeSession &session, EventPipeEvent &event, EventPipeEventPayload &payload, LPCGUID pActivityId, LPCGUID pRelatedActivityId, StackContents *pStack) diff --git a/src/vm/eventpipebuffermanager.cpp b/src/vm/eventpipebuffermanager.cpp index 7daf6891e35a..8f216a38c3cb 100644 --- a/src/vm/eventpipebuffermanager.cpp +++ b/src/vm/eventpipebuffermanager.cpp @@ -168,6 +168,12 @@ EventPipeBuffer* EventPipeBufferManager::AllocateBufferForThread(EventPipeThread const unsigned int maxBufferSize = 1024 * 1024; bufferSize = Min(bufferSize, maxBufferSize); + // Make the buffer size fit into with pagesize-aligned block, + // since ClrVirtualAlloc expects page-aligned sizes to be passed + // as arguments (see ctor of EventPipeBuffer) + bufferSize = (bufferSize + g_SystemInfo.dwAllocationGranularity - 1) & ~static_cast(g_SystemInfo.dwAllocationGranularity - 1); + + // EX_TRY is used here as opposed to new (nothrow) because // the constructor also allocates a private buffer, which // could throw, and cannot be easily checked