Community
Participate
Working Groups
CDT 7.1.0.201007271806 (nightly build) Steps to reproduce: 1. Open a non trivial file (1K lines+) with the ProblemBinding checker activated 2. Modify the code, save. 3. Notice the indexing job taking a long time. The call stack reveals that the checker is spending most of its time in isInClassContext. The ckecker now takes minutes to complete. I also experienced bug 220585 many times because of this. I don't think there is a need to get the actual ast node of the class in this case. We only need to need if it's a class (or struct?), so we could check if the binding is an instance of ICompositeType and return a bool.
Not sure how you propose to fix it. Is patch coming?
I checked the performance. It is unacceptable for run as type checkers. On my machine all checkers runs 2-3 milliseconds each. This one runs 3 min. If I remove most of isInClassContext method it gets down to 152 ms which is still too much... If we cannot improve it we cannot leave it on.
Created attachment 175646 [details] proposed fix I agree, this can be done on index simpler. I was just starting to explore the wonders of PDOM back then. This patch should do the trick, but please have a look there. BTW- there's a discouraged access warning there; thats a simple function, you can copy/paste the contents to clear the warning, but I really think that Codan should have access to org.eclipse.internal.core.dom. Eclipse infractructure allows that, doesn't it?
(In reply to comment #3) > This patch should do the trick, but please have a look there. Looks good to me. > BTW- there's a discouraged access warning there; thats a simple function, you > can copy/paste the contents to clear the warning, but I really think that Codan > should have access to org.eclipse.internal.core.dom. Eclipse infractructure > allows that, doesn't it? I think you'd add cdt.codan.checkers as a friend of cdt.core. I'm not sure it's worth it for a method this small. (In reply to comment #2) > I checked the performance. It is unacceptable for run as type checkers. > On my machine all checkers runs 2-3 milliseconds each. This one runs > 3 min. If I remove most of isInClassContext method it gets down to 152 ms > which is still too much... If we cannot improve it we cannot leave it on. I agree that performance could be improved and I will work on that. If we can't bring it down further then it could be off by default but still included. I used this checker for weeks on a very big project and the ~150 ms version was acceptable to me. I opened bug 321436 for this issue.
fixed on head (8.0), thanks for the patch
*** cdt cvs genie on behalf of elaskavaia *** Bug 321383 - fixed performance issue for ProblemBindingChecker [*] ProblemBindingChecker.java 1.2 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/ProblemBindingChecker.java?root=Tools_Project&r1=1.1&r2=1.2
(In reply to comment #5) > fixed on head (8.0), thanks for the patch I don't think the patch was correctly applied. Can you double-check? Thanks!
yeah, apparently I did not apply the patch - I just committed the code I was playing with, thanks for noticing.
