Skip to content

Implemented runtime thread count resizing for execution pool#1

Open
SerhiiHrabarskiy wants to merge 2 commits intomasterfrom
resizable-execution-pool
Open

Implemented runtime thread count resizing for execution pool#1
SerhiiHrabarskiy wants to merge 2 commits intomasterfrom
resizable-execution-pool

Conversation

@SerhiiHrabarskiy
Copy link
Copy Markdown
Collaborator

No description provided.

Comment thread include/execq/internal/ExecutionPool.h
Comment thread src/ExecutionPool.cpp
Comment thread src/ExecutionPool.cpp Outdated
Comment thread src/ThreadWorker.cpp Outdated
Comment thread src/ThreadWorker.cpp Outdated
Comment thread src/ThreadWorker.cpp Outdated
Comment thread tests/ExecqTestUtil.h
{
public:
MOCK_CONST_METHOD1(createWorker, std::unique_ptr<execq::impl::IThreadWorker>(execq::impl::ITaskProvider& provider));
MOCK_CONST_METHOD2(createWorker, std::unique_ptr<execq::impl::IThreadWorker>(execq::impl::ITaskProvider& provider, execq::impl::ThreadStopCb cb));
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use modern mock style ?
like MOCK_METHOD(bool, name, (), (const, override));

Comment thread src/ExecutionPool.cpp

namespace
{
static constexpr uint64_t packThreadsCount(uint32_t target, uint32_t current)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'packThreadsCount' is a static definition in anonymous namespace; static is redundant here

Comment thread src/ExecutionPool.cpp
{
return (uint64_t(target) << 32) | uint64_t(current);
}
static constexpr uint32_t unpackTarget(uint64_t threadCount)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is a static definition in anonymous namespace; static is redundant here

Comment thread src/ExecutionPool.cpp
{
return uint32_t(threadCount >> 32);
}
static constexpr uint32_t unpackCurrent(uint64_t threadCount)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is a static definition in anonymous namespace; static is redundant here

Comment thread src/ExecutionPool.cpp
}
static constexpr uint32_t unpackCurrent(uint64_t threadCount)
{
return uint32_t(threadCount & 0xFFFFFFFFu);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment thread src/ThreadWorker.cpp
{
public:
virtual std::unique_ptr<IThreadWorker> createWorker(ITaskProvider& provider) const final
virtual std::unique_ptr<IThreadWorker> createWorker(ITaskProvider& provider, ThreadStopCb cb = nullptr) const final
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'virtual' is redundant since the function is already declared 'final'

Comment thread src/ThreadWorker.cpp
virtual std::unique_ptr<IThreadWorker> createWorker(ITaskProvider& provider, ThreadStopCb cb = nullptr) const final
{
return std::unique_ptr<IThreadWorker>(new ThreadWorker(provider));
return std::unique_ptr<IThreadWorker>(new ThreadWorker(provider, cb));
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

std::make_uniquestd::thread

Comment thread src/ThreadWorker.cpp
virtual ~ThreadWorker();

virtual bool notifyWorker() final;
virtual bool finished() final;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'virtual' is redundant since the function is already declared 'final'

virtual void notifyAllWorkers() final;


virtual void setThreadCount(uint32_t threadCount) final;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'virtual' is redundant since the function is already declared 'final'

namespace details
{
bool NotifyWorkers(const std::vector<std::unique_ptr<IThreadWorker>>& workers, const bool single);
bool NotifyWorkers(const ThreadWorkers& workers, const bool single);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need const bool ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants