Bugzilla – Full Text Bug Listing
| Summary: | Eclipse hangs when attempting to refactor using the "change method signature" | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] JDT | Reporter: | Steve Garbarini <sgarbarini> | ||||||||||||||||
| Component: | Core | Assignee: | Satyam Kandula <satyam.kandula> | ||||||||||||||||
| Status: | VERIFIED FIXED | QA Contact: | |||||||||||||||||
| Severity: | critical | ||||||||||||||||||
| Priority: | P3 | CC: | daniel_megert, frederic_fusier, jarthana, markus.kell.r, Olivier_Thomann, satyam.kandula, srikanth_sankaran | ||||||||||||||||
| Version: | 3.5.1 | Flags: | daniel_megert:
pmc_approved+
srikanth_sankaran: review+ markus.kell.r: review+ |
||||||||||||||||
| Target Milestone: | 3.5.2 | ||||||||||||||||||
| Hardware: | PC | ||||||||||||||||||
| OS: | Linux | ||||||||||||||||||
| Whiteboard: | |||||||||||||||||||
| Attachments: |
|
||||||||||||||||||
|
Description
Steve Garbarini
Created attachment 157805 [details]
Stack trace when I "break" the debugger during Eclipse hang
The problem is typically highlighted by your code, hence it would be really easier for us to figure out what happens there with an example. As you can debug it and break in the offending loop, you can step in the debugger and then get all information from the stack trace. So, you could have a more precise idea about the code area where the Scanner seems to become crazy. Then hopefully, you should be able to extract a small sample and put it in this bug to help us to reproduce this issue... TIA I don't see an infinite loop in this code (org.eclipse.jdt.internal.compiler.parser.Scanner.checkTaskTag(int, int)). Do you have task tags in the corresponding file ? If you could at least give us the code where the scanner is running checkTaskTag, this could be really helpful. As is, it is difficult to see what could be wrong. I can do remote debugging if required. I'll see what I can do to narrow it down. I guess I need to download the source code to do effective debugging, yes? I "broke" into the debugger several more times... the common method seems to be: Parser.checkNonNLSAfterBodyEnd(int) line: 1139 so maybe it is possible that that method is in an infinite loop? Possibly.
The code is doing:
protected void checkNonNLSAfterBodyEnd(int declarationEnd){
if(this.scanner.currentPosition - 1 <= declarationEnd) {
this.scanner.eofPosition = declarationEnd < Integer.MAX_VALUE ? declarationEnd + 1 : declarationEnd;
try {
while(this.scanner.getNextToken() != TokenNameEOF){/*empty*/}
} catch (InvalidInputException e) {
// Nothing to do
}
}
}
So this would mean that somehow we never end up returning a TokenNameEOF token. This can technically be an infinite loop, but I would like to understand why the scanner is initialized in a way that never returns a token TokenNameEOF.
So a regression test would be welcome.
Do you have syntax errors ?
I have no syntax errors and, as it turns out, Eclipse was not hung, so periodically "breaking" the debugger just showed me where the code was most of the time (and, of course, it was in getNextToken or thereabouts). I finally downloaded the source code so I could debug effectively. The code was looping through 926 classes in: MethodChecks.isDeclaredInInterface(IMethod method, ITypeHierarchy hierarchy, IProgressMonitor monitor) and it took aver 20 minutes to return from that method. As I write this comment the code now seems "hung" again because the "Checking preconditions..." dialog is sitting there with no progress. I assume if I leave it long enough that it will return. This is on a Core i7-860 where 1GB was alloted to Eclipse. I'm an Eclipse fan, but Netbeans was able to perform the entire refactor in less than 5 minutes (and it showed progress whenever the system was busy... Eclipse did not and that is one of the reasons I thought it was hung). Anyway, thanks for being interested in the problem. (In reply to comment #6) > I'm an Eclipse fan, but Netbeans was able to perform the entire refactor in > less than 5 minutes (and it showed progress whenever the system was busy... > Eclipse did not and that is one of the reasons I thought it was hung). I'd like to debug it myself so that I can try to understand why it is looping. Now as I understood, it is not possible to get access to the source code. So unless you can provide reproducable steps, I don't see how we can proceed. I've tried to model our real code with some more simplistic code. I've created something that is slow (30 seconds before there is any indication that Eclipse isn't hung), but it is a far cry from 20 minutes. I think the problem has to do with large numbers of inner classes (it is a method in the base class of all those inner classes that is getting refactored). If I'm understanding the code right, these multiple inner classes cause the outer class to get parsed multiple times. Anyway, see the attached zip field and try to refactor (change the method signature of) Foo.method1. Created attachment 157869 [details]
Zip of a project that demonstrates the slowness of the refactor operation
I got a similar call stack with the file GenericTypeTest of jdt core test. Open this file, right click on this element on the package explorer and try to click on Run As. Satyam, please investigate. Created attachment 158662 [details] Proposed patch Parsing the methods which is not necessary is causing the slowdown. A part of the fix for bug 254738 is causing the un-necessary parsing of the methods. This part is actually not required because of the subsequent fix for bug 288621. So, I removed those unnecessary changes. Created attachment 158672 [details]
Test patch
Test to demonstrate the time taken with and without the patch. I don't think it is necessary to put it in the regression test.
(In reply to comment #12) > This part is actually not required because of the subsequent fix for bug 288621. This is my mistake, 288621 shouldn't have any impact. However, the changes that were done earlier are not necessary and hence they can be removed as stated in the attached patch. This is indeed a critical regression that got introduced into R3.5.1 and a must fix for 3.5.2. Approving to fix for 3.5.2. Created attachment 158831 [details]
Proposed patch for 3.5 maintenance stream
Refactorings and selection of "Run As" in the context menu are most impacted. The impact varies depending on the size of the file! Basically calls to newSupertypeHierarchy() on a type that has the CU open will experience slowdown. Method statements which weren't getting parsed of an already open CU was getting parsed causing the slowdown. Here are some performance numbers of the call to newSupertypeHierarchy() on some JDT/core source files: Parser.java: ~10K lines 3.5 - ~12 ms 3.5.2 - ~1000 seconds 3.5.2 with fix - ~12 ms HierarchyResolver.java : ~800 lines 3.5 - ~3 ms 3.5.2 - ~15 ms 3.5.2 with fix - ~3 ms JavaCore.java: ~5K lines 3.5 - ~12 ms 3.5.2 - ~334 ms 3.5.2 with fix - ~12 ms (In reply to comment #12) > Created an attachment (id=158662) [details] > Proposed patch > > Parsing the methods which is not necessary is causing the slowdown. A part of > the fix for bug 254738 is causing the un-necessary parsing of the methods. This > part is actually not required because of the subsequent fix for bug 288621. So, > I removed those unnecessary changes. I agree with the change. But could you please add a suitable comment at the point in source. If it was mistaken to be superfluous once, it can be mistaken again too. I meant 1000 ms not 1000 seconds for Parser.java on 3.5.2 (In reply to comment #17) > Refactorings and selection of "Run As" in the context menu are most impacted. > The impact varies depending on the size of the file! > > Basically calls to newSupertypeHierarchy() on a type that has the CU open will > experience slowdown. Method statements which weren't getting parsed of an > already open CU was getting parsed causing the slowdown. > > Here are some performance numbers of the call to newSupertypeHierarchy() on > some JDT/core source files: > > Parser.java: ~10K lines > 3.5 - ~12 ms > 3.5.2 - ~1000 seconds > 3.5.2 with fix - ~12 ms > > HierarchyResolver.java : ~800 lines > 3.5 - ~3 ms > 3.5.2 - ~15 ms > 3.5.2 with fix - ~3 ms > > JavaCore.java: ~5K lines > 3.5 - ~12 ms > 3.5.2 - ~334 ms > 3.5.2 with fix - ~12 ms For the attached testcase, call to newSupertypeHieararchy() on one of the subtypes of Foo() in Bar1 takes 400 ms with 352 and 4ms with 3.5 and the patch. (In reply to comment #16) > Created an attachment (id=158831) [details] [diff] > Proposed patch for 3.5 maintenance stream +1 for this patch. The patch from comment 12 did too much. With 3.5.2 "Change method signature" on the test case provided takes around 4 mins for the dialog box to come up. Whereas with 3.5 it just takes less than 8 seconds. With the attached patch also, it takes less than 8 seconds. Created attachment 158845 [details] Patch on 3.5 maintenance branch Added a comment as Srikanth had mentioned in comment 18. Created attachment 158846 [details]
Patch on Head
Made the changes similar to the one on 3.5 maintenance.
Released for 3.5.2. Released for 3.6M6 with the performance regression test. I released the regression test in the perf_35x branch. Verified for 3.6M6 using build I20100305-1011 *** Bug 298948 has been marked as a duplicate of this bug. *** |