Author Topic: codecompletion plugin lead codeblocks crash, do GUI operation in worker thread  (Read 12864 times)

Offline kipade

  • Multiple posting newcomer
  • *
  • Posts: 32
I am using codeblocks under linux, I have tested under several distributions, but got the same result.
CodeBlocks always crash during working, due to codecompletion plugin.
I have had a look on its source code, I want to know why it must do GUI operation in non-main thread, but in worker thread, which cause crashing.

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5109
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
I am using codeblocks under linux, I have tested under several distributions, but got the same result.
CodeBlocks always crash during working, due to codecompletion plugin.
Me too, but it is really hard to catch the bug, I even don't have ways to reproduce some hang bug. Sometimes, it works, sometimes, it hangs.

Quote
I have had a look on its source code, I want to know why it must do GUI operation in non-main thread, but in worker thread, which cause crashing.
In CC, the symbol browser trees (wxTreeCtrl) were built from a thread class named ClassBrowserBuilderThread. The reason I think is that the tree can have many labels(Tokens), and it take several seconds to build the tree, so the devs put those operations in a worker thread. This was done years before I becomes a C::B developer.

Do you think there are some better solution?

BTW: I'm not sure which code cause the crash, maybe it is not the GUI operation in non-main thread. CC code contains a lot of mutex lockers, I hate them, but I also don't know how to fix them. So, comments/solutions/patches are all welcome.

If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

Offline kipade

  • Multiple posting newcomer
  • *
  • Posts: 32
hello ollydbg, thanks for your reply. As you said, it crashes mostly at BuildTree here, which do some UI operations.
And, I have no further ideas about that before I have a deep look on CB's source code. But, its a common method to do UI operations in worker thread is via notifications, right?
Mightbe, the operations and data should be separated, for this condition.
« Last Edit: February 10, 2014, 09:10:45 am by kipade »

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5109
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
...But, its a common method to do UI operations in worker thread is via notifications, right?
Basically, all the UI operations should be put in the main thread, note the bottle neck is building the big wxTreeCtrl, so notification method is not involved here.

Quote
Mightbe, the operations and data should be separated, for this condition.
Yes, they should be separated, but if you want to construct a big wxTreeCtrl, what is a good method?
If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

Offline kipade

  • Multiple posting newcomer
  • *
  • Posts: 32
Mightbe a big TreeCtrl was not necessary, a ugly way is to define a custom event to do such UI operations, just use custom data and callback method, I do not think its impossiable.

Offline kipade

  • Multiple posting newcomer
  • *
  • Posts: 32
Hello ollydbg, I just define a simple event to notify ClassBrowser to call ClassBrowserBuilderThread's BuildTree function, because BuildTree was originally called in a while loop, also has semaphore to guarantee ths order of the operations. So, it works for me, under linux. Although this is not a good idea, it really works. And, this also has not make the GUI busy, almost fell nothing of it.

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5109
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Hello ollydbg, I just define a simple event to notify ClassBrowser to call ClassBrowserBuilderThread's BuildTree function, because BuildTree was originally called in a while loop, also has semaphore to guarantee ths order of the operations. So, it works for me, under linux. Although this is not a good idea, it really works. And, this also has not make the GUI busy, almost fell nothing of it.
Thanks, can you show some patches? So that I can tested on my local PC(winXP).
If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 11200
    • Travis build status
Probably the best way is to define a custom event where you can store 50 or 100 items. Then in the worker thread you just send these elements and in the main thread you add the items to the tree in small batches. I think this won't block the ui and user events might be process normally.
(most of the time I ignore long posts)
[strangers don't send me private messages, I'll ignore them; post a topic in the forum, but first read the rules!]

Offline kipade

  • Multiple posting newcomer
  • *
  • Posts: 32
Probably the best way is to define a custom event where you can store 50 or 100 items. Then in the worker thread you just send these elements and in the main thread you add the items to the tree in small batches. I think this won't block the ui and user events might be process normally.
Mightbe. But, it need more coding work, right? I just define a custom event and simply move the BuildTree into ClassBrowser window messages processing chain. I also think this operation does need to be processed in this window class, since, all operations of this function is work for ClassBrowser. All of this GUI operations in this function also would not take too much time. If move the responsibility to ClassBrowser, the function should be rewrite, and, some class definition should be redesined,too. I think.

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 11200
    • Travis build status
