Skip to content

Commit

Permalink
Re #841. Changed the Mutex raw pointer in Task to a boost::shared_ptr.
Browse files Browse the repository at this point in the history
  • Loading branch information
peterfpeterson committed Dec 10, 2013
1 parent 5fb852c commit 9d5ea6e
Show file tree
Hide file tree
Showing 7 changed files with 36 additions and 36 deletions.
6 changes: 3 additions & 3 deletions Code/Mantid/Framework/DataHandling/src/LoadEventNexus.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -452,7 +452,7 @@ class LoadBankFromDiskTask : public Task
*/
LoadBankFromDiskTask(LoadEventNexus * alg, const std::string& entry_name, const std::string & entry_type,
const std::size_t numEvents, const bool oldNeXusFileNames,
Progress * prog, Mutex * ioMutex, ThreadScheduler * scheduler)
Progress * prog, boost::shared_ptr<Mutex> ioMutex, ThreadScheduler * scheduler)
: Task(),
alg(alg), entry_name(entry_name), entry_type(entry_type),
pixelID_to_wi_vector(alg->pixelID_to_wi_vector), pixelID_to_wi_offset(alg->pixelID_to_wi_offset),
Expand Down Expand Up @@ -1606,7 +1606,7 @@ void LoadEventNexus::loadEvents(API::Progress * const prog, const bool monitors)
// Make the thread pool
ThreadScheduler * scheduler = new ThreadSchedulerMutexes();
ThreadPool pool(scheduler);
Mutex * diskIOMutex = new Mutex();
boost::shared_ptr<Mutex> diskIOMutex(new Mutex());
size_t bank0 = 0;
size_t bankn = bankNames.size();

Expand Down Expand Up @@ -1680,7 +1680,7 @@ void LoadEventNexus::loadEvents(API::Progress * const prog, const bool monitors)
}
// Start and end all threads
pool.joinAll();
delete diskIOMutex;
diskIOMutex.reset();
delete prog2;


Expand Down
11 changes: 6 additions & 5 deletions Code/Mantid/Framework/Kernel/inc/MantidKernel/Task.h
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#ifndef MANTID_KERNEL_TASK_H_
#define MANTID_KERNEL_TASK_H_

#include <boost/shared_ptr.hpp>
#include "MantidKernel/DllConfig.h"
#include "MantidKernel/Exception.h"
#include "MantidKernel/MultiThreaded.h"
Expand All @@ -27,7 +28,7 @@ namespace Kernel
//---------------------------------------------------------------------------------------------
/** Default constructor */
Task() :
m_cost(1.0), m_mutex(NULL)
m_cost(1.0)
{ }

//---------------------------------------------------------------------------------------------
Expand All @@ -36,7 +37,7 @@ namespace Kernel
* @param cost :: computational cost
*/
Task(double cost) :
m_cost(cost), m_mutex(NULL)
m_cost(cost)
{ }

/// Destructor
Expand Down Expand Up @@ -73,7 +74,7 @@ namespace Kernel
/** Get the mutex object for this Task
* @return Mutex pointer, or NULL
*/
Mutex * getMutex()
boost::shared_ptr<Mutex> getMutex()
{
return m_mutex;
}
Expand All @@ -82,7 +83,7 @@ namespace Kernel
/** Set the mutex object for this Task
* @param mutex :: Mutex pointer, or NULL
*/
void setMutex(Mutex * mutex)
void setMutex( boost::shared_ptr<Mutex> &mutex)
{
m_mutex = mutex;
}
Expand All @@ -94,7 +95,7 @@ namespace Kernel
double m_cost;

/// Mutex associated with this task (can be NULL)
Mutex * m_mutex;
boost::shared_ptr<Mutex> m_mutex;
};


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ namespace Kernel
m_queueLock.lock();
m_cost += newTask->cost();

