Author Topic: Several improvements to Code Completion plugin  (Read 160749 times)

Offline Huki

  • Multiple posting newcomer
  • *
  • Posts: 95
Re: Several improvements to Code Completion plugin
« Reply #75 on: April 30, 2014, 06:14:04 pm »
Hi again.. I've made some corrections to my last patch, so I'm re-posting it.

In case a token is prefixed with the ~ operator, and the user performs "go to decl / impl" on it, we just assume it's a destructor and there is no fallback if there is no such destructor. So the operation would fail in case of normal variables. eg:
Code
int token, result;

[...]

result = ~token;
          ^^^^^------------ right-click on 'token' and use go to decl / impl

My patch makes the following changes:
- Don't display the tilde in the right-click context menu, eg., Find declaration of: 'token' rather than Find declaration of: '~token'. (i.e., don't prepend ~ to "NameUnderCursor" string)
- Also don't display the tilde in "symbol not found" messages after go to decl / impl (i.e., don't prepend ~ to "target" string).
- If the tilde prefix is there, first look for a class destructor as usual but if one is not found, fallback to accept any variable.

Code
Index: src/plugins/codecompletion/codecompletion.cpp
===================================================================
--- src/plugins/codecompletion/codecompletion.cpp (revision 9271)
+++ src/plugins/codecompletion/codecompletion.cpp (working copy)
@@ -249,8 +249,9 @@
                 if (!word.IsEmpty())
                 {
                     NameUnderCursor.Clear();
-                    if (GetLastNonWhitespaceChar(control, start) == _T('~'))
-                        NameUnderCursor << _T('~');
+                    //FIX(huki), don't prepend '~' to NameUnderCursor (only used for displaying in right-click popup menu)
+                    //if (GetLastNonWhitespaceChar(control, start) == _T('~'))
+                    //    NameUnderCursor << _T('~');
                     NameUnderCursor << word;
                     ReturnValue = true;
                     IsInclude = false;
@@ -2182,8 +2233,9 @@
     const int startPos = editor->GetControl()->WordStartPosition(pos, true);
     const int endPos   = editor->GetControl()->WordEndPosition(pos, true);
     wxString target;
+    bool isDestructor = false;  //NOTE(huki), don't prepend '~' to target, use isDestructor flag instead
     if (CodeCompletionHelper::GetLastNonWhitespaceChar(editor->GetControl(), startPos) == _T('~'))
-        target << _T('~');
+        isDestructor = true; //target << _T('~');
     target << editor->GetControl()->GetTextRange(startPos, endPos);
     if (target.IsEmpty())
         return;
@@ -2201,7 +2253,7 @@
     CC_LOCKER_TRACK_TT_MTX_LOCK(s_TokenTreeMutex)
 
     // special handle destructor function
-    if (target[0] == _T('~'))
+    if (isDestructor)   //if (target[0] == _T('~'))
     {
         TokenIdxSet tmp = result;
         result.clear();
@@ -2211,11 +2263,15 @@
             const Token* token = tree->at(*it);
             if (token && token->m_TokenKind == tkClass)
             {
-                token = tree->at(tree->TokenExists(target, token->m_Index, tkDestructor));
+                token = tree->at(tree->TokenExists(_T("~") + target, token->m_Index, tkDestructor));
                 if (token)
                     result.insert(token->m_Index);
             }
         }
+
+        // no destructor found, but could be a variable.
+        if (result.empty())
+            result = tmp;
     }
     // special handle constructor function
     else

Let me know if it's ok for commit.

Offline Huki

  • Multiple posting newcomer
  • *
  • Posts: 95
Re: Several improvements to Code Completion plugin
« Reply #76 on: April 30, 2014, 06:27:30 pm »
@ollydbg: Also, I think you can go ahead and commit your patch for constructors "go to decl".
From my review and testing it does the following:
- Better handling of "go to decl"  (and also "go to impl") with class constructors.
- In case of function-like usage of a class token (eg, AAA() where AAA is a valid class), go to constructor decl / impl. If none is found, go to the class decl as fallback.
- In case of normal usage of class token, go to class decl as usual.

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5910
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: Several improvements to Code Completion plugin
« Reply #77 on: May 01, 2014, 12:35:32 am »
@ollydbg: Also, I think you can go ahead and commit your patch for constructors "go to decl".
From my review and testing it does the following:
- Better handling of "go to decl"  (and also "go to impl") with class constructors.
- In case of function-like usage of a class token (eg, AAA() where AAA is a valid class), go to constructor decl / impl. If none is found, go to the class decl as fallback.
- In case of normal usage of class token, go to class decl as usual.

