From 19ddeeded94eedce5fc7db365e25dc0a4c56d647 Mon Sep 17 00:00:00 2001 From: Esteban Papp <81431996+amznestebanpapp@users.noreply.github.com> Date: Thu, 2 Dec 2021 12:40:39 -0800 Subject: [PATCH] Removes ThreadDrillerEvents, replaces handlers with TreadEventBus Signed-off-by: Esteban Papp <81431996+amznestebanpapp@users.noreply.github.com> --- Code/Framework/AtomCore/Tests/Main.cpp | 4 +-- .../AzCore/AzCore/Debug/EventTraceDriller.cpp | 4 +-- .../AzCore/AzCore/Debug/EventTraceDriller.h | 2 +- .../AzCore/AzCore/Debug/MemoryProfiler.h | 2 +- .../AzCore/AzCore/std/parallel/threadbus.h | 17 ------------ .../std/parallel/internal/thread_UnixLike.cpp | 2 -- .../std/parallel/internal/thread_WinAPI.cpp | 2 -- .../Framework/AzCore/Tests/AZStd/Parallel.cpp | 27 ++++++------------- 8 files changed, 14 insertions(+), 46 deletions(-) diff --git a/Code/Framework/AtomCore/Tests/Main.cpp b/Code/Framework/AtomCore/Tests/Main.cpp index eb4c6bc835..728624d427 100644 --- a/Code/Framework/AtomCore/Tests/Main.cpp +++ b/Code/Framework/AtomCore/Tests/Main.cpp @@ -37,7 +37,7 @@ namespace AZ using namespace AZ; // Handle asserts -class TraceDrillerHook +class TestEnvironmentHook : public AZ::Test::ITestEnvironment , public UnitTest::TraceBusRedirector { @@ -57,5 +57,5 @@ public: } }; -AZ_UNIT_TEST_HOOK(new TraceDrillerHook()); +AZ_UNIT_TEST_HOOK(new TestEnvironmentHook()); diff --git a/Code/Framework/AzCore/AzCore/Debug/EventTraceDriller.cpp b/Code/Framework/AzCore/AzCore/Debug/EventTraceDriller.cpp index d0722616f3..cdaf7ec56f 100644 --- a/Code/Framework/AzCore/AzCore/Debug/EventTraceDriller.cpp +++ b/Code/Framework/AzCore/AzCore/Debug/EventTraceDriller.cpp @@ -29,12 +29,12 @@ namespace AZ::Debug EventTraceDriller::EventTraceDriller() { EventTraceDrillerSetupBus::Handler::BusConnect(); - AZStd::ThreadDrillerEventBus::Handler::BusConnect(); + AZStd::ThreadEventBus::Handler::BusConnect(); } EventTraceDriller::~EventTraceDriller() { - AZStd::ThreadDrillerEventBus::Handler::BusDisconnect(); + AZStd::ThreadEventBus::Handler::BusDisconnect(); EventTraceDrillerSetupBus::Handler::BusDisconnect(); } diff --git a/Code/Framework/AzCore/AzCore/Debug/EventTraceDriller.h b/Code/Framework/AzCore/AzCore/Debug/EventTraceDriller.h index fee71147ca..b96a1a88f4 100644 --- a/Code/Framework/AzCore/AzCore/Debug/EventTraceDriller.h +++ b/Code/Framework/AzCore/AzCore/Debug/EventTraceDriller.h @@ -22,7 +22,7 @@ namespace AZ : public Driller , public EventTraceDrillerBus::Handler , public EventTraceDrillerSetupBus::Handler - , public AZStd::ThreadDrillerEventBus::Handler + , public AZStd::ThreadEventBus::Handler , public AZ::TickBus::Handler { public: diff --git a/Code/Framework/AzCore/AzCore/Debug/MemoryProfiler.h b/Code/Framework/AzCore/AzCore/Debug/MemoryProfiler.h index 5b22e20c60..827f00134d 100644 --- a/Code/Framework/AzCore/AzCore/Debug/MemoryProfiler.h +++ b/Code/Framework/AzCore/AzCore/Debug/MemoryProfiler.h @@ -8,7 +8,7 @@ #pragma once #ifndef AZ_PROFILE_MEMORY_ALLOC -// No other profiler has defined the performance markers AZ_PROFILE_MEMORY_ALLOC (and friends), fall back to a Driller implementation (currently empty) +// No other profiler has defined the performance markers AZ_PROFILE_MEMORY_ALLOC (and friends), fall back to current implementation (empty) # define AZ_PROFILE_MEMORY_ALLOC(category, address, size, context) # define AZ_PROFILE_MEMORY_ALLOC_EX(category, filename, lineNumber, address, size, context) # define AZ_PROFILE_MEMORY_FREE(category, address) diff --git a/Code/Framework/AzCore/AzCore/std/parallel/threadbus.h b/Code/Framework/AzCore/AzCore/std/parallel/threadbus.h index 26c2f92edb..2caf4df522 100644 --- a/Code/Framework/AzCore/AzCore/std/parallel/threadbus.h +++ b/Code/Framework/AzCore/AzCore/std/parallel/threadbus.h @@ -32,24 +32,7 @@ namespace AZStd virtual void OnThreadExit(const AZStd::thread::id& id) = 0; }; - //! Thread events driller bus - only "drillers" (profilers) should connect to this. - //! A global mutex that includes a lock on the memory manager and other driller busses - //! is held during dispatch, and listeners are expected to do no allocation - //! or thread workloads or blocking or mutex operations of their own - only dump the data to - //! network or file ASAP. - //! DO NOT USE this bus unless you are a profiler capture system, use the ThreadEvents / ThreadBus instead - class ThreadDrillerEvents - : public AZ::Debug::DrillerEBusTraits - { - public: - /// Called when we enter a thread, optional thread_desc is provided when the use provides one. - virtual void OnThreadEnter(const AZStd::thread::id& id, const AZStd::thread_desc* desc) = 0; - /// Called when we exit a thread. - virtual void OnThreadExit(const AZStd::thread::id& id) = 0; - }; - typedef AZ::EBus ThreadEventBus; - typedef AZ::EBus ThreadDrillerEventBus; } #endif // AZSTD_THREAD_BUS_H diff --git a/Code/Framework/AzCore/Platform/Common/UnixLike/AzCore/std/parallel/internal/thread_UnixLike.cpp b/Code/Framework/AzCore/Platform/Common/UnixLike/AzCore/std/parallel/internal/thread_UnixLike.cpp index b615f677f1..da5d95b9a4 100644 --- a/Code/Framework/AzCore/Platform/Common/UnixLike/AzCore/std/parallel/internal/thread_UnixLike.cpp +++ b/Code/Framework/AzCore/Platform/Common/UnixLike/AzCore/std/parallel/internal/thread_UnixLike.cpp @@ -35,7 +35,6 @@ namespace AZStd destroy_thread_info(ti); ThreadEventBus::Broadcast(&ThreadEventBus::Events::OnThreadExit, this_thread::get_id()); - ThreadDrillerEventBus::Broadcast(&ThreadDrillerEventBus::Events::OnThreadExit, this_thread::get_id()); pthread_exit(nullptr); return nullptr; } @@ -88,7 +87,6 @@ namespace AZStd Platform::PostCreateThread(tId, name, cpuId); ThreadEventBus::Broadcast(&ThreadEventBus::Events::OnThreadEnter, thread::id(tId), desc); - ThreadDrillerEventBus::Broadcast(&ThreadDrillerEventBus::Events::OnThreadEnter, thread::id(tId), desc); return tId; } } diff --git a/Code/Framework/AzCore/Platform/Common/WinAPI/AzCore/std/parallel/internal/thread_WinAPI.cpp b/Code/Framework/AzCore/Platform/Common/WinAPI/AzCore/std/parallel/internal/thread_WinAPI.cpp index c84b4ecd98..cd638a34fa 100644 --- a/Code/Framework/AzCore/Platform/Common/WinAPI/AzCore/std/parallel/internal/thread_WinAPI.cpp +++ b/Code/Framework/AzCore/Platform/Common/WinAPI/AzCore/std/parallel/internal/thread_WinAPI.cpp @@ -38,7 +38,6 @@ namespace AZStd destroy_thread_info(ti); ThreadEventBus::Broadcast(&ThreadEventBus::Events::OnThreadExit, this_thread::get_id()); // goes to client listeners - ThreadDrillerEventBus::Broadcast(&ThreadDrillerEventBus::Events::OnThreadExit, this_thread::get_id()); // goes to the profiler. return Platform::PostThreadRun(); } @@ -73,7 +72,6 @@ namespace AZStd } ThreadEventBus::Broadcast(&ThreadEventBus::Events::OnThreadEnter, thread::id(*id), desc); - ThreadDrillerEventBus::Broadcast(&ThreadDrillerEventBus::Events::OnThreadEnter, thread::id(*id), desc); ::ResumeThread(hThread); diff --git a/Code/Framework/AzCore/Tests/AZStd/Parallel.cpp b/Code/Framework/AzCore/Tests/AZStd/Parallel.cpp index 6b9202133c..9c59cd450e 100644 --- a/Code/Framework/AzCore/Tests/AZStd/Parallel.cpp +++ b/Code/Framework/AzCore/Tests/AZStd/Parallel.cpp @@ -1438,44 +1438,33 @@ namespace UnitTest TEST_F(ThreadEventsBus, Broadcasts_BothBusses) { ThreadEventCounter eventBusCounter; - ThreadEventCounter drillerBusCounter; auto thread_function = [&]() { ; // intentionally left blank }; eventBusCounter.Connect(); - drillerBusCounter.Connect(); AZStd::thread starter = AZStd::thread(thread_function); starter.join(); - EXPECT_EQ(drillerBusCounter.m_enterCount, 1); - EXPECT_EQ(drillerBusCounter.m_exitCount, 1); EXPECT_EQ(eventBusCounter.m_enterCount, 1); EXPECT_EQ(eventBusCounter.m_exitCount, 1); eventBusCounter.Disconnect(); - drillerBusCounter.Disconnect(); } - // this class tests for deadlocks caused by interactions between the thread - // driller bus and the other driller busses. - // Client code (ie, not part of the driller system) can connec to the - // ThreadEventBus and be told when threads are started and stopped - // However, if they instead listen to the ThreadDrillerEventBus, a deadlock condition - // could be caused if they lock a mutex that another thread needs in order to proceed. - // This test makes sure that using the ThreadEventBus (ie, the one meant for client code) - // instead of the ThreadDrillerEventBus (the one meant only for profilers) does NOT cause - // a deadlock. + // This class tests for deadlocks caused by multiple threads interacting with the ThreadEventBus. + // Client code can connect to the ThreadEventBus and be told when threads are started and stopped. + // A deadlock condition could be caused if they lock a mutex that another thread needs in order to proceed. + // This test makes sure that using the ThreadEventBus does NOT cause a deadlock. // We will simulate this series of events by doing the following // 1. Main thread listens on the ThreadEventBus // 2. OnThreadExit will lock a mutex, perform an allocation, unlock a mutex // 3. The thread itself will lock the mutex, perform an allocation, unlock the mutex. - // As long as there is no cross talk between the client and the driller busses, the - // above operation should not deadlock. - // but if there is, then a deadlock can occur where one thread will be unable to perform - // its allocation because the other is in OnThreadExit() - // and the other will not be able to perform OnThreadExit() because it cannot lock the mutex. + // As long as there is no cross talk between threads, the above operation should not deadlock. + // If there is, then a deadlock can occur where one thread will be unable to perform + // its allocation because the other is in OnThreadExit() and the other will not be able to perform + // OnThreadExit() because it cannot lock the mutex. class ThreadEventsDeathTest : public AllocatorsFixture