kipade: Have you do some work on this or this is just a suggestion?
(most of the time I ignore long posts)
[strangers don't send me private messages, I'll ignore them; post a topic in the forum, but first read the rules!]

Offline kipade

  • Multiple posting newcomer
  • *
  • Posts: 32
Hello ollydbg, I just define a simple event to notify ClassBrowser to call ClassBrowserBuilderThread's BuildTree function, because BuildTree was originally called in a while loop, also has semaphore to guarantee ths order of the operations. So, it works for me, under linux. Although this is not a good idea, it really works. And, this also has not make the GUI busy, almost fell nothing of it.
Thanks, can you show some patches? So that I can tested on my local PC(winXP).
I packed my modified files, I do not use patch file because Im afraid my base code was outdated.  my base code was svn9925
This code is just a try, a joke, lol.

Offline kipade

  • Multiple posting newcomer
  • *
  • Posts: 32
kipade: Have you do some work on this or this is just a suggestion?
This just a try, of course I want the code looks perfect and works well.

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5109
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Thanks kipade, this is the patch against git-svn head, but I have build errors.
Code: [Select]
196d6a6c37e1ff7f91bd76cbafd62d645c1f2e03
 src/plugins/codecompletion/BuildTreeEvent.h        | 37 ++++++++++++++++++++++
 src/plugins/codecompletion/classbrowser.cpp        | 11 +++++++
 src/plugins/codecompletion/classbrowser.h          |  3 ++
 .../codecompletion/classbrowserbuilderthread.cpp   | 11 +++++--
 4 files changed, 60 insertions(+), 2 deletions(-)

diff --git a/src/plugins/codecompletion/BuildTreeEvent.h b/src/plugins/codecompletion/BuildTreeEvent.h
new file mode 100644
index 0000000..73cf2c5
--- /dev/null
+++ b/src/plugins/codecompletion/BuildTreeEvent.h
@@ -0,0 +1,37 @@
+#ifndef __BUILD_TREE_H__
+#define __BUILD_TREE_H__
+
+/*
+ * Simple BuildTree event for ClassBrowser
+ *
+ * Author: ZhiYu
+ */
+
+#include <wx/event.h>
+#include "classbrowserbuilderthread.h"
+
+class cbBuildTreeEvent;
+wxDECLARE_EVENT(cbBUILD_CLASSTREE_EVENT,cbBuildTreeEvent);
+class cbBuildTreeEvent : public wxCommandEvent
+{
+public:
+  cbBuildTreeEvent(ClassBrowserBuilderThread* builderThreadObject,wxEventType cmdType=cbBUILD_CLASSTREE_EVENT):wxCommandEvent(cmdType),clsBuilderThread(builderThreadObject){
+  }
+
+  ClassBrowserBuilderThread* GetBuilderThreadObject() const { return clsBuilderThread; }
+  cbBuildTreeEvent* Clone()
+  {
+    return new cbBuildTreeEvent(*this);
+  }
+private:
+  ClassBrowserBuilderThread* clsBuilderThread;
+};
+
+//following lines are only for you if you did not like use event table
+typedef void (wxEvtHandler::*cbBuildTreeFunction)(cbBuildTreeEvent&);
+
+#define cbBuildTreeEventHandler(func) wxEVENT_HANDLER_CAST(cbBuildTreeFunction,func)
+#define EVT_CBBUILD_CLASSTREE(id,func) \
+  wx__DECLARE_EVT1(cbBUILD_CLASSTREE_EVENT,id,cbBuildTreeEventHandler(func))
+
+#endif
diff --git a/src/plugins/codecompletion/classbrowser.cpp b/src/plugins/codecompletion/classbrowser.cpp
index cd96467..4914d86 100644
--- a/src/plugins/codecompletion/classbrowser.cpp
+++ b/src/plugins/codecompletion/classbrowser.cpp
@@ -44,6 +44,7 @@
 #include "nativeparser.h"
 
 #include "parser/ccdebuginfo.h"
+#include <stdio.h>
 
 #define CC_CLASS_BROWSER_DEBUG_OUTPUT 0
 
@@ -76,6 +77,8 @@
     #define TRACE2(format, args...)
 #endif
 