Hi, Huki, thanks, the mentioned patch is already in trunk, see: Re: Find Declaration of constructor doesn't work

Hi again.. I've made some corrections to my last patch, so I'm re-posting it.

In case a token is prefixed with the ~ operator, and the user performs "go to decl / impl" on it, we just assume it's a destructor and there is no fallback if there is no such destructor. So the operation would fail in case of normal variables. eg:
Code
int token, result;

[...]

result = ~token;
          ^^^^^------------ right-click on 'token' and use go to decl / impl

My patch makes the following changes:
- Don't display the tilde in the right-click context menu, eg., Find declaration of: 'token' rather than Find declaration of: '~token'. (i.e., don't prepend ~ to "NameUnderCursor" string)
- Also don't display the tilde in "symbol not found" messages after go to decl / impl (i.e., don't prepend ~ to "target" string).
- If the tilde prefix is there, first look for a class destructor as usual but if one is not found, fallback to accept any variable.
...
...
...
Let me know if it's ok for commit.

I will check your patches in the following days. Thanks.

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 Huki

  • Multiple posting newcomer
  • *
  • Posts: 95
Re: Several improvements to Code Completion plugin
« Reply #78 on: June 16, 2014, 12:56:18 am »
Thanks for commiting the tilde patch.

A small bugfix in nativeparser.cpp, NativeParser::ParseLocalBlock():
I think it's only supposed to be run for function blocks, but it's actually run for any kind of code block (classes, etc). So for example if the user clicks on a class declaration, the entire class block will be parsed as if it's a local block and several token info (such as the line index) will be overwritten. To fix it I've added a check, here's the patch:

Code
Index: src/plugins/codecompletion/nativeparser.cpp
===================================================================
--- src/plugins/codecompletion/nativeparser.cpp (revision 9271)
+++ src/plugins/codecompletion/nativeparser.cpp (working copy)
@@ -1877,7 +1892,8 @@
 
         CC_LOCKER_TRACK_TT_MTX_UNLOCK(s_TokenTreeMutex)
 
-        if (!parent)
+        //FIX(huki), not for tkClass, etc.
+        if (!parent || !(parent->m_TokenKind & tkAnyFunction))
             return false;
     }
 
« Last Edit: June 16, 2014, 12:59:04 am by Huki »

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5910
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: Several improvements to Code Completion plugin
« Reply #79 on: June 16, 2014, 06:52:50 am »
Thanks for commiting the tilde patch.

A small bugfix in nativeparser.cpp, NativeParser::ParseLocalBlock():
I think it's only supposed to be run for function blocks, but it's actually run for any kind of code block (classes, etc). So for example if the user clicks on a class declaration, the entire class block will be parsed as if it's a local block and several token info (such as the line index) will be overwritten. To fix it I've added a check, here's the patch:

Code
Index: src/plugins/codecompletion/nativeparser.cpp
===================================================================
--- src/plugins/codecompletion/nativeparser.cpp (revision 9271)
+++ src/plugins/codecompletion/nativeparser.cpp (working copy)
@@ -1877,7 +1892,8 @@
 
         CC_LOCKER_TRACK_TT_MTX_UNLOCK(s_TokenTreeMutex)
 
-        if (!parent)
+        //FIX(huki), not for tkClass, etc.
+        if (!parent || !(parent->m_TokenKind & tkAnyFunction))
             return false;
     }
 

Hi, Huki, thanks, tested and applied in trunk now (revision 9802).
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 Huki

  • Multiple posting newcomer
  • *
  • Posts: 95
