From 3c384f6d916a83929f106e750e3d4d33e3eaff24 Mon Sep 17 00:00:00 2001 From: Chen-Pang He Date: Wed, 5 Jun 2024 03:38:27 +0800 Subject: [PATCH 1/2] Remove manual memory management (especially delete) to enhance memory safety --- src/Memory.cpp | 36 +++++++++--------------------------- src/Memory.h | 9 +++------ src/System.cpp | 44 +++++++++++++++----------------------------- src/System.h | 2 +- 4 files changed, 28 insertions(+), 63 deletions(-) diff --git a/src/Memory.cpp b/src/Memory.cpp index 4938e180..e563fe07 100644 --- a/src/Memory.cpp +++ b/src/Memory.cpp @@ -11,21 +11,10 @@ #include "Memory.h" -Memory::Memory() -{ -} - - -Memory::~Memory() -{ - Memory::Resize(0, DDS_TT_SMALL, 0, 0); -} - - void Memory::ReturnThread(const unsigned thrId) { - memory[thrId]->transTable->ReturnAllMemory(); - memory[thrId]->memUsed = Memory::MemoryInUseMB(thrId); + memory[thrId].transTable->ReturnAllMemory(); + memory[thrId].memUsed = Memory::MemoryInUseMB(thrId); } @@ -40,12 +29,6 @@ void Memory::Resize( if (memory.size() > n) { - // Downsize. - for (unsigned i = n; i < memory.size(); i++) - { - delete memory[i]->transTable; - delete memory[i]; - } memory.resize(static_cast(n)); threadSizes.resize(static_cast(n)); } @@ -57,22 +40,21 @@ void Memory::Resize( threadSizes.resize(n); for (unsigned i = oldSize; i < n; i++) { - memory[i] = new ThreadData(); if (flag == DDS_TT_SMALL) { - memory[i]->transTable = new TransTableS; + memory[i].transTable.reset(new TransTableS); threadSizes[i] = "S"; } else { - memory[i]->transTable = new TransTableL; + memory[i].transTable.reset(new TransTableL); threadSizes[i] = "L"; } - memory[i]->transTable->SetMemoryDefault(memDefault_MB); - memory[i]->transTable->SetMemoryMaximum(memMaximum_MB); + memory[i].transTable->SetMemoryDefault(memDefault_MB); + memory[i].transTable->SetMemoryMaximum(memMaximum_MB); - memory[i]->transTable->MakeTT(); + memory[i].transTable->MakeTT(); } } } @@ -91,13 +73,13 @@ ThreadData * Memory::GetPtr(const unsigned thrId) cout << "Memory::GetPtr: " << thrId << " vs. " << memory.size() << endl; exit(1); } - return memory[thrId]; + return &memory[thrId]; } double Memory::MemoryInUseMB(const unsigned thrId) const { - return memory[thrId]->transTable->MemoryInUse() + + return memory[thrId].transTable->MemoryInUse() + 8192. * sizeof(relRanksType) / static_cast(1024.); } diff --git a/src/Memory.h b/src/Memory.h index 90b928dc..f3cb29ed 100644 --- a/src/Memory.h +++ b/src/Memory.h @@ -10,6 +10,7 @@ #ifndef DDS_MEMORY_H #define DDS_MEMORY_H +#include #include #include "TransTable.h" @@ -78,7 +79,7 @@ struct ThreadData // 960 KB relRanksType rel[8192]; - TransTable * transTable; + std::unique_ptr transTable; Moves moves; @@ -116,16 +117,12 @@ class Memory { private: - vector memory; + vector memory; vector threadSizes; public: - Memory(); - - ~Memory(); - void ReturnThread(const unsigned thrId); void Resize( diff --git a/src/System.cpp b/src/System.cpp index f71f8262..a0f3e4df 100644 --- a/src/System.cpp +++ b/src/System.cpp @@ -83,11 +83,6 @@ System::System() } -System::~System() -{ -} - - void System::Reset() { runCat = DDS_RUN_SOLVE; @@ -434,19 +429,16 @@ int System::RunThreadsGCD() int System::RunThreadsBoost() { #ifdef DDS_THREADS_BOOST - vector threads; + vector threads; const unsigned nu = static_cast(numThreads); - threads.resize(nu); + threads.reserve(nu); for (unsigned k = 0; k < nu; k++) - threads[k] = new boost::thread(fptr, k); + threads.emplace_back(fptr, k); - for (unsigned k = 0; k < nu; k++) - { - threads[k]->join(); - delete threads[k]; - } + for (boost::thread & t : threads) + t.join(); #endif return RETURN_NO_FAULT; @@ -460,23 +452,20 @@ int System::RunThreadsBoost() int System::RunThreadsSTL() { #ifdef DDS_THREADS_STL - vector threads; + vector threads; vector uniques; vector crossrefs; (* CallbackDuplList[runCat])(* bop, uniques, crossrefs); const unsigned nu = static_cast(numThreads); - threads.resize(nu); + threads.reserve(nu); for (unsigned k = 0; k < nu; k++) - threads[k] = new thread(fptr, k); + threads.emplace_back(fptr, k); - for (unsigned k = 0; k < nu; k++) - { - threads[k]->join(); - delete threads[k]; - } + for (thread & t : threads) + t.join(); #endif return RETURN_NO_FAULT; @@ -534,19 +523,16 @@ int System::RunThreadsSTLIMPL() int System::RunThreadsTBB() { #ifdef DDS_THREADS_TBB - vector threads; + vector threads; const unsigned nu = static_cast(numThreads); - threads.resize(nu); + threads.reserve(nu); for (unsigned k = 0; k < nu; k++) - threads[k] = new tbb::tbb_thread(fptr, k); + threads.emplace_back(fptr, k); - for (unsigned k = 0; k < nu; k++) - { - threads[k]->join(); - delete threads[k]; - } + for (tbb::tbb_thread & t : threads) + t.join(); #endif return RETURN_NO_FAULT; diff --git a/src/System.h b/src/System.h index 9f78fea3..9db6ef64 100644 --- a/src/System.h +++ b/src/System.h @@ -81,7 +81,7 @@ class System public: System(); - ~System(); + ~System() = default; void Reset(); From a19f328f0d2734c7dada70d25322821da02b9ba2 Mon Sep 17 00:00:00 2001 From: Chen-Pang He Date: Wed, 5 Jun 2024 03:38:57 +0800 Subject: [PATCH 2/2] Replace empty destructors with default ones to allow inlining --- src/ABstats.cpp | 5 ----- src/ABstats.h | 2 +- src/Moves.cpp | 5 ----- src/Moves.h | 2 +- src/Scheduler.cpp | 5 ----- src/Scheduler.h | 2 +- src/ThreadMgr.cpp | 5 ----- src/ThreadMgr.h | 2 +- src/TimeStat.cpp | 5 ----- src/TimeStat.h | 2 +- src/TimeStatList.cpp | 11 ----------- src/TimeStatList.h | 4 ---- src/Timer.cpp | 5 ----- src/Timer.h | 2 +- src/TimerGroup.cpp | 5 ----- src/TimerGroup.h | 2 +- src/TimerList.cpp | 5 ----- src/TimerList.h | 2 +- src/TransTable.h | 4 ++-- 19 files changed, 10 insertions(+), 65 deletions(-) diff --git a/src/ABstats.cpp b/src/ABstats.cpp index f45ec888..5c03d5e9 100644 --- a/src/ABstats.cpp +++ b/src/ABstats.cpp @@ -24,11 +24,6 @@ ABstats::ABstats() } -ABstats::~ABstats() -{ -} - - void ABstats::Reset() { for (int depth = 0; depth < DDS_MAXDEPTH; depth++) diff --git a/src/ABstats.h b/src/ABstats.h index 3312d410..3ff357c6 100644 --- a/src/ABstats.h +++ b/src/ABstats.h @@ -106,7 +106,7 @@ class ABstats ABstats(); - ~ABstats(); + ~ABstats() = default; void Reset(); diff --git a/src/Moves.cpp b/src/Moves.cpp index c2e5e629..b9e054cd 100644 --- a/src/Moves.cpp +++ b/src/Moves.cpp @@ -97,11 +97,6 @@ Moves::Moves() } -Moves::~Moves() -{ -} - - void Moves::Init( const int tricks, const int relStartHand, diff --git a/src/Moves.h b/src/Moves.h index 46ce2089..bbe7a5e3 100644 --- a/src/Moves.h +++ b/src/Moves.h @@ -176,7 +176,7 @@ class Moves public: Moves(); - ~Moves(); + ~Moves() = default; void Init( const int tricks, diff --git a/src/Scheduler.cpp b/src/Scheduler.cpp index b3d58b41..f4fabbfd 100644 --- a/src/Scheduler.cpp +++ b/src/Scheduler.cpp @@ -85,11 +85,6 @@ void Scheduler::InitTimes() #endif -Scheduler::~Scheduler() -{ -} - - void Scheduler::Reset() { for (int b = 0; b < MAXNOOFBOARDS; b++) diff --git a/src/Scheduler.h b/src/Scheduler.h index 606f9953..6134fa0c 100644 --- a/src/Scheduler.h +++ b/src/Scheduler.h @@ -162,7 +162,7 @@ class Scheduler Scheduler(); - ~Scheduler(); + ~Scheduler() = default; void RegisterThreads( const int n); diff --git a/src/ThreadMgr.cpp b/src/ThreadMgr.cpp index 5ee45489..9380bb24 100644 --- a/src/ThreadMgr.cpp +++ b/src/ThreadMgr.cpp @@ -29,11 +29,6 @@ ThreadMgr::ThreadMgr() } -ThreadMgr::~ThreadMgr() -{ -} - - void ThreadMgr::Reset(const int nThreads) { const unsigned n = static_cast(nThreads); diff --git a/src/ThreadMgr.h b/src/ThreadMgr.h index a7653a52..e0a08b3d 100644 --- a/src/ThreadMgr.h +++ b/src/ThreadMgr.h @@ -29,7 +29,7 @@ class ThreadMgr ThreadMgr(); - ~ThreadMgr(); + ~ThreadMgr() = default; void Reset(const int nThreads); diff --git a/src/TimeStat.cpp b/src/TimeStat.cpp index ff15b08e..a535f555 100644 --- a/src/TimeStat.cpp +++ b/src/TimeStat.cpp @@ -22,11 +22,6 @@ TimeStat::TimeStat() } -TimeStat::~TimeStat() -{ -} - - void TimeStat::Reset() { number = 0; diff --git a/src/TimeStat.h b/src/TimeStat.h index 0a3b630a..aedb2640 100644 --- a/src/TimeStat.h +++ b/src/TimeStat.h @@ -27,7 +27,7 @@ class TimeStat TimeStat(); - ~TimeStat(); + ~TimeStat() = default; void Reset(); diff --git a/src/TimeStatList.cpp b/src/TimeStatList.cpp index 6a60c5f4..908612ea 100644 --- a/src/TimeStatList.cpp +++ b/src/TimeStatList.cpp @@ -15,17 +15,6 @@ #include "TimeStatList.h" -TimeStatList::TimeStatList() -{ - TimeStatList::Reset(); -} - - -TimeStatList::~TimeStatList() -{ -} - - void TimeStatList::Reset() { } diff --git a/src/TimeStatList.h b/src/TimeStatList.h index 53bb20c7..c139c1b5 100644 --- a/src/TimeStatList.h +++ b/src/TimeStatList.h @@ -28,10 +28,6 @@ class TimeStatList public: - TimeStatList(); - - ~TimeStatList(); - void Reset(); void Init( diff --git a/src/Timer.cpp b/src/Timer.cpp index f6ee618e..d7c6d18a 100644 --- a/src/Timer.cpp +++ b/src/Timer.cpp @@ -24,11 +24,6 @@ Timer::Timer() } -Timer::~Timer() -{ -} - - void Timer::Reset() { name = ""; diff --git a/src/Timer.h b/src/Timer.h index 31ac9385..4e91228a 100644 --- a/src/Timer.h +++ b/src/Timer.h @@ -35,7 +35,7 @@ class Timer Timer(); - ~Timer(); + ~Timer() = default; void Reset(); diff --git a/src/TimerGroup.cpp b/src/TimerGroup.cpp index bf48aa32..c8829b50 100644 --- a/src/TimerGroup.cpp +++ b/src/TimerGroup.cpp @@ -22,11 +22,6 @@ TimerGroup::TimerGroup() } -TimerGroup::~TimerGroup() -{ -} - - void TimerGroup::Reset() { timers.resize(TIMER_DEPTH); diff --git a/src/TimerGroup.h b/src/TimerGroup.h index ef94c45b..3d7bd267 100644 --- a/src/TimerGroup.h +++ b/src/TimerGroup.h @@ -29,7 +29,7 @@ class TimerGroup TimerGroup(); - ~TimerGroup(); + ~TimerGroup() = default; void Reset(); diff --git a/src/TimerList.cpp b/src/TimerList.cpp index 1722ff57..315d71ac 100644 --- a/src/TimerList.cpp +++ b/src/TimerList.cpp @@ -24,11 +24,6 @@ TimerList::TimerList() } -TimerList::~TimerList() -{ -} - - void TimerList::Reset() { timerGroups.resize(TIMER_NO_SIZE); diff --git a/src/TimerList.h b/src/TimerList.h index 6838d4a3..e5245e31 100644 --- a/src/TimerList.h +++ b/src/TimerList.h @@ -87,7 +87,7 @@ class TimerList public: TimerList(); - ~TimerList(); + ~TimerList() = default; void Reset(); diff --git a/src/TransTable.h b/src/TransTable.h index ab97f3f5..e91a751e 100644 --- a/src/TransTable.h +++ b/src/TransTable.h @@ -64,9 +64,9 @@ struct nodeCardsType // 8 bytes class TransTable { public: - TransTable(){}; + TransTable() = default; - virtual ~TransTable(){}; + virtual ~TransTable() = default; virtual void Init(const int handLookup[][15]){};