+wxDEFINE_EVENT(cbBUILD_CLASSTREE_EVENT,cbBuildTreeEvent);
+
 int idMenuJumpToDeclaration    = wxNewId();
 int idMenuJumpToImplementation = wxNewId();
 int idMenuRefreshTree          = wxNewId();
@@ -127,6 +130,7 @@ BEGIN_EVENT_TABLE(ClassBrowser, wxPanel)
     EVT_MENU(idCBBottomTree,                             ClassBrowser::OnCBViewMode)
 
     EVT_COMMAND(idThreadEvent, wxEVT_COMMAND_ENTER,      ClassBrowser::OnThreadEvent)
+    EVT_CBBUILD_CLASSTREE(wxID_ANY,ClassBrowser::OnBuileTree)
 END_EVENT_TABLE()
 
 // class constructor
@@ -971,3 +975,10 @@ void ClassBrowser::OnThreadEvent(wxCommandEvent& event)
             break;
     }
 }
+
+void ClassBrowser::OnBuileTree(cbBuildTreeEvent& event)
+{
+  ClassBrowserBuilderThread* thread = event.GetBuilderThreadObject();
+  if(thread)
+    thread->BuildTree();
+}
diff --git a/src/plugins/codecompletion/classbrowser.h b/src/plugins/codecompletion/classbrowser.h
index 9715c07..1a272c7 100644
--- a/src/plugins/codecompletion/classbrowser.h
+++ b/src/plugins/codecompletion/classbrowser.h
@@ -16,6 +16,7 @@
 #include "classbrowserbuilderthread.h"
 #include "parser/parser.h"
 #include "parser/token.h"
+#include "BuildTreeEvent.h"
 
 class NativeParser;
 class wxComboBox;
@@ -90,6 +91,8 @@ private:
     void OnDebugSmartSense(wxCommandEvent& event);
     void OnSetSortType(wxCommandEvent& event);
 
+    void OnBuileTree(cbBuildTreeEvent& event);
+
     void OnSearch(wxCommandEvent& event);
     bool FoundMatch(const wxString& search, wxTreeCtrl* tree, const wxTreeItemId& item);
     wxTreeItemId FindNext(const wxString& search, wxTreeCtrl* tree, const wxTreeItemId& start);
diff --git a/src/plugins/codecompletion/classbrowserbuilderthread.cpp b/src/plugins/codecompletion/classbrowserbuilderthread.cpp
index 24f05cb..6640ef0 100644
--- a/src/plugins/codecompletion/classbrowserbuilderthread.cpp
+++ b/src/plugins/codecompletion/classbrowserbuilderthread.cpp
@@ -63,6 +63,8 @@
     #define TRACE2(format, args...)
 #endif
 
+#include "BuildTreeEvent.h"
+
 ClassBrowserBuilderThread::ClassBrowserBuilderThread(wxEvtHandler* evtHandler, wxSemaphore& sem) :
     wxThread(wxTHREAD_JOINABLE),
     m_Parent(evtHandler),
@@ -184,7 +186,7 @@ void ClassBrowserBuilderThread::Init(NativeParser*         np,
 }
 
 // Thread function
-
+#include <stdio.h>
 void* ClassBrowserBuilderThread::Entry()
 {
     while (!m_TerminationRequested && !Manager::IsAppShuttingDown() )
@@ -206,7 +208,10 @@ void* ClassBrowserBuilderThread::Entry()
                 ::wxMutexGuiEnter();
         }
 
