From cc26bb3bfd9e5ae2fa3269c5a31fe06cbb188cfa Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Wed, 14 Jul 2021 03:18:52 +0200 Subject: [PATCH] wip --- .../PublicTerminalCore.vcxproj | 8 - src/inc/til/gate.h | 66 ++++++ src/renderer/base/thread.cpp | 202 ++++-------------- src/renderer/base/thread.hpp | 18 +- src/renderer/dx/DxRenderer.cpp | 2 +- 5 files changed, 111 insertions(+), 185 deletions(-) create mode 100644 src/inc/til/gate.h diff --git a/src/cascadia/PublicTerminalCore/PublicTerminalCore.vcxproj b/src/cascadia/PublicTerminalCore/PublicTerminalCore.vcxproj index 1665034e1..c6d5d5e8d 100644 --- a/src/cascadia/PublicTerminalCore/PublicTerminalCore.vcxproj +++ b/src/cascadia/PublicTerminalCore/PublicTerminalCore.vcxproj @@ -45,12 +45,4 @@ - - - - - Uiautomationcore.lib;onecoreuap.lib;%(AdditionalDependencies) - - diff --git a/src/inc/til/gate.h b/src/inc/til/gate.h new file mode 100644 index 000000000..a2f7c32be --- /dev/null +++ b/src/inc/til/gate.h @@ -0,0 +1,66 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +#pragma once + +#include "atomic.h" + +namespace til +{ + template + struct gate + { + constexpr explicit gate(const bool desired) noexcept : + _state(desired) + { + } + + gate(const gate&) = delete; + gate& operator=(const gate&) = delete; + + // acquire/release provide classical semaphore semantics. + void acquire() noexcept + { + while (!_state.exchange(false, AcquireOrder)) + { + til::atomic_wait(_state, false); + } + } + + // acquire/release provide classical semaphore semantics. + void release() noexcept + { + _state.store(true, ReleaseOrder); + til::atomic_notify_all(_state); + } + + // open/close respectively signal that this "real world gate" is open or closed. + // They deviate from the classical semaphore, by not being blocking operations. + inline void open() noexcept + { + release(); + } + + // open/close respectively signal that this "real world gate" is open or closed. + // They deviate from the classical semaphore, by not being blocking operations. + void close() noexcept + { + _state.store(false, ReleaseOrder); + } + + // Block until this "real world gate" can be passed, by being open. + // This works just like acquiring a semaphore, but doesn't decrease the count. + void pass() noexcept + { + while (!_state.load(AcquireOrder)) + { + til::atomic_wait(_state, false); + } + } + + private: + std::atomic _state; + }; + + using gate_relaxed = gate; +} diff --git a/src/renderer/base/thread.cpp b/src/renderer/base/thread.cpp index 8bd9a1bd8..5d654c427 100644 --- a/src/renderer/base/thread.cpp +++ b/src/renderer/base/thread.cpp @@ -12,12 +12,10 @@ using namespace Microsoft::Console::Render; RenderThread::RenderThread() : _pRenderer(nullptr), _hThread(nullptr), - _hEvent(nullptr), - _hPaintCompletedEvent(nullptr), _fKeepRunning(true), - _hPaintEnabledEvent(nullptr), - _fNextFrameRequested(false), - _fWaiting(false) + _hPaintCompletedEvent(true), + _hPaintEnabledEvent(false), + _fNextFrameRequested(false) { } @@ -25,30 +23,11 @@ RenderThread::~RenderThread() { if (_hThread) { - _fKeepRunning = false; // stop loop after final run + _fKeepRunning.store(false, std::memory_order_relaxed); // stop loop after final run EnablePainting(); // if we want to get the last frame out, we need to make sure it's enabled - SignalObjectAndWait(_hEvent, _hThread, INFINITE, FALSE); // signal final paint and wait for thread to finish. + WaitForSingleObject(_hThread, INFINITE); // wait for thread to finish. CloseHandle(_hThread); - _hThread = nullptr; - } - - if (_hEvent) - { - CloseHandle(_hEvent); - _hEvent = nullptr; - } - - if (_hPaintEnabledEvent) - { - CloseHandle(_hPaintEnabledEvent); - _hPaintEnabledEvent = nullptr; - } - - if (_hPaintCompletedEvent) - { - CloseHandle(_hPaintCompletedEvent); - _hPaintCompletedEvent = nullptr; } } @@ -65,89 +44,25 @@ RenderThread::~RenderThread() { _pRenderer = pRendererParent; - HRESULT hr = S_OK; - // Create event before thread as thread will start immediately. - if (SUCCEEDED(hr)) - { - HANDLE hEvent = CreateEventW(nullptr, // non-inheritable security attributes - FALSE, // auto reset event - FALSE, // initially unsignaled - nullptr // no name - ); + _hThread = CreateThread( + nullptr, // non-inheritable security attributes + 0, // use default stack size + s_ThreadProc, + this, + 0, // create immediately + nullptr // we don't need the thread ID + ); + RETURN_LAST_ERROR_IF_NULL(_hThread); - if (hEvent == nullptr) - { - hr = HRESULT_FROM_WIN32(GetLastError()); - } - else - { - _hEvent = hEvent; - } + // SetThreadDescription only works on 1607 and higher. If we cannot find it, + // then it's no big deal. Just skip setting the description. + auto func = GetProcAddressByFunctionDeclaration(GetModuleHandleW(L"kernel32.dll"), SetThreadDescription); + if (func) + { + LOG_IF_FAILED(func(_hThread, L"Rendering Output Thread")); } - if (SUCCEEDED(hr)) - { - HANDLE hPaintEnabledEvent = CreateEventW(nullptr, - TRUE, // manual reset event - FALSE, // initially signaled - nullptr); - - if (hPaintEnabledEvent == nullptr) - { - hr = HRESULT_FROM_WIN32(GetLastError()); - } - else - { - _hPaintEnabledEvent = hPaintEnabledEvent; - } - } - - if (SUCCEEDED(hr)) - { - HANDLE hPaintCompletedEvent = CreateEventW(nullptr, - TRUE, // manual reset event - TRUE, // initially signaled - nullptr); - - if (hPaintCompletedEvent == nullptr) - { - hr = HRESULT_FROM_WIN32(GetLastError()); - } - else - { - _hPaintCompletedEvent = hPaintCompletedEvent; - } - } - - if (SUCCEEDED(hr)) - { - HANDLE hThread = CreateThread(nullptr, // non-inheritable security attributes - 0, // use default stack size - s_ThreadProc, - this, - 0, // create immediately - nullptr // we don't need the thread ID - ); - - if (hThread == nullptr) - { - hr = HRESULT_FROM_WIN32(GetLastError()); - } - else - { - _hThread = hThread; - - // SetThreadDescription only works on 1607 and higher. If we cannot find it, - // then it's no big deal. Just skip setting the description. - auto func = GetProcAddressByFunctionDeclaration(GetModuleHandleW(L"kernel32.dll"), SetThreadDescription); - if (func) - { - LOG_IF_FAILED(func(hThread, L"Rendering Output Thread")); - } - } - } - - return hr; + return S_OK; } DWORD WINAPI RenderThread::s_ThreadProc(_In_ LPVOID lpParameter) @@ -164,61 +79,21 @@ DWORD WINAPI RenderThread::s_ThreadProc(_In_ LPVOID lpParameter) } } -DWORD WINAPI RenderThread::_ThreadProc() +DWORD RenderThread::_ThreadProc() { - while (_fKeepRunning) + while (_fKeepRunning.load(std::memory_order_relaxed)) { - WaitForSingleObject(_hPaintEnabledEvent, INFINITE); + //Sleep(8); // frame rate limit to ~60 FPS in practice - if (!_fNextFrameRequested.exchange(false, std::memory_order_acq_rel)) - { - // <-- - // If `NotifyPaint` is called at this point, then it will not - // set the event because `_fWaiting` is not `true` yet so we have - // to check again below. + _hPaintEnabledEvent.pass(); + _fNextFrameRequested.acquire(); - _fWaiting.store(true, std::memory_order_release); - - // check again now (see comment above) - if (!_fNextFrameRequested.exchange(false, std::memory_order_acq_rel)) - { - // Wait until a next frame is requested. - WaitForSingleObject(_hEvent, INFINITE); - } - - // <-- - // If `NotifyPaint` is called at this point, then it _will_ set - // the event because `_fWaiting` is `true`, but we're not waiting - // anymore! - // This can probably happen quite often: imagine a scenario where - // we are waiting, and the terminal calls `NotifyPaint` twice - // very quickly. - // In that case, both calls might end up calling `SetEvent`. The - // first one will resume this thread and the second one will - // `SetEvent` the event. So the next time we wait, the event will - // already be set and we won't actually wait. - // Because it can happen often, and because rendering is an - // expensive operation, we should reset the event to not render - // again if nothing changed. - - _fWaiting.store(false, std::memory_order_release); - - // see comment above - ResetEvent(_hEvent); - } - - ResetEvent(_hPaintCompletedEvent); + _hPaintCompletedEvent.store(false, std::memory_order_relaxed); _pRenderer->WaitUntilCanRender(); LOG_IF_FAILED(_pRenderer->PaintFrame()); - SetEvent(_hPaintCompletedEvent); - - // extra check before we sleep since it's a "long" activity, relatively speaking. - if (_fKeepRunning) - { - Sleep(s_FrameLimitMilliseconds); - } + _hPaintCompletedEvent.store(true, std::memory_order_relaxed); } return S_OK; @@ -226,24 +101,17 @@ DWORD WINAPI RenderThread::_ThreadProc() void RenderThread::NotifyPaint() { - if (_fWaiting.load(std::memory_order_acquire)) - { - SetEvent(_hEvent); - } - else - { - _fNextFrameRequested.store(true, std::memory_order_release); - } + _fNextFrameRequested.release(); } void RenderThread::EnablePainting() { - SetEvent(_hPaintEnabledEvent); + _hPaintEnabledEvent.open(); } void RenderThread::DisablePainting() { - ResetEvent(_hPaintEnabledEvent); + _hPaintEnabledEvent.close(); } void RenderThread::WaitForPaintCompletionAndDisable(const DWORD dwTimeoutMs) @@ -299,6 +167,10 @@ void RenderThread::WaitForPaintCompletionAndDisable(const DWORD dwTimeoutMs) // DirectX on OneCoreUAP times out while switching console // applications. - ResetEvent(_hPaintEnabledEvent); - WaitForSingleObject(_hPaintCompletedEvent, dwTimeoutMs); + _hPaintEnabledEvent.close(); + + while (!_hPaintCompletedEvent.load(std::memory_order_relaxed)) + { + til::atomic_wait(_hPaintCompletedEvent, false, dwTimeoutMs); + } } diff --git a/src/renderer/base/thread.hpp b/src/renderer/base/thread.hpp index e65bb1771..b7d5623dd 100644 --- a/src/renderer/base/thread.hpp +++ b/src/renderer/base/thread.hpp @@ -17,6 +17,8 @@ Author(s): #include "../inc/IRenderer.hpp" #include "../inc/IRenderThread.hpp" +#include + namespace Microsoft::Console::Render { class RenderThread final : public IRenderThread @@ -35,20 +37,14 @@ namespace Microsoft::Console::Render private: static DWORD WINAPI s_ThreadProc(_In_ LPVOID lpParameter); - DWORD WINAPI _ThreadProc(); - - static DWORD const s_FrameLimitMilliseconds = 8; + DWORD _ThreadProc(); HANDLE _hThread; - HANDLE _hEvent; - - HANDLE _hPaintEnabledEvent; - HANDLE _hPaintCompletedEvent; - IRenderer* _pRenderer; // Non-ownership pointer - bool _fKeepRunning; - std::atomic _fNextFrameRequested; - std::atomic _fWaiting; + std::atomic _fKeepRunning; + std::atomic _hPaintCompletedEvent; + til::gate_relaxed _hPaintEnabledEvent; + til::gate_relaxed _fNextFrameRequested; }; } diff --git a/src/renderer/dx/DxRenderer.cpp b/src/renderer/dx/DxRenderer.cpp index 03904814e..3439ed99e 100644 --- a/src/renderer/dx/DxRenderer.cpp +++ b/src/renderer/dx/DxRenderer.cpp @@ -493,7 +493,7 @@ void DxEngine::_ComputePixelShaderSettings() noexcept // actual failure from the API itself. [[nodiscard]] HRESULT DxEngine::_CreateSurfaceHandle() noexcept { - wil::unique_hmodule hDComp{ LoadLibraryEx(L"Dcomp.dll", nullptr, LOAD_LIBRARY_SEARCH_SYSTEM32) }; + wil::unique_hmodule hDComp{ LoadLibraryExW(L"Dcomp.dll", nullptr, LOAD_LIBRARY_SEARCH_SYSTEM32) }; RETURN_LAST_ERROR_IF(hDComp.get() == nullptr); auto fn = GetProcAddressByFunctionDeclaration(hDComp.get(), DCompositionCreateSurfaceHandle);