Mutex * mut = newTask->getMutex();
boost::shared_ptr<Mutex> mut = newTask->getMutex();
m_supermap[mut].insert( std::pair<double, Task*>(newTask->cost(), newTask) );
m_queueLock.unlock();
}
Expand All @@ -68,7 +68,7 @@ namespace Kernel
for (; it != it_end; ++it)
{
// The key is the mutex associated with the inner map
Mutex * mapMutex = it->first;
boost::shared_ptr<Mutex> mapMutex = it->first;
if ((!mapMutex) || (m_mutexes.empty()) || (m_mutexes.find(mapMutex) == m_mutexes.end()))
{
// The mutex of this map is free!
Expand Down Expand Up @@ -112,7 +112,7 @@ namespace Kernel
// --- Add the mutex (if any) to the list of "busy" ones ---
if (temp)
{
Mutex * mut = temp->getMutex();
boost::shared_ptr<Mutex> mut = temp->getMutex();
if (mut)
m_mutexes.insert(mut);
}
Expand All @@ -134,7 +134,7 @@ namespace Kernel
virtual void finished(Task * task, size_t threadnum)
{
UNUSED_ARG(threadnum);
Mutex * mut = task->getMutex();
boost::shared_ptr<Mutex> mut = task->getMutex();
if (mut)
{
m_queueLock.lock();
Expand Down Expand Up @@ -199,14 +199,14 @@ namespace Kernel
/// Map to tasks, sorted by cost
typedef std::multimap<double, Task*> InnerMap;
/// Map to maps, sorted by Mutex*
typedef std::map<Mutex*, InnerMap> SuperMap;
typedef std::map<boost::shared_ptr<Mutex>, InnerMap> SuperMap;

/** A super map; first key = a Mutex *
* Inside it: second key = the cost. */
SuperMap m_supermap;

/// Vector of currently used mutexes.
std::set<Mutex *> m_mutexes;
std::set<boost::shared_ptr<Mutex> > m_mutexes;

};

Expand Down
4 changes: 2 additions & 2 deletions Code/Mantid/Framework/Kernel/src/ThreadPoolRunnable.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,8 @@ namespace Kernel
if (task)
{
//Task-specific mutex if specified?
Mutex * mutex = task->getMutex();
if (mutex)
boost::shared_ptr<Mutex> mutex = task->getMutex();
if (bool(mutex))
mutex->lock();

try
Expand Down
3 changes: 1 addition & 2 deletions Code/Mantid/Framework/Kernel/test/TaskTest.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,9 @@ class TaskTest : public CxxTest::TestSuite
void test_mutex()
{
MyTask t;
Mutex * mut = new Mutex();
boost::shared_ptr<Mutex> mut(new Mutex());
t.setMutex(mut);
TS_ASSERT_EQUALS( mut, t.getMutex() );
delete mut;
}


Expand Down
4 changes: 2 additions & 2 deletions Code/Mantid/Framework/Kernel/test/ThreadPoolTest.h
Original file line number Diff line number Diff line change
Expand Up @@ -382,13 +382,13 @@ class ThreadPoolTest : public CxxTest::TestSuite
TimeWaster mywaster;
size_t num = 30000;
mywaster.total = 0;
Mutex * lastMutex = NULL;
boost::shared_ptr<Mutex> lastMutex;
for (size_t i=0; i<=num; i++)
{
Task * task = new FunctionTask( boost::bind(&TimeWaster::add_to_number, &mywaster, i), static_cast<double>(i) );
// Create a new mutex every 1000 tasks. This is more relevant to the ThreadSchedulerMutexes; others ignore it.
if (i % 1000 == 0)
lastMutex = new Mutex();
lastMutex = boost::shared_ptr<Mutex>(new Mutex());
task->setMutex(lastMutex);
p.schedule( task );
}
Expand Down
32 changes: 16 additions & 16 deletions Code/Mantid/Framework/Kernel/test/ThreadSchedulerMutexesTest.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ class ThreadSchedulerMutexesTest : public CxxTest::TestSuite
class TaskWithMutex : public Task
{
public:
TaskWithMutex(Mutex * mutex, double cost)
TaskWithMutex(boost::shared_ptr<Mutex> mutex, double cost)
{
m_mutex = mutex;
m_cost = cost;
Expand All @@ -44,8 +44,8 @@ class ThreadSchedulerMutexesTest : public CxxTest::TestSuite
void test_push()
{
ThreadSchedulerMutexes sc;
Mutex * mut1 = new Mutex();
Mutex * mut2 = new Mutex();
boost::shared_ptr<Mutex> mut1(new Mutex());
boost::shared_ptr<Mutex> mut2(new Mutex());
TaskWithMutex * task1 = new TaskWithMutex(mut1, 10.0);
TaskWithMutex * task2 = new TaskWithMutex(mut2, 9.0);

Expand All @@ -56,23 +56,23 @@ class ThreadSchedulerMutexesTest : public CxxTest::TestSuite

// delete task1;
// delete task2;
delete mut1;
delete mut2;
// delete mut1;
// delete mut2;
}

void test_queue()
{
ThreadSchedulerMutexes sc;
Mutex * mut1 = new Mutex();
Mutex * mut2 = new Mutex();
Mutex * mut3 = new Mutex();
boost::shared_ptr<Mutex> mut1(new Mutex());
boost::shared_ptr<Mutex> mut2(new Mutex());
boost::shared_ptr<Mutex> mut3(new Mutex());
TaskWithMutex * task1 = new TaskWithMutex(mut1, 10.0);
TaskWithMutex * task2 = new TaskWithMutex(mut1, 9.0);
TaskWithMutex * task3 = new TaskWithMutex(mut1, 8.0);
TaskWithMutex * task4 = new TaskWithMutex(mut2, 7.0);
TaskWithMutex * task5 = new TaskWithMutex(mut2, 6.0);
TaskWithMutex * task6 = new TaskWithMutex(mut3, 5.0);
TaskWithMutex * task7 = new TaskWithMutex(NULL, 4.0);
TaskWithMutex * task7 = new TaskWithMutex(boost::shared_ptr<Mutex>(), 4.0);
sc.push(task1);
sc.push(task2);
sc.push(task3);
Expand Down Expand Up @@ -130,17 +130,17 @@ class ThreadSchedulerMutexesTest : public CxxTest::TestSuite
// delete task5;
// delete task6;
// delete task7;
delete mut1;
delete mut2;
delete mut3;
// delete mut1;
// delete mut2;
// delete mut3;
}

void test_clear()
{
ThreadSchedulerMutexes sc;
for (size_t i=0; i<10; i++)
{
TaskWithMutex * task = new TaskWithMutex(new Mutex(), 10.0);
TaskWithMutex * task = new TaskWithMutex(boost::shared_ptr<Mutex>(new Mutex()), 10.0);
sc.push(task);
}
TS_ASSERT_EQUALS(sc.size(), 10);
Expand All @@ -155,7 +155,7 @@ class ThreadSchedulerMutexesTest : public CxxTest::TestSuite
{
ThreadSchedulerMutexes sc;
Timer tim0;
Mutex * mut1 = new Mutex();
boost::shared_ptr<Mutex> mut1(new Mutex());
size_t num = 500;
for (size_t i=0; i < num; i++)
{
Expand All @@ -172,7 +172,7 @@ class ThreadSchedulerMutexesTest : public CxxTest::TestSuite
//std::cout << tim1.elapsed() << " secs to pop." << std::endl;
TS_ASSERT_EQUALS( sc.size(), 0);

delete mut1;
// delete mut1;
}

void test_performance_lotsOfMutexes()
Expand All @@ -182,7 +182,7 @@ class ThreadSchedulerMutexesTest : public CxxTest::TestSuite
size_t num = 500;
for (size_t i=0; i < num; i++)
{
sc.push(new TaskWithMutex(new Mutex(), 10.0));
sc.push(new TaskWithMutex(boost::shared_ptr<Mutex>(new Mutex()), 10.0));
}
//std::cout << tim0.elapsed() << " secs to push." << std::endl;
TS_ASSERT_EQUALS( sc.size(), num);
Expand Down

0 comments on commit 9d5ea6e

Please sign in to comment.