-        BuildTree();
+        //BuildTree();
+        cbBuildTreeEvent* evt = new cbBuildTreeEvent(this,cbBUILD_CLASSTREE_EVENT);
+        if(m_Parent)
+          m_Parent->QueueEvent(evt);
 
         if (platform::gtk || platform::macosx)
         {
@@ -447,6 +452,8 @@ void ClassBrowserBuilderThread::BuildTree()
     // 6.) Hide&Freeze trees shown
     if (m_BrowserOptions.treeMembers)
     {
+        if(m_CCTreeCtrlBottom==NULL)
+          wxMessageBox(wxT("kcynice:classbrowserbuilderthread.cpp::BuildTree m_CCTreeCtrlBottom is null"));
         m_CCTreeCtrlBottom->Hide();
         m_CCTreeCtrlBottom->Freeze();
     }


Build error wx2.8.12, winXP.
Code: [Select]
[ 20.0%] g++.exe -Wall -g -pipe -fmessage-length=0 -fexceptions -mthreads -Winvalid-pch -DHAVE_W32API_H -D__WXMSW__ -DWXUSINGDLL -DcbDEBUG -DCB_PRECOMP -DWX_PRECOMP -DwxUSE_UNICODE -DEXPORT_LIB=0 -DEXPORT_EVENTS=0 -DwxUSE_WX_RESOURCES=0 -DBUILDING_PLUGIN -IE:\code\wx-mingw-build-481-dw2\wxWidgets-2.8.12\include -IE:\code\wx-mingw-build-481-dw2\wxWidgets-2.8.12\lib\gcc_dll\mswu -I..\..\include -I..\..\sdk\wxscintilla\include -I..\..\include\mozilla_chardet -I..\..\include\mozilla_chardet\mfbt -I..\..\include\mozilla_chardet\nsprpub\pr\include -I..\..\include\mozilla_chardet\xpcom -I..\..\include\mozilla_chardet\xpcom\base -I..\..\include\mozilla_chardet\xpcom\glue -c classbrowser.cpp -o ..\..\.objs\plugins\codecompletion\classbrowser.o
In file included from classbrowser.h:19:0,
                 from classbrowser.cpp:43:
BuildTreeEvent.h:14:16: error: expected constructor, destructor, or type conversion before '(' token
BuildTreeEvent.h:18:87: error: 'cbBUILD_CLASSTREE_EVENT' was not declared in this scope
classbrowser.cpp:80:15: error: expected constructor, destructor, or type conversion before '(' token
classbrowser.cpp:133:5: error: 'cbBUILD_CLASSTREE_EVENT' was not declared in this scope
In file included from classbrowser.h:16:0,
                 from classbrowser.cpp:43:
classbrowserbuilderthread.h: In member function 'void ClassBrowser::OnBuileTree(cbBuildTreeEvent&)':
classbrowserbuilderthread.h:64:10: error: 'void ClassBrowserBuilderThread::BuildTree()' is protected
classbrowser.cpp:983:23: error: within this context
Process terminated with status 1 (0 minute(s), 13 second(s))
6 error(s), 0 warning(s) (0 minute(s), 13 second(s))

:)
If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

Offline kipade

  • Multiple posting newcomer
  • *
  • Posts: 32
