Community
Participate
Working Groups
I20070503-1400 1. Open 'sun.net.www.protocol.http.HttpURLConnection.class' Is: Even on my pretty fast machine this takes like 10/15 seconds Should: Be faster or show a progress
JDT Core seems to create/open zip files like hell when opening this class.
Created attachment 66295 [details] profiler data
NOTE to the profiler data: the debug* files got wrongly included in the ZIP and are not related.
This is nuts! We're calling JavaModelManager.getZipFile() 1000's of times. 8x for rt.jar & 2281 times for src.zip!
This problem isn't in 3.1.2, but its in 3.2.2.
I'll investigate it. Something must be wrong in the roots detection.
We have two issues. 1) We must cache somehow that the class file has no source. The ClassFile editor tries many, many times to get either the source or the source range of some elements. Each time we try to get the source for the same class file even if we could not find it the first time. And since the current rootPath doesn't work we rescan all the roots again. If we cache the fact that classFile has no source, then it is pretty fast to open the class file editor. I hacked the code to set a new tag bit on the element info and I could get back to some more normal performance. Jérôme, if you could take care of caching this information ? 2) We should open the zip file only once when we iterate over the roots. The attached patch is fixing this issue.
Created attachment 66397 [details] Proposed fix Proposed patch to open the zip file only once when iterating the roots.
It looks like the source for the ClassFile java element is requested 745 times during the opening of the ClassFile editor. This is shared between the ClassFileEditor itself and the Java outliner. For the outliner, the problem seems to come from: org.eclipse.jdt.internal.core.BinaryMember#getCategories() SourceMapper mapper= getSourceMapper(); if (mapper != null) { // ensure the class file's buffer is open so that categories are computed ((ClassFile)getClassFile()).getBuffer(); if (mapper.categories != null) { String[] categories = (String[]) mapper.categories.get(this); if (categories != null) return categories; } } return CharOperation.NO_STRINGS; } In this case, ((ClassFile)getClassFile()).getBuffer(); requests the source of the ClassFile. We definitely need a way to cache the fact that a class file has no source and there is no need to try to get it over and over again.
Jerome - isn't it going into the buffer manager ?
It doesn't use the buffer manager since there is no source.
Opened bug 186144 for the JDT/Text issue.
I don't think this is a client issue: when we have the source we also access it many times and this is expected to be fast. Are you saying we should no do the caching on the client side?
What I am saying it that both sides are guilty. We should be faster to return the source or no source when there is none, but I consider it insane the number of times the source is requested.
I think the right approach to this bug is that JDT/Core caches the fact that a class file has no source (as Olivier already said). This way clients can do as many calls as they wish, and the proposed patch (even if it is good) is not needed to fix the slowness.
No it isn't. But it will prevent too many zip accesses even on the first access.
Created attachment 66497 [details] Proposed fix The two patches are complementary. This patch introduces the notion of NullBuffer for a class file. Doing this, we reuse the buffer manager caching mechanism for free.
Jérôme, please review the last patch. Let me know what you think. I checked that on the project close() the NullBuffer is properly removed from the cache. Also this patch is passing all model tests.
Last patch looks good.
Why is wrong about the first patch ? I know it is not essential to fix this issue, but it would still prevent the zip file from being opened too many times (n + 1 times for n root instead of 2).
Released for 3.3RC1. Verifier, please check the test case described in comment 0.
I released only the second patch since I didn't get Jérôme's approval on the first one.
(In reply to comment #20) > Why is wrong about the first patch ? > I know it is not essential to fix this issue, but it would still prevent the > zip file from being opened too many times (n + 1 times for n root instead of > 2). Just give me some time to review it. The changes are not obvious. Changes in the last patch were much simpler.
Verified for 3.3 RC1 using I20070515-0010
