Author Topic: Symbols browser issue of CC has been fixed for my Linux  (Read 2480 times)

Offline kipade

  • Multiple posting newcomer
  • *
  • Posts: 44
Symbols browser issue of CC has been fixed for my Linux
« on: November 22, 2019, 05:37:54 am »
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.

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 12129
    • Travis build status
Re: Symbols browser issue of CC has been fixed for my Linux
« Reply #1 on: November 22, 2019, 10:28:17 am »
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.
(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: 44
Re: Symbols browser issue of CC has been fixed for my Linux
« Reply #2 on: November 22, 2019, 12:21:33 pm »
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.

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 12129
    • Travis build status
Re: Symbols browser issue of CC has been fixed for my Linux
« Reply #3 on: November 22, 2019, 09:06:29 pm »
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.
(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: 44
Re: Symbols browser issue of CC has been fixed for my Linux
« Reply #4 on: November 23, 2019, 04:22:55 pm »
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.

Offline RRomano001

  • Multiple posting newcomer
  • *
  • Posts: 10
Re: Symbols browser issue of CC has been fixed for my Linux
« Reply #5 on: November 24, 2019, 12:28:36 pm »
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?

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 12129
    • Travis build status
Re: Symbols browser issue of CC has been fixed for my Linux
« Reply #6 on: November 24, 2019, 01:17:08 pm »
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.
(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: 44
Re: Symbols browser issue of CC has been fixed for my Linux
« Reply #7 on: November 25, 2019, 02:59:16 am »
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.

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5247
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: Symbols browser issue of CC has been fixed for my Linux
« Reply #8 on: November 25, 2019, 06:16:14 am »
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?
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 Miguel Gimenez

  • Regular
  • ***
  • Posts: 383
Re: Symbols browser issue of CC has been fixed for my Linux
« Reply #9 on: November 25, 2019, 09:46:02 am »
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.

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 12129
    • Travis build status
Re: Symbols browser issue of CC has been fixed for my Linux
« Reply #10 on: November 25, 2019, 08:18:15 pm »
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. :(
(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 Miguel Gimenez

  • Regular
  • ***
  • Posts: 383
Re: Symbols browser issue of CC has been fixed for my Linux
« Reply #11 on: November 25, 2019, 08:32:39 pm »
Quote
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.

Offline New Pagodi

  • Multiple posting newcomer
  • *
  • Posts: 34
Re: Symbols browser issue of CC has been fixed for my Linux
« Reply #12 on: November 25, 2019, 11:21:16 pm »
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.

Offline Miguel Gimenez

  • Regular
  • ***
  • Posts: 383
Re: Symbols browser issue of CC has been fixed for my Linux
« Reply #13 on: November 26, 2019, 09:44:56 am »
@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.

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 12129
    • Travis build status
Re: Symbols browser issue of CC has been fixed for my Linux
« Reply #14 on: November 26, 2019, 07:54:42 pm »
Which of these happen on the main UI thread?
(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 Miguel Gimenez

  • Regular
  • ***
  • Posts: 383
Re: Symbols browser issue of CC has been fixed for my Linux
« Reply #15 on: November 26, 2019, 08:14:17 pm »
The second (dump)


Edit: both are executed in the main thread, and the first number was mistakenly including the second. In my next message the numbers are corrected and compared with the new version
« Last Edit: November 27, 2019, 04:30:52 pm by Miguel Gimenez »

Offline Miguel Gimenez

  • Regular
  • ***
  • Posts: 383
Re: Symbols browser issue of CC has been fixed for my Linux
« Reply #16 on: November 27, 2019, 12:31:51 pm »
I have changed the std::list to a kind of intrusive list (each element has pointers to parent, previous and next siblings and first child).

I still have to adapt the quicksort algorithm, but the timings with CB project look better. Below are the old times (tree creation times revised) wth the new ones at the right side (remember, no sort).

- Current projects symbols
    - tree creation 36 ms, now 8 ms
    - top tree dump 227 ms, now 15 ms
    - bottom tree (creation+dump)
        - Global functions 40+132, now 21+105
        - Global typedefs 16+29, now 13+26
        - Global variables 32+97, now 17+77
        - Macro definitions 390+1093, now 167+707

- Everything
    - tree creation 427 ms, now 241 ms
    - top tree dump 452 ms, now 159 ms
    - bottom tree (creation+dump)
        - Global functions 1720+7702, now 838+2264
        - Global typedefs 634+2003, now 329+1496
        - Global variables 183+526, now 114+435
        - Macro definitions 12321+26790, now 5803+14174

The top tree creation is made in the worker thread, while the dump and the bottom tree work are made in the main thread.

Offline Miguel Gimenez

  • Regular
  • ***
  • Posts: 383
Re: Symbols browser issue of CC has been fixed for my Linux
« Reply #17 on: November 28, 2019, 11:23:01 am »
Quicksort reactivated; the timings for tree creation have increased, but not much

21 -> 30
13 -> 14
17 -> 23
167 -> 215
838 -> 978
329 -> 384
114 -> 143
5803 -> 6211

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 12129
    • Travis build status
Re: Symbols browser issue of CC has been fixed for my Linux
« Reply #18 on: November 30, 2019, 10:33:20 am »
The posts you're making aren't very clear.
As I've said earlier we're interested only/mostly in the timings in the main thread.
How long does it take to execute the workers is not that important (it is, but it is secondary).
The question is what is taking the time? You'll have to use a profiler. Try either vtune or perf.
VTune is easier to setup for stack-gathering and it recently switched to be free instead of being mightily expensive.

As I've said previously the main thread must not pause for more than 10ms during this operation.
After you know what is slow you could decide what is best course of action. I won't be surprised that the tree-ctrl api is slow.
(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: 5247
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: Symbols browser issue of CC has been fixed for my Linux
« Reply #19 on: November 30, 2019, 10:41:22 am »
One idea: why not creating the whole tree, is there any optimized way we can build some part of the tree which are shown to the user. That's only a very small part of the whole tree. If user scroll up and down the tree, we have to build the tree gradually.
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: 12129
    • Travis build status
Re: Symbols browser issue of CC has been fixed for my Linux
« Reply #20 on: November 30, 2019, 10:43:44 am »
For lists this concept/mode is called virtual list. We use this to make opening the alt-g dialog to be really fast no matter the size of the project (the dialog opens in <100ms even on the linux kernel project (40+k files)).

I'm not sure this is available for trees though :( Probably we could do create-children-on-expand thing, but I'm not sure if this would benefit us.

As I've said we first need to know what is slow then we could find a solution. And more importantly why it is slow. Then we'll be able to find a solution. Guessing about performance is never a good idea.
« Last Edit: November 30, 2019, 10:45:48 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 Miguel Gimenez

  • Regular
  • ***
  • Posts: 383
Re: Symbols browser issue of CC has been fixed for my Linux
« Reply #21 on: November 30, 2019, 11:59:10 am »
I was using wxStopWatch.

The code works the same as before, only the dump parts have been added, and they use the main thread because it is the only one allowed to access the CCTreeCtrl.

- The top tree creation is made in the worker thread as before.
- The worker thread only activates (now and before) when the top tree changes due to a code completion token tree change.
- The top tree dump is made by the main thread when the tree is finished (this part is new).
- The bottom tree creation and dump are made by the main thread in the OnSelect event (just like before). Of course, the dump adds delay.

The tree leaves are created dynamically when expanding their parent, and deleted when the parent is collapsed, just like in the original code. This work is (and was) made by the main thread.

I find it quite usable if you don't use the Everything option.

I have attached my final changes, just in case anybody wants to test/profile/modify or reuse parts.

Offline earlgrey

  • Multiple posting newcomer
  • *
  • Posts: 102
Re: Symbols browser issue of CC has been fixed for my Linux
« Reply #22 on: November 30, 2019, 09:00:20 pm »
Seems to work, thank you.
* OS = Debian Buster - Linux 4.19.06 x64 SMP
* C::B = svn11267 wx-3.0.4 - Linux, unicode 64 bit

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5247
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: Symbols browser issue of CC has been fixed for my Linux
« Reply #23 on: December 02, 2019, 03:57:36 pm »
I was using wxStopWatch.

The code works the same as before, only the dump parts have been added, and they use the main thread because it is the only one allowed to access the CCTreeCtrl.

- The top tree creation is made in the worker thread as before.
- The worker thread only activates (now and before) when the top tree changes due to a code completion token tree change.
- The top tree dump is made by the main thread when the tree is finished (this part is new).
- The bottom tree creation and dump are made by the main thread in the OnSelect event (just like before). Of course, the dump adds delay.

The tree leaves are created dynamically when expanding their parent, and deleted when the parent is collapsed, just like in the original code. This work is (and was) made by the main thread.

I find it quite usable if you don't use the Everything option.

I have attached my final changes, just in case anybody wants to test/profile/modify or reuse parts.
I try to apply the patch file "symbol_browser.patch" in my local git repo.

But it looks like I have no way to apply the patch. :(

Code: [Select]
$ patch -p0 ../symbol_browser.patch

This will make the patch command hangs without  returning, I have to press the "CTRL+C" to halt the patch command. (I'm using the shell from git-for-windows)


EDIT:
There is another issue, I see this patch contains some changes like:
Code: [Select]
-            const TokenIdxSet* tokens = tree->GetTokensBelongToFile(*itf);
+            const TokenIdxSet *tokens = tree->GetTokensBelongToFile(*itf);
I would still prefer the old styling.  :)
« Last Edit: December 02, 2019, 04:01:06 pm 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 Miguel Gimenez

  • Regular
  • ***
  • Posts: 383
Re: Symbols browser issue of CC has been fixed for my Linux
« Reply #24 on: December 02, 2019, 06:40:03 pm »
Here you have a revised copy with the coding standard restored. I prefer the other way, but obviously the existing standard must be respected.

The patch is generated with svn (svn diff > symbol_browser.patch), I can generate it with svn diff --git if you need it.
in the next message you have a version generated with --git option.

I have been testing timings in Symbol Browser with 17.12, and the delays in main thread are similar (as I said above, most of the work in the bottom tree is, and was, made by the main thread).
« Last Edit: December 02, 2019, 06:47:25 pm by Miguel Gimenez »

Offline Miguel Gimenez

  • Regular
  • ***
  • Posts: 383
Re: Symbols browser issue of CC has been fixed for my Linux
« Reply #25 on: December 02, 2019, 06:45:56 pm »
I have attached a patch file generated with svn diff --git

Offline kipade

  • Multiple posting newcomer
  • *
  • Posts: 44
Re: Symbols browser issue of CC has been fixed for my Linux
« Reply #26 on: December 10, 2019, 04:04:19 am »
I have attached a patch file generated with svn diff --git
after several busy days, I tried this patch and it works find for me.
I use git, use this command to do:
Code: [Select]
git apply ~/Downloads/symbol_browser_git.patch  -p2
Thanks for this perfect work and patch.

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 12129
    • Travis build status
Re: Symbols browser issue of CC has been fixed for my Linux
« Reply #27 on: December 10, 2019, 09:24:41 am »
Thanks for this perfect work and patch.
I guess your project is either small or you could tolerate long UI pauses.
(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 Miguel Gimenez

  • Regular
  • ***
  • Posts: 383
Re: Symbols browser issue of CC has been fixed for my Linux
« Reply #28 on: December 10, 2019, 02:03:10 pm »
Most of the pauses are already in the original code, the greatest one is in the OnSelect() event due to full bottom tree creation in main thread.

Offline kipade

  • Multiple posting newcomer
  • *
  • Posts: 44
Re: Symbols browser issue of CC has been fixed for my Linux
« Reply #29 on: December 11, 2019, 02:24:25 am »
Most of the pauses are already in the original code, the greatest one is in the OnSelect() event due to full bottom tree creation in main thread.
Yes, you are right. there are some short pauses when click the top tree items.
Since it have to be done in the main UI thread, I think no way to avoid that if did not keep all parsed symbols tree item in memory.