Thanks kipade, this is the patch against git-svn head, but I have build errors.
I am sorry that, I forgot upload one file, but with only one line added, its in classbrowserbuilderthread.h, add ClassBrowser as a friend class of ClassBrowserBuilderThread. btw, you can do some slight modification to make it work.  :)

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5109
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
A working patch (fix some bugs in kipade' original patch), it seems to work, I don't see time lags (browser tree even set to show everything in a sample wx3.0 prj).

Code: [Select]
src/plugins/codecompletion/BuildTreeEvent.h        | 66 ++++++++++++++++++++++
 src/plugins/codecompletion/classbrowser.cpp        | 18 ++++++
 src/plugins/codecompletion/classbrowser.h          |  3 +
 .../codecompletion/classbrowserbuilderthread.cpp   | 14 ++++-
 .../codecompletion/classbrowserbuilderthread.h     |  1 +
 5 files changed, 99 insertions(+), 3 deletions(-)

diff --git a/src/plugins/codecompletion/BuildTreeEvent.h b/src/plugins/codecompletion/BuildTreeEvent.h
new file mode 100644
index 0000000..73d0b1a
--- /dev/null
+++ b/src/plugins/codecompletion/BuildTreeEvent.h
@@ -0,0 +1,66 @@
+#ifndef __BUILD_TREE_H__
+#define __BUILD_TREE_H__
+
+/*
+ * Simple BuildTree event for ClassBrowser
+ *
+ * Author: ZhiYu
+ */
+
+#include <wx/event.h>
+#include "classbrowserbuilderthread.h"
+
+extern const wxEventType cbBUILD_CLASSTREE_EVENT;
+// the below does not work, see:
+// http://permalink.gmane.org/gmane.comp.lib.wxwindows.general/62431
+//DECLARE_EVENT_TYPE(cbBUILD_CLASSTREE_EVENT, wxID_ANY)
+
+class cbBuildTreeEvent : public wxCommandEvent
+{
+public:
+    cbBuildTreeEvent(ClassBrowserBuilderThread* builderThreadObject = NULL,
+                     wxEventType cmdType = cbBUILD_CLASSTREE_EVENT)
+        :wxCommandEvent(cmdType),
+         clsBuilderThread(builderThreadObject)
+    {
+    }
+
+    cbBuildTreeEvent(const cbBuildTreeEvent& rhs)
+        : wxCommandEvent(rhs),
+          clsBuilderThread(rhs.clsBuilderThread)
+
+    {
+    }
+
+    ClassBrowserBuilderThread* GetBuilderThreadObject() const
+    {
+        return clsBuilderThread;
+    }
+    virtual wxEvent *Clone() const
+    {
+        return new cbBuildTreeEvent(*this);
+    }
+//private:
+    ClassBrowserBuilderThread* clsBuilderThread;
+    DECLARE_DYNAMIC_CLASS( cbBuildTreeEvent )
+};
+
+//following lines are only for you if you did not like use event table
+
+typedef void (wxEvtHandler::*cbBuildTreeFunction)(cbBuildTreeEvent&);
+
+
+#define cbBuildTreeEventHandler(func)  \
+ (wxObjectEventFunction)(wxEventFunction) \
+ wxStaticCastEvent(cbBuildTreeFunction, &func)
+
+
+
+#define EVT_CBBUILD_CLASSTREE(id, fn) \
+    DECLARE_EVENT_TABLE_ENTRY(cbBUILD_CLASSTREE_EVENT, id, wxID_ANY, \
+    (wxObjectEventFunction) (wxEventFunction) (cbBuildTreeFunction) \
+    wxStaticCastEvent( cbBuildTreeFunction, & fn ), (wxObject *) NULL ),
+
+
+
+#endif
diff --git a/src/plugins/codecompletion/classbrowser.cpp b/src/plugins/codecompletion/classbrowser.cpp
index cd96467..498113d 100644
--- a/src/plugins/codecompletion/classbrowser.cpp
+++ b/src/plugins/codecompletion/classbrowser.cpp
@@ -44,6 +44,7 @@
 #include "nativeparser.h"
 
 #include "parser/ccdebuginfo.h"
+#include <stdio.h>
 
 #define CC_CLASS_BROWSER_DEBUG_OUTPUT 0
 
@@ -76,6 +77,15 @@
     #define TRACE2(format, args...)
 #endif
 
+// below is used on wx3.x only
+//wxDEFINE_EVENT(cbBUILD_CLASSTREE_EVENT,cbBuildTreeEvent);
+
+IMPLEMENT_DYNAMIC_CLASS(cbBuildTreeEvent, wxCommandEvent)
+DEFINE_EVENT_TYPE(cbBUILD_CLASSTREE_EVENT)
+
+
+
+
 int idMenuJumpToDeclaration    = wxNewId();
 int idMenuJumpToImplementation = wxNewId();
 int idMenuRefreshTree          = wxNewId();
@@ -127,6 +137,7 @@ BEGIN_EVENT_TABLE(ClassBrowser, wxPanel)
     EVT_MENU(idCBBottomTree,                             ClassBrowser::OnCBViewMode)
 
     EVT_COMMAND(idThreadEvent, wxEVT_COMMAND_ENTER,      ClassBrowser::OnThreadEvent)
+    EVT_CBBUILD_CLASSTREE(wxID_ANY,ClassBrowser::OnBuileTree)
 END_EVENT_TABLE()
 
 // class constructor
@@ -971,3 +982,10 @@ void ClassBrowser::OnThreadEvent(wxCommandEvent& event)
             break;
     }
 }
+
+void ClassBrowser::OnBuileTree(cbBuildTreeEvent& event)
+{
+  ClassBrowserBuilderThread* thread = event.GetBuilderThreadObject();
+  if(thread)
+    thread->BuildTree();
+}
diff --git a/src/plugins/codecompletion/classbrowser.h b/src/plugins/codecompletion/classbrowser.h
index 9715c07..1a272c7 100644
--- a/src/plugins/codecompletion/classbrowser.h
+++ b/src/plugins/codecompletion/classbrowser.h
@@ -16,6 +16,7 @@
 #include "classbrowserbuilderthread.h"
 #include "parser/parser.h"
 #include "parser/token.h"
