Interesting, I just debugged a little, I first set a breakpoint in the line:
inline void cbThreadedTask::Abort()
{
m_abort = true;
}
[debug]> p &m_abort
[debug]$2 = (bool *) 0xd9e3efc
[debug]> p m_threads
[debug]$3 = std::vector of length 1, capacity 1 = {0xd555fb8}
After that I set a breakpoint in the line:
inline bool cbThreadedTask::TestDestroy() const
{
return m_abort;
}
[debug]> p &m_abort
[debug]$5 = (bool *) 0xda25fec
Normally, when a task is running in the thread pool(note that our thread pool only runs one thread), when you try to stop the parsing, the running task should be stopped, and the queued task should be deleted.
Thanks Ollydbg,
After doing more tracing, I understand your explanation.
I also see that just after setting m_abort, the parserThreads are immediately deleted by
cbThreadPool::AbortAllTasks()
std::for_each(m_tasksQueue.begin(), m_tasksQueue.end(), std::mem_fun_ref(&cbThreadedTaskElement::Delete));
so it doesn't even matter if each ParserThread is posted to abort or not.
Although they are in fact posted by cbThreadPool at
void cbThreadPool::cbWorkerThread::AbortTask()
{
wxMutexLocker lock(m_taskMutex);
if (m_pTask)
m_pTask->Abort();
}
But traping on the ParserThreads call to cbThreadedTask::TestDestroy() always shows that m_abort is always false, never true. (except for the main Parser thread you mentioned above.
inline bool cbThreadedTask::TestDestroy() const
{
return m_abort;
}
So I'm guessing, then, that the 20 seconds (on my laptop) to close CodeBlocks.cbp (windows 7) is caused by the delete of each ParserThread. About 432 of them (give or take a few when I hit close project).
I'll do some more tracing to confirm that.
My objective is to find a way to post abort or delete those threads in a faster manner.
This may be folly on my part, but I have the nasty habit of opening, viewing, closing CodeBlocks.cbp before parsing finishes, then having to wait nearly 30 seconds for it to close. Half the time, CB shows the "... is not responding" message from Windows 7.
Thanks for the explanation.
I also see that just after setting m_abort, the parserThreads are immediately deleted by
cbThreadPool::AbortAllTasks()
std::for_each(m_tasksQueue.begin(), m_tasksQueue.end(), std::mem_fun_ref(&cbThreadedTaskElement::Delete));
so it doesn't even matter if each ParserThread is posted to abort or not.
Although they are in fact posted by cbThreadPool at
void cbThreadPool::cbWorkerThread::AbortTask()
{
wxMutexLocker lock(m_taskMutex);
if (m_pTask)
m_pTask->Abort();
}
I'm not sure, but looked at your description, I think you are not fully understand the logic here, let me explain.
The code:
void cbThreadPool::AbortAllTasks()
{
wxMutexLocker lock(m_Mutex);
std::for_each(m_threads.begin(), m_threads.end(), std::mem_fun(&cbWorkerThread::AbortTask));
std::for_each(m_tasksQueue.begin(), m_tasksQueue.end(), std::mem_fun_ref(&cbThreadedTaskElement::Delete));
m_tasksQueue.clear();
}
2, when you call cbWorkerThread::AbortTask(), it just mark the running task's flag to true:
inline void cbThreadedTask::Abort()
{
m_abort = true;
}
3, now, if you have a project, which contains 3 cpp files.
a.cpp -> x.h -> y.h -> z.h ->...
b.cpp -> x.h -> y.h -> z.h ->...
c.cpp -> x.h -> y.h -> z.h ->...
When the parser start to work, it create three tasks(that's one ParserThread object per a cpp file) and put them in the thread pool. Our thread pool currently only allow one running thread, because we can avoid multiply threads to access the symbol table(TokenTree).
Now, we are running the ParserThread associated with a.cpp, and if will create a new ParserThread x.h when it see a #include "x.h" directive. But please not that at this time, the ParserThread associated with x.h is not put into the thread pool, it just runs inside the top level(the a.cpp's) ParserThread's DoParse() function, it may recursively create another Parserthread object if it see a "#include y.h".
When you try to abort the parsing, you set the flag m_abort on the a.cpp's ParserThread, this is the running task, but the bad thing is, you are actually running the embedding y.h's ParserThread, so this task won't finished immediately, until all the header files are parsed, and the control flow goes back to the top level a.cpp's ParserThread, so it will take a lot of time!
The two ParserThread objects associated with b.cpp and c.cpp are the tasked put in the m_tasksQueue, so they will be deleted without any delay.
But traping on the ParserThreads call to cbThreadedTask::TestDestroy() always shows that m_abort is always false, never true. (except for the main Parser thread you mentioned above.
inline bool cbThreadedTask::TestDestroy() const
{
return m_abort;
}
So I'm guessing, then, that the 20 seconds (on my laptop) to close CodeBlocks.cbp (windows 7) is caused by the delete of each ParserThread. About 432 of them (give or take a few when I hit close project).
I'll do some more tracing to confirm that.
My objective is to find a way to post abort or delete those threads in a faster manner.
This may be folly on my part, but I have the nasty habit of opening, viewing, closing CodeBlocks.cbp before parsing finishes, then having to wait nearly 30 seconds for it to close. Half the time, CB shows the "... is not responding" message from Windows 7.
Thanks for the explanation.
For my example I exaplaned above, you 20 second are just waiting for the ParserThread associated with a.cpp to finish. Currently we need to find a way to exit the embedding(inner) Parserthread quickly.
ollydbg,
I'd like to propose a patch to parser.h/cpp that records the internal threads (just like cbthreadpool) and posts Abort() to the internal threads when ~parser is invoked.
Initial testing with the patch shows that closing a large project (like CodeBlocks.cbp) terminates very fast (without the long delay previously experienced).
If you agree/modify the patch, we can later clean it up (removing //(pecan 2015/11/13) tags which I use to find mods fast) and apply.
I also include one line patches to the logger code to avoid crashes I've experienced a few times when closing CB while CC was busy.
svn build rev 10554 (2015-11-01 09:18:34) gcc 4.9.2 Windows/unicode - 32 bit
Index: src/plugins/codecompletion/parser/cclogger.cpp
===================================================================
--- src/plugins/codecompletion/parser/cclogger.cpp (revision 10554)
+++ src/plugins/codecompletion/parser/cclogger.cpp (working copy)
@@ -87,6 +87,10 @@
void CCLogger::Log(const wxString& msg)
{
+ //(pecan 2015/11/12) could crash here; should check if shutting down
+ if (Manager::IsAppShuttingDown())
+ return;
+
if (!m_Parent || m_LogId<1) return;
CodeBlocksThreadEvent evt(wxEVT_COMMAND_MENU_SELECTED, m_LogId);
@@ -100,6 +104,10 @@
void CCLogger::DebugLog(const wxString& msg)
{
+ //(pecan 2015/11/12) could crash here; should check if shutting down
+ if (Manager::IsAppShuttingDown())
+ return;
+
if (!m_Parent || m_DebugLogId<1) return;
CodeBlocksThreadEvent evt(wxEVT_COMMAND_MENU_SELECTED, m_DebugLogId);
Index: src/plugins/codecompletion/parser/parser.cpp
===================================================================
--- src/plugins/codecompletion/parser/parser.cpp (revision 10554)
+++ src/plugins/codecompletion/parser/parser.cpp (working copy)
@@ -338,7 +338,9 @@
TRACE(_T("Parser::Parse(): Parsing included header, %s"), filename.wx_str());
// run the parse recursively
+ AddParserThread(thread);
result = thread->Parse();
+ RemoveParserThread(thread);
delete thread;
return true;
}
@@ -538,6 +540,7 @@
// NOTE: This should not be locked with s_ParserMutex, otherwise we'll be stuck in an
// infinite loop below since the worker thread also enters s_ParserMutex.
// In fact cbThreadPool maintains it's own mutex, so m_Pool is probably threadsafe.
+ AbortParserThreads();
m_Pool.AbortAllTasks();
while (!m_Pool.Done())
wxMilliSleep(1);
@@ -919,3 +922,38 @@
// Page "Documentation:
// m_Options.storeDocumentation will be written by DocumentationPopup
}
+
+void Parser::AddParserThread (cbThreadedTask *task) //(pecan 2015/11/13)+
+{
+ if (!task)
+ return;
+
+ m_tasksQueue.push_back (task);
+
+}
+void Parser::RemoveParserThread (cbThreadedTask* task) //(pecan 2015/11/13)+
+{
+ if (!task)
+ return;
+
+ for (TasksQueue::iterator it = m_tasksQueue.begin(); it != m_tasksQueue.end(); ++it)
+ {
+ if ((*it) == task)
+ {
+ m_tasksQueue.erase(it);
+ break;
+ }
+
+ }
+}
+void Parser::AbortParserThreads() //(pecan 2015/11/13)+
+{
+ if (not m_tasksQueue.size())
+ return;
+
+ for (TasksQueue::iterator it = m_tasksQueue.begin(); it != m_tasksQueue.end(); ++it)
+ {
+ (*it)->Abort();
+
+ }
+}
Index: src/plugins/codecompletion/parser/parser.h
===================================================================
--- src/plugins/codecompletion/parser/parser.h (revision 10554)
+++ src/plugins/codecompletion/parser/parser.h (working copy)
@@ -355,6 +355,13 @@
/** if true, all the files of the current project will be labeled as "local" */
bool m_NeedMarkFileAsLocal;
+
+ typedef std::list<cbThreadedTask*> TasksQueue; //(pecan 2015/11/13)+
+ TasksQueue m_tasksQueue; //(pecan 2015/11/13)+
+ void AddParserThread(cbThreadedTask* task); //(pecan 2015/11/13)+
+ void RemoveParserThread(cbThreadedTask* task); //(pecan 2015/11/13)+
+ void AbortParserThreads(); //(pecan 2015/11/13)+
+
};
#endif // PARSER_H