Re: Several improvements to Code Completion plugin
« Reply #80 on: June 16, 2014, 06:07:37 pm »
A bugfix for crash when trying to cancel the ongoing project parsing (to reparse, quit CB, etc). Easiest way to reproduce is to select "Project -> Reparse current project" twice successively (or once before the initial parsing has finished). It's in fact a mutex deadlock, so here is the patch:
Code
Index: src/plugins/codecompletion/parser/parser.cpp
===================================================================
--- src/plugins/codecompletion/parser/parser.cpp (revision 9271)
+++ src/plugins/codecompletion/parser/parser.cpp (working copy)
@@ -385,11 +385,14 @@
 
 Parser::~Parser()
 {
-    CC_LOCKER_TRACK_P_MTX_LOCK(ParserCommon::s_ParserMutex)
+    //FIX: There was a deadlock in TerminateAllThreads() when calling DeleteParser() before
+    // parsing has finished. Moved s_ParserMutex lock below and updated TerminateAllThreads().
 
     DisconnectEvents();
     TerminateAllThreads();
 
+    CC_LOCKER_TRACK_P_MTX_LOCK(ParserCommon::s_ParserMutex)
+
     if (ParserCommon::s_CurrentParser == this)
         ParserCommon::s_CurrentParser = nullptr;
 
@@ -851,6 +867,8 @@
 
 void Parser::TerminateAllThreads()
 {
+    CC_LOCKER_TRACK_P_MTX_LOCK(ParserCommon::s_ParserMutex)
+
     while (!m_PoolTask.empty())
     {
         PTVector& v = m_PoolTask.front();
@@ -859,6 +877,11 @@
         m_PoolTask.pop();
     }
 
+    CC_LOCKER_TRACK_P_MTX_UNLOCK(ParserCommon::s_ParserMutex)
+
+    //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.
     m_Pool.AbortAllTasks();
     while (!m_Pool.Done())
         wxMilliSleep(1);

And another patch to speed up canceling the ongoing parser:
Code
Index: src/plugins/codecompletion/nativeparser.cpp
===================================================================
--- src/plugins/codecompletion/nativeparser.cpp (revision 9271)
+++ src/plugins/codecompletion/nativeparser.cpp (working copy)
@@ -616,14 +616,20 @@
 
     if (m_ParsedProjects.empty())
     {
-        if (it->second == m_Parser)
-          SetParser(m_TempParser); // Also updates class browser
-
         wxString log(F(_("NativeParser::DeleteParser(): Deleting parser for project '%s'!"), prj.wx_str()));
         CCLogger::Get()->Log(log);
         CCLogger::Get()->DebugLog(log);
 
         delete it->second;
+
+        //NOTE: Moved here as RemoveLastFunctionChildren() in SetParser() takes quite a while if
+        // parsing hasn't finished yet. Just delete the entire parser (above), then change to m_TempParser.
+        if (it->second == m_Parser)
+        {
+            m_Parser = nullptr;
+            SetParser(m_TempParser); // Also updates class browser
+        }
+
         m_ParserList.erase(it);
 
         return true;
@@ -1253,7 +1275,9 @@
     if (m_Parser == parser)
         return;
 
-    RemoveLastFunctionChildren(m_Parser->GetTokenTree(), m_LastFuncTokenIdx);
+    if (m_Parser)   //ADD, in case parser deleted before calling SetParser().
+        RemoveLastFunctionChildren(m_Parser->GetTokenTree(), m_LastFuncTokenIdx);
+
     InitCCSearchVariables();
     m_Parser = parser;
 
« Last Edit: June 16, 2014, 06:24:30 pm by Huki »

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5910
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: Several improvements to Code Completion plugin
« Reply #81 on: June 17, 2014, 08:34:23 am »
Hi, Huki, thanks, would you mind to update your local copy of C::B to the latest SVN? I have some tiny issue to apply your patch, though I can manually applied it, but it looks like some of the code I have already changed.

In the latest revision (rev9807), I have already add a comments here in the function.
Code
void Parser::TerminateAllThreads()
{
    // FIXME (ollydbg#1#): Do we need to use a mutex to protect the m_PoolTask accessing?
    while (!m_PoolTask.empty())
    {
        PTVector& v = m_PoolTask.front();
        for (PTVector::iterator it = v.begin(); it != v.end(); ++it)
            delete *it;
        m_PoolTask.pop();
    }
Look at the FIXME.

BTW, in one testing branch of my local copy, I have try to remove all the lockers in CC (locker of the Parser and locker of the TokenTree), also, I remove the m_PoolTask, it was introduced to handle the priority header files, but now, we can already recursive to parse the included files, so I don't think m_PoolTask is needed any longer. Also, my local branch has many other issues I need to fix, so it will take a long time before I publish the patches  ;)


EDIT:
A bugfix for crash when trying to cancel the ongoing project parsing (to reparse, quit CB, etc). Easiest way to reproduce is to select "Project -> Reparse current project" twice successively (or once before the initial parsing has finished). It's in fact a mutex deadlock, so here is the patch:
...
I try to reproduce this bug, but sorry I can't reproduce the dead lock in rev9807, under Window XP. I try the two method:
method one:
Code
1, load a project, so the parsing started.
2, right click on the project, and select Peparse current project
3, no crash happened

method two:
Code
1, right click on the project, and select Peparse current project
2, right click on the project, and select Peparse current project again
3, no crash happened

Can you give some more detailed explanation? Thanks.
« Last Edit: June 17, 2014, 09:09:00 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 White-Tiger

  • Multiple posting newcomer
  • *
  • Posts: 83
Re: Several improvements to Code Completion plugin
« Reply #82 on: June 17, 2014, 10:26:41 am »
I guess he fixed what I were talking about back here: http://forums.codeblocks.org/index.php/topic,18044.msg124560.html#msg124560
You never fixed it IIRC... probably because you couldn't reproduce it.. (I couldn't get a proper stack trace)

But it's been a while since I had a freeze like that... I'm carefully with closing projects :P (and I've got a new PC now... so it's even more unlikely)
Windoze 8.1 x86_64 16GiB RAM, wxWidgets-2.8x (latest,trunk), MinGW-builds (latest, posix-threads)
Code::Blocks (x86 , latest , selection length patch , build option fixes/additions , toggle comments)

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5910
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: Several improvements to Code Completion plugin
« Reply #83 on: June 17, 2014, 10:48:11 am »
I guess he fixed what I were talking about back here: http://forums.codeblocks.org/index.php/topic,18044.msg124560.html#msg124560
You never fixed it IIRC... probably because you couldn't reproduce it.. (I couldn't get a proper stack trace)

But it's been a while since I had a freeze like that... I'm carefully with closing projects :P (and I've got a new PC now... so it's even more unlikely)
Thanks for the remind, yes, CC sometimes just hangs on my system too (I use a notepad which bought in 2009, CC hangs when loading a large project, it always hang at the first time I load the project, not the second time), It looks like other devs don't have such hang issue.  :(
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 ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5910
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: Several improvements to Code Completion plugin
« Reply #84 on: June 17, 2014, 11:27:27 am »
The parser handling is a mass, see a belief draft diagram
Code
CodeCompletion receive Workspace changed (mostly because project loaded finishes)
->NativeParser::CreateParser for the active project
  ->new Parser
  ->DoFullParsing
    ->Fill Parser's macro definition(from compiler and from project setting)
    ->Fill Parser's file list need to parse, kick the batch timer
   
Parser receive batch timer event
->new ParserThreadedTask (this task will executed in thread pool)
->send Parse Start Event!

ParserThreadedTask is executed
->parse the macro definition
->new ParserThread for each file in file list
->put ParserThread in thread queue

Thread pool finish running tasks:
->do one of below
  *CASE1: if thread queue is not empty, copy to thread pool, run
  *CASE2: if file list is not empty, kick the batch timer
  *CASE3: if macro definition is not empty, kick the batch timer
  *CASE4: non of the above cases, send Parse Finish Event!

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 White-Tiger

  • Multiple posting newcomer
  • *
  • Posts: 83
Re: Several improvements to Code Completion plugin
« Reply #85 on: June 17, 2014, 05:11:03 pm »
non the less.. I couldn't reproduce it with my current system :P
All I've got when I close a project while the parser is running, or when forcing a reparse, is just a small hang... so about 3 seconds or more.. (tested with Code::Blocks project itself)

I've also tried it with CB only 2 cores assigned (that's what my old system was, Dualcore) So maybe you've fixed the problems I had... maybe my new system makes the difference, idk.
Windoze 8.1 x86_64 16GiB RAM, wxWidgets-2.8x (latest,trunk), MinGW-builds (latest, posix-threads)
Code::Blocks (x86 , latest , selection length patch , build option fixes/additions , toggle comments)

Offline Huki

  • Multiple posting newcomer
  • *
  • Posts: 95
Re: Several improvements to Code Completion plugin
« Reply #86 on: June 17, 2014, 07:01:57 pm »
Hi, Huki, thanks, would you mind to update your local copy of C::B to the latest SVN? I have some tiny issue to apply your patch, though I can manually applied it, but it looks like some of the code I have already changed.
Oh, sorry about that. I'm in fact planning on moving some of my projects to git very soon and if everything goes well I'll be updating to obf's repo here https://github.com/obfuscated/codeblocks_sf. In the meantime I'm trying to get some of the small patches in (just a few more actually).

BTW, in one testing branch of my local copy, I have try to remove all the lockers in CC (locker of the Parser and locker of the TokenTree), also, I remove the m_PoolTask, it was introduced to handle the priority header files, but now, we can already recursive to parse the included files, so I don't think m_PoolTask is needed any longer. Also, my local branch has many other issues I need to fix, so it will take a long time before I publish the patches
The CC code indeed needs some cleanup since we don't use the priority headers. It's good if the m_PoolTask can be removed, and while it's here I think it does needs to be protected since the last time I checked it was accessed from different threads..

Btw, though I'm using an old revision I do check the latest revisions of files which I've modified, to make sure my fix is still relevant. For this one (i.e., the mutex deadlock) I noticed no changes so I was quite sure the bug still exists in the newer revisions. It's very strange that you and White-Tiger are unable to reproduce it. Let me explain:

When calling DeleteParser() we lock the parser mutex before calling TerminateAllThreads():
Code
Parser::~Parser()
{
    CC_LOCKER_TRACK_P_MTX_LOCK(ParserCommon::s_ParserMutex)

    DisconnectEvents();
    TerminateAllThreads();

    [...]

    CC_LOCKER_TRACK_P_MTX_UNLOCK(ParserCommon::s_ParserMutex)
}

In TerminateAllThreads(), we send the request to abort all threads, and wait in a while loop till all threads have aborted:
Code
    m_Pool.AbortAllTasks();
    while (!m_Pool.Done())
        wxMilliSleep(1);

At this time if one of the worker threads has already been executed, you can see in the Execute call that it would enter and leave the parser mutex many times during the course of the call:
Code: parserthreadtask.cpp, parserthreadedtask::execute()
int ParserThreadedTask::Execute()
{
    TRACE(_T("ParserThreadedTask::Execute(): Enter"));
    if (!m_Parser) return 0;

    CC_LOCKER_TRACK_P_MTX_LOCK(m_ParserMutex)

    [...]

    CC_LOCKER_TRACK_P_MTX_UNLOCK(m_ParserMutex);

    [...]

    CC_LOCKER_TRACK_P_MTX_LOCK(m_ParserMutex)

    [...]


And if Execute() is waiting to enter this mutex while we are doing the while loop in TerminateAllThreads(), we would never return. FYI, I had confirmed this condition when checking this bug under debugger.

Now, the m_Pool object happens to keep it's own mutex to protect calls to it from multiple threads, so there doesn't seem to be any reason to use the parser mutex around the m_Pool object. So in my patch I had removed the lock around the while loop, and also around the DisconnectEvents() call in ~Parser().
« Last Edit: June 17, 2014, 07:08:53 pm by Huki »

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5910
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: Several improvements to Code Completion plugin
« Reply #87 on: June 18, 2014, 04:32:59 am »
Hi, Huki, thanks for the detailed explanation, I debugged this issue, the interesting thing is that the ParserthreadedTask runs very fast after creating the parser, so when I close the parser, I always see ParserthreadedTask already finishes its job.  :)

I just find a way to reproduce the dead lock, I add a long sleep. (during the long sleep, I just close the project)
Code
 src/plugins/codecompletion/parser/parserthreadedtask.cpp | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/plugins/codecompletion/parser/parserthreadedtask.cpp b/src/plugins/codecompletion/parser/parserthreadedtask.cpp
index d3c98b6..ac11852 100644
--- a/src/plugins/codecompletion/parser/parserthreadedtask.cpp
+++ b/src/plugins/codecompletion/parser/parserthreadedtask.cpp
@@ -66,6 +66,8 @@ int ParserThreadedTask::Execute()
     TRACE(_T("ParserThreadedTask::Execute(): Enter"));
     if (!m_Parser) return 0;
 
+    wxSleep(15);
+
     CC_LOCKER_TRACK_P_MTX_LOCK(m_ParserMutex)
 
     wxString   preDefs(m_Parser->m_PredefinedMacros);
Now, I see the dead lock. I will commit your patch later today. Thanks.
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 ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5910
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: Several improvements to Code Completion plugin
« Reply #88 on: June 18, 2014, 04:35:39 am »
non the less.. I couldn't reproduce it with my current system :P
All I've got when I close a project while the parser is running, or when forcing a reparse, is just a small hang... so about 3 seconds or more.. (tested with Code::Blocks project itself)

I've also tried it with CB only 2 cores assigned (that's what my old system was, Dualcore) So maybe you've fixed the problems I had... maybe my new system makes the difference, idk.
The issue reported by Huki is located, see my previous post, so I will commit his patch.

Quote
is just a small hang... so about 3 seconds or more
I also noticed this 3 seconds hang, not sure what cause this hang.
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 ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5910
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: Several improvements to Code Completion plugin
« Reply #89 on: June 18, 2014, 07:35:19 am »
@Huki, the two patches in Re: Several improvements to Code Completion plugin are in trunk now (rev9809 and rev9810), thanks for contribution.
BTW: I add some comments.  ;)
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.