Symbols browser issue of CC has been fixed for my Linux

[kipade]

Since wx3.x, symbols browser of CC was disabled because of crash.
However, I think I need this basic function, so, I re-open the function code of CC and do a two hours test,
and found the problem of CC plugin:
1) It do GUI operations within worker thread although there is some protection, which will make it crashing
     in xcb library.
>> For this issue, I just rebuild the symbols tree with GUI main thread simply, then, the xcb crash disappear.
     but it still would crash frequently.
I took two hours to do a black-box test, and found it crashed only for some files, so, i create such a simple
project only included some files might cause the issue. And found the crash just because sort the symbols
tree node. tree->SortChildren, which is the standard interface of wx!
    I traced the crash and found the issue in std::sort. My OS is GNU/Slackware, with gcc-9.2, and the crash
point located in: /usr/include/c++/9.2.0/bits/stl_algo.h

Code: [Select]

  /// This is a helper function for the sort routine.
  template<typename _RandomAccessIterator, typename _Compare>
    void
    __unguarded_linear_insert(_RandomAccessIterator __last,
      _Compare __comp)
    {
      typename iterator_traits<_RandomAccessIterator>::value_type
__val = _GLIBCXX_MOVE(*__last);
      _RandomAccessIterator __next = __last;
      --__next;
      while (__comp(__val, __next))  ---->ISSUE: might go ahead of the first iterator
{
  *__last = _GLIBCXX_MOVE(*__next);
  __last = __next;
  --__next;
}
      *__last = _GLIBCXX_MOVE(__val);
    }and found the other issue:
2) the non-deterministic compares will make std::sort(Introspective Sort)'s second step will go ahead the the head of the
vector array, which lead a invalid reference.

Code: [Select]

    if (!lhs || !rhs)
        return 1;
    if (lhs->m_SpecialFolder != sfToken || rhs->m_SpecialFolder != sfToken)
        return -1;
    if (!lhs->m_Token || !rhs->m_Token)
        return 1;the code slice above is cited from CCTreeCtrl's treenode compare function, that will got some  non-deterministic order
of some items. I simple fixed by expanding the || operation.

Because my locale code is different with the svn code, I just packaged my changed in a tarball and uploaded it here.
I submitted my simple solution at: https://github.com/kipade/codeblocks_sf/commit/f607bfb8e2f23645ba3f2e1900d41bea488fb081

I tested the solution on my computer. My environment is:
GNU/Slackware X86_64, current, with wx3.1.3
You are welcome to test it. And I home some developers can make a professional fix the release the right version.

[oBFusCATed]

Good but what is the performance of this change?
What happens if you open a big project?
How much time does it take to execute the ClassBrowser::SyncBuildTree function?

Keep in mind that everything above 50-100msec is unacceptable.

[kipade]

Good question. Yes, I have not done enough pressure test.
but, as i said this just a simple solution and expect the right professional fix. also is a proof of the bug issue
Another word, I had tested using some of my big projects, it work fine, working at least.

[oBFusCATed]

We know what the problem is.
There is no single person willing to spend the time to do the correct fix.
I don't have the time nor interest in doing this any time soon.

ollydbg had a similar fix in some of the topics about this issue.

[kipade]

If so, I think do it by myself.
Fortunately, I will have some time slices in dutu
y days.I will do my best to improve it, If I can, an If I can do.

[RRomano001]

We know what the problem is.
There is no single person willing to spend the time to do the correct fix.
I don't have the time nor interest in doing this any time soon.

ollydbg had a similar fix in some of the topics about this issue.

Hi, your word scary me a lot about this.

 I feel this scenario:
 Bug was known but none is interested to fix it?
 Leaving this bug fire a dangling reference to somewhere in memory, this seems work on windows where memory leakage come mainly from OS than application and ignored. Linux/Unixes caught it and abort the offending program when it try access memory area without owner right.

 A similar fix was proposed but never applied to.. This is again exposing C::B to dangling code endangering reliability of a great program.

 Am I wrong about?

[oBFusCATed]

Am I wrong about?

The feature is disabled when it leads to problems, so users don't hit the problem.

And yes every software is full of bugs, many of them are known to the developers, but they don't have time to fix all of them. That's life.

[kipade]

