Author Topic: The 19 september 2010 build (6608) CODECOMPLETION BRANCH version is out.  (Read 70068 times)

Offline Loaden

  • Lives here!
  • ****
  • Posts: 1014
Re: The 19 september 2010 build (6608) CODECOMPLETION BRANCH version is out.
« Reply #60 on: September 24, 2010, 08:08:02 am »
Fix system headers complete hangs.
I see this in the patch:
+            return !!m_Locker;
Before digging deeper into it: Is that what you want or a mistake?!

It just convert to bool.
It's i want.

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 13438
    • Travis build status
Re: The 19 september 2010 build (6608) CODECOMPLETION BRANCH version is out.
« Reply #61 on: September 24, 2010, 08:12:02 am »
This is C-ish style of coding, not very appropriate for C++.
If you can use != 0, != NULL or something like that it is better.
What is the type of m_Locker?
(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 Loaden

  • Lives here!
  • ****
  • Posts: 1014
Re: The 19 september 2010 build (6608) CODECOMPLETION BRANCH version is out.
« Reply #62 on: September 24, 2010, 08:48:21 am »
This is C-ish style of coding, not very appropriate for C++.
If you can use != 0, != NULL or something like that it is better.
What is the type of m_Locker?
I just like this convertion. It's very simply. :?
Here is the completly code.
Code
class SystemHeadersThread : public wxThread
{
public:
    SystemHeadersThread(CodeCompletion* parent, SystemHeadersMap& headersMap, const wxArrayString& incDirs) :
        m_Parent(parent),
        m_SystemHeadersMap(headersMap),
        m_IncludeDirs(incDirs)
    {
        Create();
        SetPriority(WXTHREAD_MIN_PRIORITY);
    }

    virtual void* Entry()
    {
        wxArrayString dirs;
        {
            wxCriticalSectionLocker locker(s_HeadersCriticalSection);
            for (size_t i = 0; i < m_IncludeDirs.GetCount(); ++i)
            {
                if (m_SystemHeadersMap.find(m_IncludeDirs[i]) == m_SystemHeadersMap.end())
                {
                    dirs.Add(m_IncludeDirs[i]);
                    m_SystemHeadersMap[m_IncludeDirs[i]] = std::list<wxString>();
                }
            }
        }

        for (size_t i = 0; i < dirs.GetCount() && !TestDestroy(); ++i)
        {
            wxDir dir(dirs[i]);
            if (!dir.IsOpened())
                continue;

            HeaderDirTraverser traverser(m_SystemHeadersMap, dirs[i]);
            dir.Traverse(traverser, wxEmptyString, wxDIR_FILES | wxDIR_DIRS);

            wxCommandEvent evt(wxEVT_COMMAND_MENU_SELECTED, THREAD_UPDATE);
            evt.SetClientData(this);
            evt.SetString(wxString::Format(_T("Get Headers: %s , %d"), dirs[i].wx_str(),
                                           m_SystemHeadersMap[dirs[i]].size()));
            wxPostEvent(m_Parent, evt);
        }

        if (!TestDestroy())
        {
            wxCommandEvent evt(wxEVT_COMMAND_MENU_SELECTED, THREAD_COMPLETED);
            evt.SetClientData(this);
            if (!dirs.IsEmpty())
                evt.SetString(wxString::Format(_T("Get the system header file path: %d"), dirs.GetCount()));
            wxPostEvent(m_Parent, evt);
        }

        return NULL;
    }

private:
    CodeCompletion*   m_Parent;
    SystemHeadersMap& m_SystemHeadersMap;
    wxArrayString     m_IncludeDirs;

private:
    class HeaderDirTraverser : public wxDirTraverser
    {
    public:
        HeaderDirTraverser(SystemHeadersMap& headersMap, const wxString& searchDir) :
            m_SystemHeadersMap(headersMap),
            m_SearchDir(searchDir),
            m_Headers(headersMap[searchDir]),
            m_Locker(NULL),
            m_HeaderCount(0)
        {}

        ~HeaderDirTraverser()
        {
            if (m_Locker)
                delete m_Locker;
        }

        virtual wxDirTraverseResult OnFile(const wxString& filename)
        {
            if (!AddLock())
                return wxDIR_STOP;

            wxFileName fn(filename);
            if (!fn.HasExt() || fn.GetExt().GetChar(0) == _T('h'))
            {
                fn.MakeRelativeTo(m_SearchDir);
                wxString final(fn.GetFullPath());
                final.Replace(_T("\\"), _T("/"), true);
                m_Headers.push_back(final);
            }

            return wxDIR_CONTINUE;
        }

        virtual wxDirTraverseResult OnDir(const wxString& dirname)
        {
            if (!AddLock())
                return wxDIR_STOP;

            wxString path(dirname);
            if (path.Last() != wxFILE_SEP_PATH)
                path.Append(wxFILE_SEP_PATH);

            if (m_SystemHeadersMap.find(path) != m_SystemHeadersMap.end())
                return wxDIR_IGNORE;
            return wxDIR_CONTINUE;
        }

        bool AddLock()
        {
            if (++m_HeaderCount % 100 == 1)
            {
                if (m_Locker)
                {
                    delete m_Locker;
                    m_Locker = NULL;
                }

                wxMilliSleep(1);
                m_Locker = new(std::nothrow) wxCriticalSectionLocker(s_HeadersCriticalSection);
            }

            return !!m_Locker;
        }

    private:
        const SystemHeadersMap&  m_SystemHeadersMap;
        const wxString&          m_SearchDir;
        std::list<wxString>&     m_Headers;
        wxCriticalSectionLocker* m_Locker;
        size_t                   m_HeaderCount;
    };
};

Offline Borr

  • Multiple posting newcomer
  • *
  • Posts: 29

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 13438
    • Travis build status
Re: The 19 september 2010 build (6608) CODECOMPLETION BRANCH version is out.
« Reply #64 on: September 24, 2010, 09:03:29 am »
It's very simply. :?
You mean simple?

In this case != NULL is best (in fact you could use != nullptr in C::B), because you're documenting that this is a check for NULL.
"!!" is very confusing for people not seen this before...

Also why do you create the locker on the heap, probably some commenting will be good, this is way to non trivial code you've written.
« Last Edit: September 24, 2010, 09:08:16 am by oBFusCATed »
(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 Loaden

  • Lives here!
  • ****
  • Posts: 1014
Re: The 19 september 2010 build (6608) CODECOMPLETION BRANCH version is out.
« Reply #65 on: September 24, 2010, 09:20:25 am »
Also why do you create the locker on the heap, probably some commenting will be good, this is way to non trivial code you've written.
Because we need to give the main thread of a chance to enter the critical.
You can check this code:
Code
// Do the code completion when we enter:
// #include "| or #include <|
void CodeCompletion::CodeCompleteIncludes()
{
...
    // #include <|
    if (m_CCSystemHeaderFiles)
    {
        wxCriticalSectionLocker locker(s_HeadersCriticalSection);
        wxArrayString& incDirs = GetSystemIncludeDirs(&m_NativeParser.GetParser(),
                                                      project ? project->GetModified() : true);
        for (size_t i = 0; i < incDirs.GetCount(); ++i)
        {
            SystemHeadersMap::iterator it = m_SystemHeadersMap.find(incDirs[i]);
            if (it != m_SystemHeadersMap.end())
            {
                const std::list<wxString>& headers = it->second;
                for (std::list<wxString>::const_iterator it = headers.begin(); it != headers.end(); ++it)
                    files.insert(*it);
            }
        }
    }
This will make the UI more responsive.

Offline Loaden

  • Lives here!
  • ****
  • Posts: 1014
Re: The 19 september 2010 build (6608) CODECOMPLETION BRANCH version is out.
« Reply #66 on: September 24, 2010, 09:23:06 am »
In this case != NULL is best (in fact you could use != nullptr in C::B), because you're documenting that this is a check for NULL.
"!!" is very confusing for people not seen this before...
Got it! Thanks!!
Quote
Index: src/plugins/codecompletion/codecompletion.cpp
===================================================================
--- src/plugins/codecompletion/codecompletion.cpp    (revision 6632)
+++ src/plugins/codecompletion/codecompletion.cpp    (working copy)
@@ -339,7 +339,7 @@
                 m_Locker = new(std::nothrow) wxCriticalSectionLocker(s_HeadersCriticalSection);
             }
 
-            return !!m_Locker;
+            return m_Locker != NULL;
         }
 
     private:

Offline Loaden

  • Lives here!
  • ****
  • Posts: 1014

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 13438
    • Travis build status
Re: The 19 september 2010 build (6608) CODECOMPLETION BRANCH version is out.
« Reply #68 on: September 24, 2010, 09:34:17 am »
Loaden:
Why don't you use message passing for this?
What you've done is just a hack...
Also, It seems that the list of files won't be correct if the main thread enters the critical section before the worker have finished.
Probably it will be better to add an item in the list -> "Processing 10%"
(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 killerbot

  • Administrator
  • Lives here!
  • *****
  • Posts: 5297
Re: The 19 september 2010 build (6608) CODECOMPLETION BRANCH version is out.
« Reply #69 on: September 24, 2010, 09:37:25 am »
well :

Code
return m_Locker != 0;

NULL :  :-(    , please C++0X come fast (or at least gcc implement it fast) nullptr.


Offline Loaden

  • Lives here!
  • ****
  • Posts: 1014
Re: The 19 september 2010 build (6608) CODECOMPLETION BRANCH version is out.
« Reply #70 on: September 24, 2010, 09:46:19 am »
Also, It seems that the list of files won't be correct if the main thread enters the critical section before the worker have finished.
I think it does not matter, because the probability is too small.
Even if other implementations, can not be perfect!

Probably it will be better to add an item in the list -> "Processing 10%"
I think it is not necessary.

Offline Loaden

  • Lives here!
  • ****
  • Posts: 1014
Re: The 19 september 2010 build (6608) CODECOMPLETION BRANCH version is out.
« Reply #71 on: September 24, 2010, 09:48:43 am »
well :

Code
return m_Locker != 0;

NULL :  :-(    , please C++0X come fast (or at least gcc implement it fast) nullptr.


It is easy replace all from "NULL" to "nullptr", but it is diffcult replace all from "0" to "nullptr".

Offline killerbot

  • Administrator
  • Lives here!
  • *****
  • Posts: 5297
Re: The 19 september 2010 build (6608) CODECOMPLETION BRANCH version is out.
« Reply #72 on: September 24, 2010, 10:02:05 am »
I knew this argument would be brought up ;-)

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 13438
    • Travis build status
Re: The 19 september 2010 build (6608) CODECOMPLETION BRANCH version is out.
« Reply #73 on: September 24, 2010, 10:31:04 am »
As far as I know nullptr is defined somewhere in CB's sources, so it could be used.

p.s. nullptr will be in gcc 4.6 :)
(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 Loaden

  • Lives here!
  • ****
  • Posts: 1014
Re: The 19 september 2010 build (6608) CODECOMPLETION BRANCH version is out.
« Reply #74 on: September 24, 2010, 10:45:53 am »
Add new feature for remove all bookmarks