+#include "BuildTreeEvent.h"
 
 class NativeParser;
 class wxComboBox;
@@ -90,6 +91,8 @@ private:
     void OnDebugSmartSense(wxCommandEvent& event);
     void OnSetSortType(wxCommandEvent& event);
 
+    void OnBuileTree(cbBuildTreeEvent& event);
+
     void OnSearch(wxCommandEvent& event);
     bool FoundMatch(const wxString& search, wxTreeCtrl* tree, const wxTreeItemId& item);
     wxTreeItemId FindNext(const wxString& search, wxTreeCtrl* tree, const wxTreeItemId& start);
diff --git a/src/plugins/codecompletion/classbrowserbuilderthread.cpp b/src/plugins/codecompletion/classbrowserbuilderthread.cpp
index 24f05cb..5b74337 100644
--- a/src/plugins/codecompletion/classbrowserbuilderthread.cpp
+++ b/src/plugins/codecompletion/classbrowserbuilderthread.cpp
@@ -26,11 +26,12 @@
     #include <projectmanager.h>
 #endif
 
+#include "BuildTreeEvent.h"
 #include "classbrowserbuilderthread.h"
 
 // sanity check for the build tree functions, this function should only be called in a worker thread
 // also, there should be no termination requested, otherwise, it will return false
-#define CBBT_SANITY_CHECK ((!::wxIsMainThread() && m_TerminationRequested) || Manager::IsAppShuttingDown())
+#define CBBT_SANITY_CHECK (( m_TerminationRequested) || Manager::IsAppShuttingDown())
 
 #define CC_BUILDERTHREAD_DEBUG_OUTPUT 0
 
@@ -63,6 +64,8 @@
     #define TRACE2(format, args...)
 #endif
 
+#include "BuildTreeEvent.h"
+
 ClassBrowserBuilderThread::ClassBrowserBuilderThread(wxEvtHandler* evtHandler, wxSemaphore& sem) :
     wxThread(wxTHREAD_JOINABLE),
     m_Parent(evtHandler),
@@ -184,7 +187,7 @@ void ClassBrowserBuilderThread::Init(NativeParser*         np,
 }
 
 // Thread function
-
+#include <stdio.h>
 void* ClassBrowserBuilderThread::Entry()
 {
     while (!m_TerminationRequested && !Manager::IsAppShuttingDown() )
@@ -206,7 +209,10 @@ void* ClassBrowserBuilderThread::Entry()
                 ::wxMutexGuiEnter();
         }
 
