Community
Participate
Working Groups
Build ID: 3.4.2 M20090211-1700 Steps To Reproduce: 1. Setup a project which has large binary and source jars in the build path. 2. Try to get java proposals 3. Sometimes it's very slow or hangs More information: We have this stack trace during one of these hanging operations: "main" prio=10 tid=0x080c5c00 nid=0x4a2e runnable [0xffd35000..0xffd38298] java.lang.Thread.State: RUNNABLE at java.util.zip.ZipFile.open(Native Method) at java.util.zip.ZipFile.<init>(ZipFile.java:114) at java.util.zip.ZipFile.<init>(ZipFile.java:131) at org.eclipse.jdt.internal.core.JavaModelManager.getZipFile(JavaModelManager.java:2203) at org.eclipse.jdt.internal.core.SourceMapper.findSource(SourceMapper.java:912) at org.eclipse.jdt.internal.core.SourceMapper.getSourceForRootPath(SourceMapper.java:891) at org.eclipse.jdt.internal.core.SourceMapper.findSource(SourceMapper.java:864) at org.eclipse.jdt.internal.core.SourceMapper.findSource(SourceMapper.java:834) at org.eclipse.jdt.internal.core.BinaryMethod.getParameterNames(BinaryMethod.java:165) at org.eclipse.jdt.internal.codeassist.InternalCompletionProposal.findMethodParameterNames(InternalCompletionProposal.java:120) at org.eclipse.jdt.core.CompletionProposal.findParameterNames(CompletionProposal.java:1950) at org.eclipse.jdt.ui.text.java.CompletionProposalLabelProvider.appendUnboundedParameterList(CompletionProposalLabelProvider.java:106) at org.eclipse.jdt.ui.text.java.CompletionProposalLabelProvider.createMethodProposalLabel(CompletionProposalLabelProvider.java:265) at org.eclipse.jdt.ui.text.java.CompletionProposalLabelProvider.createStyledLabel(CompletionProposalLabelProvider.java:536) at org.eclipse.jdt.internal.ui.text.java.LazyJavaCompletionProposal.computeDisplayString(LazyJavaCompletionProposal.java:245) at org.eclipse.jdt.internal.ui.text.java.LazyJavaCompletionProposal.getStyledDisplayString(LazyJavaCompletionProposal.java:224) at org.eclipse.jface.text.contentassist.CompletionProposalPopup.handleSetData(CompletionProposalPopup.java:824) at org.eclipse.jface.text.contentassist.CompletionProposalPopup.access$24(CompletionProposalPopup.java:814) at org.eclipse.jface.text.contentassist.CompletionProposalPopup$3.handleEvent(CompletionProposalPopup.java:580) I have a proposed patch which I will attach.
Please attach your patch. I don't see a deadlock. Jay, please investigate.
Created attachment 140240 [details] Proposed patch to fix the issue Sorry, thought I had attached it when I did the original report. It doesn't deadlock, but if you have big jar files on non-local disk (or even sometimes local), it pauses for long enough to seem like a hang, several minutes in some cases. I haven't been able to confirm with our users who reported the issue that this patch completely fixes the symptoms, but it shouldn't make it any worse.
Patch looks interesting. Would be good to get feedbacks on improvement using this patch.
Feedback from our users who have been using the patch is positive. They say it doesn't hang for 20+ seconds like it did before, but still has slight pauses which are still annoying so it is an improvement.
Created attachment 142439 [details] Slightly better proposed patch There's an earlier call to getSourceForRootPath() I should have wrapped with the zip file cache too. Updated patch attached.
I definitely see some improvement with this patch, at least in one scenario. When the source jar is attached to the project as a library (along with the binary) (and not as source attachment), we end up opening the same zip file more than once in the same thread. Inside the SourceMapper alone, the zip file is opened twice. The patch gets rid of one of them. In fact, there are other parties that are doing this in the same thread as well. I will spend some more time investigating and see if we can move this caching call further outside in the invoking methods.
With my performance test, I see about 4% performance improvement in terms of elapsed process time. This was done with binary and source files each of size about 50MB. While this is a reasonable performance improvement, I do not know if it can make such a bottleneck without this change. Kelly, can you tell me what is the size of the jar/zip files you tested this with?
Olivier, The test FullSourceWorkspaceModelTests#testGetSourceBigJarNoAttachment() recorded a 4% improvement in terms of elapsed process time. And I am fine with the changes. Do you think the performance improvement is good enough to allow the patch?
The jar size we were testing with was about 300MB. It was causing pauses of approx 20 seconds which are now < 1 sec with the patch.
The numbers talk by themselves. Going from 20s down to less than 1s is a significant improvement. +1 for the patch. Jay, please also provide a patch for 3.5 maintenance. I think the improvement deserves a backport to 3.5.2. Make sure that iplog flag is set once the patch is released. Daniel, ok for backporting ?
The patch is fine except for the missing update of the copyright notice. Kelly, please add your credentials to the copyright notice in the following form: name <e-mail> - bugzilla summary - bugzilla URL e.g. Dani Megert <dani@eclipse.org> - this is a bug - https://bugs.eclipse.org... +1 for 3.5.2.
Created attachment 148141 [details] Proposed patch Added copyright info to the patch.
The patch provided with comment #12 goes fine with 3.5 as well.
Released for 3.6M3. Code check is required for verification.
Released for 3.5.2. Thanks for the patch, Kelly.
Improve performances is a good thing but pass existing test is mandatory before saying that a patch is OK! I got 75 failures while running all JDT/Core tests while doing the 3.5.2 build input today!!! There were all in search tests JavaSearchTests and JavaSearchBugsTests... I haven't time to investigate why these tests are failing as I want to make the build input before the maintenance build is launched... So, I revert the change and reopen the bug. Jay, please investigate the reason of these failures.
I get the same failures in HEAD!
Patch reverted in HEAD stream, hence remove the iplog flag on this bug...
It's also breaks some of the org.eclipse.jdt.ui.tests.refactoring.all.AllAllRefactoringTests.
Sorry guys. I did run the tests in the past. I'll investigate.
Created attachment 148460 [details] Proposed fix All existing search tests passed.
Frédéric, please review.
Released for 3.6M3. Verification is still required for 3.5.2 inclusion. Kelly, please double-check that the performance is still improved with what is released in HEAD. Thanks.
Created attachment 148461 [details] Proposed fix Forgot to include the latest change in the copyright.
Created attachment 148462 [details] Proposed fix Hopefully this patch looks good.
Created attachment 148465 [details] Proposed fix for 3.5 maintenance Here is a patch that applies directly on 3.5 maintenance. This replaces the last patch that was for HEAD. Frédéric, please review this patch.
Olivier, The patch looks really good. Just a question about the iplog flag, as the patch comes from you, I doubt we can consider it as an external contribution. I do not want to hide Kelly's initial effort but I think I remember that to be considered as an external contribution a patch should not be modified at all before being released. It's just for a legal matter... otherwise I'm personally completely OK to let this flag on :-)
OK, I'll discuss with John about the iplog flag.
Released for 3.5.2. Code check is required for verification. I keep the iplog set as the contribution is part of the patch.
Verified for 3.6M3 by code inspection.
Verified for 3.5.2 RC2 by code inspection.
Verified.