I think I can realize the reality.
The fact is, CB just an opensource project and with only few volunteers working for it using their free time with no pay. I also think ALL the developing users of CB will have the obligation to fix its bugs and improve it, only this can make CB become better and better.

We know what the problem is.
There is no single person willing to spend the time to do the correct fix.
I don't have the time nor interest in doing this any time soon.

ollydbg had a similar fix in some of the topics about this issue.

However, I don't agree this.  If there were some known bugs with potential solution, someone should share the issue with others even though he or she was no interest in fixing it, instead leaving other users to wasting time to re-work for it.

And, as I said before, I will take some time within every duty day to learning and improve CB, do what I can do to improve CB. I hope it can be better.
I also hope everyone here, not only tying your king fingers but also open your arms to improve such an opensource product.
Regards.

[ollydbg]

ollydbg had a similar fix in some of the topics about this issue.

This is the direction I would like to go if I have time, but it looks like this is quite complex.
Especially if we have a lot of tokens to shown in the tokentree.
See here:
Any method to build a big wxTreeCtrl progressively in GUI thread?

[Miguel Gimenez]

I am currently working on a solution and it is almost complete (it needs some polishing and testing).

I create the tree in a thread (ThreadedBuildTree() lasts 30 ms) using a non-GUI tree class, and then use the creation end event to dump the tree to the wxTreeCtrl in the main thread. This part, of course, is time critical, but dumping a ready made tree is way faster than creating it.

Once created the two trees are in sync, and both expand and collapse dynamically and quickly.

I have attached the current (not final) patch, so it can be tested/commented/used as reference or rejected.

[oBFusCATed]

Do you have some time measurements how much does it take to do the copy from CCTree to wxTreeCtrl? I'm interested to see the numbers for C::B's project/workspace for example.
10ms is something like the maximum allowed for this process. Anything else would cause bad pauses. So you should prepare the code to do this copy operation in batches in multiple on-idle/call-after events.

I've just taken a quick look at the patch, some comments:
1. I don't like std::lists; they are slow, really slow (https://www.youtube.com/watch?v=YQs6IC-vgmo). If you really need a linked list this should be your last resort and you should use intrusive lists.
2. Even more I don't like putting lists/vectors as members in tree item objects. This is massive performance disaster, because your algorithm would be dominated by calls to new/delete and the memory access pattern would be a disaster from CPU's point of view! If you want to be fast you want to use linear arrays as much as possible! And your nodes must be minimal in size.
3. The use of CCTree seems rather complex and it is a lot of code Generally I think the whole SymbolBrowser code should be redone. Currently it is linked/coupled too much to the implementation of the C/C++ CC plugin. Last time I've looked the fortran plugin had a copy of this code.

[Miguel Gimenez]

The use of CCTree seems rather complex

It has exactly the same interface of wxTreeCtrl without the GUI part, the only visible difference are the use of pointers to items and the FindFirstChild() cookie type.

I'll measure the performance tomorrow and will look for a list replacement.

[New Pagodi]

You can build a n-ary tree structure without the need to use a list/vector structure for the child nodes using a structure like this:

Code: [Select]

struct NaryTreeNode
{
    void* data;
    NaryTreeNode* nextSibling;
    NaryTreeNode* firstChild;
};
You can iterate over the children of a NaryTreeNode n like so:

Code: [Select]

for ( NaryTreeNode* i = n->firstChild; i!=NULL; i=i->nextSibling )
{...
The main drawback of this approach is that that you can't get an arbitrary child of a node without iterating over the necessary number of nodes.

I don't know if this is of any help, but I thought I'd mention it.  Sorry if it's not of any help.

[Miguel Gimenez]

@New Pagodi, thank you for your suggestion.

I have just made some measurements with CB project:

- Current projects symbols
    - tree creation 36 ms
    - top tree dump 227 ms
    - bottom tree (creation+dump)
        - Global functions 172+132
        - Global typedefs 45+29
        - Global variables 129+97
        - Macro definitions 1483+1093

- Everything
    - tree creation 427 ms
    - top tree dump 452 ms
    - bottom tree (creation+dump)
        - Global functions 9422+7702
        - Global typedefs 2637+2003
        - Global variables 709+526
        - Macro definitions 39111+26790

I will try a list replacement, hopefully it will reduce the tree creation parts. The dumping part needs more thinking.

[oBFusCATed]

Which of these happen on the main UI thread?