-        BuildTree();
+        //BuildTree();
+        cbBuildTreeEvent evt(this,cbBUILD_CLASSTREE_EVENT);
+        if(m_Parent)
+          m_Parent->AddPendingEvent(evt);
 
         if (platform::gtk || platform::macosx)
         {
@@ -447,6 +453,8 @@ void ClassBrowserBuilderThread::BuildTree()
     // 6.) Hide&Freeze trees shown
     if (m_BrowserOptions.treeMembers)
     {
+        if(m_CCTreeCtrlBottom==NULL)
+          wxMessageBox(wxT("kcynice:classbrowserbuilderthread.cpp::BuildTree m_CCTreeCtrlBottom is null"));
         m_CCTreeCtrlBottom->Hide();
         m_CCTreeCtrlBottom->Freeze();
     }
diff --git a/src/plugins/codecompletion/classbrowserbuilderthread.h b/src/plugins/codecompletion/classbrowserbuilderthread.h
index 9369a27..3acfcb8 100644
--- a/src/plugins/codecompletion/classbrowserbuilderthread.h
+++ b/src/plugins/codecompletion/classbrowserbuilderthread.h
@@ -20,6 +20,7 @@
  */
 class ClassBrowserBuilderThread : public wxThread
 {
+    friend class ClassBrowser;
 public:
     /** the builder threads' event sent to the GUI(class browser window)*/
     enum EThreadEvent

If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 11200
    • Travis build status
ollydbg: The last patch does nothing interesting. Is it the only change that has to be made to C::B to fix the problem or other patches are required?
(most of the time I ignore long posts)
[strangers don't send me private messages, I'll ignore them; post a topic in the forum, but first read the rules!]

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5109
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
ollydbg: The last patch does nothing interesting. Is it the only change that has to be made to C::B to fix the problem or other patches are required?
Yes, the patch is not mature.
It is kind of proof of concept to move those operations in GUI main thread.  ;D
If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 11200
    • Travis build status
But if I understand you correctly you're moving all operations to the UI thread, because I don't see any data stored in the event's object.
(most of the time I ignore long posts)
[strangers don't send me private messages, I'll ignore them; post a topic in the forum, but first read the rules!]

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5109
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
But if I understand you correctly you're moving all operations to the UI thread, because I don't see any data stored in the event's object.
The function "void ClassBrowserBuilderThread::BuildTree()" is not called in "void* ClassBrowserBuilderThread::Entry()", but in "void ClassBrowser::OnBuileTree(cbBuildTreeEvent& event)".

The custom event(cbBuildTreeEvent) just carry a "ClassBrowserBuilderThread* thread" from the worker thread to main GUI.
If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 11200
    • Travis build status
OK, but how do you know how many elements you have to add to the tree in a single event handler? As far as I see it you still need locking.

If I'm doing this I'd rather:
1. in the parser thread generate some data and store it in the symbolbrowser event object
2. post an event with the gathered data
3. continue with parsing
4. when I receive the event I'll add the data to the tree

This way it is explicitly stated how owns the data exclusively and there is no need for locking.
(most of the time I ignore long posts)
[strangers don't send me private messages, I'll ignore them; post a topic in the forum, but first read the rules!]

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5109
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
OK, but how do you know how many elements you have to add to the tree in a single event handler? As far as I see it you still need locking.

If I'm doing this I'd rather:
1. in the parser thread generate some data and store it in the symbolbrowser event object
2. post an event with the gathered data
3. continue with parsing
4. when I receive the event I'll add the data to the tree

This way it is explicitly stated how owns the data exclusively and there is no need for locking.

OK, I understand you idea, but we are not talking with the same lockers.
You are stating the locker for the TokenTree, but my patch try to move the build wxTreeCtrl code from worker thread to GUI thread.

Note, TokenTree locker is still there, but mostly they don't cause issue, because the wxTreeCtrl is mainly constructed after the Parser finish the job, in that time, TokenTree is always free(not owned by other one).

Back to your proposed method, to avoid the TokenTree locker when building the tree, you need to construct some data structure (some kind of a copy of a sub Tokentree?) and send to the GUI thread, but if the data structure is too large?
« Last Edit: February 17, 2014, 01:53:58 am by ollydbg »
If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

Offline MortenMacFly

  • Administrator
  • Lives here!
  • *****
  • Posts: 9473
Re:
« Reply #21 on: February 17, 2014, 06:13:19 am »
First of all the patch does not work but crashes cb at exit. Second if the data gets too large you can use a pointer. Third I don't think it's that easy.Yes, the method is called from outside but the task is still operated in the thread so it doesn't really change anything... Sorry.
Compiler logging: Settings->Compiler & Debugger->tab "Other"->Compiler logging="Full command line"
C::B Manual: http://www.codeblocks.org/docs/main_codeblocks_en.html
C::B FAQ: http://wiki.codeblocks.org/index.php?title=FAQ

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5109
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re:
« Reply #22 on: February 17, 2014, 07:10:31 am »
First of all the patch does not work but crashes cb at exit.
Hi, Morten, thanks for the test. In-fact, If we need to build the Tree from GUI, we should totally remove the worker tree builder thread stuff.

Quote
Second if the data gets too large you can use a pointer.
Yes, just as the same way as we access to Tokentree to query suggestion list information.

Quote
Third I don't think it's that easy.Yes, the method is called from outside but the task is still operated in the thread so it doesn't really change anything... Sorry.
The tree build thread is large, also, I read some comments in the code like:
Code: [Select]
// 9.) Expand item --> Bottleneck: Takes ~4 secs on C::B workspace
....
// 11.) Select the item saved before --> Bottleneck: Takes ~4 secs on C::B workspace
I won't let the GUI to hang for 8 seconds, so I won't step further in this direction for now.  :)
If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

Offline kipade

  • Multiple posting newcomer
  • *
  • Posts: 32
Hi All, thanks for all attention about this topic. I wanna say, the simple code was just a test for that, just to see if this can fixed the crash bug, a joke.
Yes, if need finally perfect fix for that issue, some more slight design for that might needed. Of course we can drive a tree control from wxTreeCtrl, it seems more familar with OO rule. But, there still have some other choices such as my moved BuidTree. I think, if all the operations could be separated with data, and GUI, one class only do which it should, all the things become easy. I think.
If it still no official support, I might continue to complete it by my hand, because I need it work under linux. Any idea is welcome.

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5109
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
If it still no official support, I might continue to complete it by my hand, because I need it work under linux. Any idea is welcome.
Do you think it is definitely the codecompletion's browser tree which cause the crash issue?
I haven't seen that on our forum about such issue, can you show us the crash backtrace?
If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 11200
    • Travis build status
Are we sure what is taking all the time when building the tree from the main thread?
Has any one run a profiler to see what is slowing things down? Most of the times it is something quite unexpected, because people are really bad at guessing bottleneck!

If someone is willing to bring back all UI building code to the main thread and provide a patch, I'll run some profiling tools on it and I'll try to improve it.
(most of the time I ignore long posts)
[strangers don't send me private messages, I'll ignore them; post a topic in the forum, but first read the rules!]

Offline MortenMacFly

  • Administrator
  • Lives here!
  • *****
  • Posts: 9473
Re:
« Reply #26 on: February 17, 2014, 10:08:51 pm »
I'm afraid there is nothing to bring back as imho it has always been like that...
Compiler logging: Settings->Compiler & Debugger->tab "Other"->Compiler logging="Full command line"
C::B Manual: http://www.codeblocks.org/docs/main_codeblocks_en.html
C::B FAQ: http://wiki.codeblocks.org/index.php?title=FAQ

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 11200
    • Travis build status
I think Loaden has moved it in a thread, but anyway, lets say to move the code to the UI thread in order to make it easier to profile.
(most of the time I ignore long posts)
[strangers don't send me private messages, I'll ignore them; post a topic in the forum, but first read the rules!]

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5109
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
I think Loaden has moved it in a thread, but anyway, ...
No, the wxTreeCtrl builder thread has a long history, it exist before Loaden became a C::B developer.
If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

Offline kipade

  • Multiple posting newcomer
  • *
  • Posts: 32
If it still no official support, I might continue to complete it by my hand, because I need it work under linux. Any idea is welcome.
Do you think it is definitely the codecompletion's browser tree which cause the crash issue?
I haven't seen that on our forum about such issue, can you show us the crash backtrace?

Im sure. I will upload a crash trace when I have some free time to have a try.

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5109
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
OK, four years later, and I still think I need symbol tree. So, this is the patch against the latest svn head.
I build it under 32bit gcc 7.2 under Windows with wx 3.1.1.

I think we can "disable" the option to show the whole project's tokens in the tree, because this should take too long. But for tokens in a single source file, it should be OK. I don't see time lags.



If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 11200
    • Travis build status
Can you explain how this patch works and what it tries to achieve?
I've tried to look at it, but I'm not familiar with the code, so I cannot really say if it is a good idea.

Generally my understanding is that without a major redesign, the Class Browser would be a constant cause of problems. As far as I can see this is not a major redesign, but a try to workaround the problem...
(most of the time I ignore long posts)
[strangers don't send me private messages, I'll ignore them; post a topic in the forum, but first read the rules!]

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5109
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Can you explain how this patch works and what it tries to achieve?
I've tried to look at it, but I'm not familiar with the code, so I cannot really say if it is a good idea.
The patch is quite simple, it just translate all the "tree building" code snippets from running in a worker thread to the main GUI thread. Those code snippets was running both from the main GUI thread and worker thread. By using this patch, a custom event is used, when we need to build the tree or operating on the tree, the event if fired, and the final operations all happens in the main GUI thread.


Quote
Generally my understanding is that without a major redesign, the Class Browser would be a constant cause of problems. As far as I can see this is not a major redesign, but a try to workaround the problem...
Yes, I agree, this patch is just a workaround.
If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 11200
    • Travis build status
Also I don't think it is a good idea to push this now. It will be good if you can wait a bit.
(most of the time I ignore long posts)
[strangers don't send me private messages, I'll ignore them; post a topic in the forum, but first read the rules!]