Community
Participate
Working Groups
From 4.20 to 4.22 indexing our whole workspace became much slower I sampled a workspace during Preferences/java/"Rebuild Index" 4.20: ~670 sec 4.22: ~1.400 sec in org.eclipse.jdt.internal.core.search.indexing.IndexManager$2.execute () the new hotspot is org.eclipse.jdt.internal.core.builder.ClasspathJar.findPackageSet() during resolve() It is not getting better with -Dorg.eclipse.jdt.disableMetaIndex=true but still i guess it is caused by the meta index (bug 570078) since that hotspot was already reported in Bug 577974, 576245 for 4.21
Created attachment 288199 [details] snapshot-java indexing 4.20.nps
Created attachment 288200 [details] snapshot-java indexing 4.22.nps
Created attachment 288201 [details] screenshot hotspot sampled 4.22.png
@Gayan can that be caused by meta index (bug 570078)?
(In reply to Jörg Kubitz from comment #4) > @Gayan can that be caused by meta index (bug 570078)? Highly unlikely Jörg, since the code responsible for updating the metaindex just intercept into existing name categorization points to extract qualifications for meta index. Seems like this happens when the JRT classpath jar gets scanned right ?
(In reply to Gayan Perera from comment #5) > Highly unlikely Jörg, since the code responsible for updating the metaindex Do you know anything else that could have lead to that regression during 4.21? > Seems like this happens when the JRT classpath jar gets scanned right ? As far as i know yes: the majority are JARs are from JRT. I see our vast majority of the slow index request "IndexManager$2" (seen in the hotspots) are generated in IndexManager.scheduleDocumentIndexing() for auto generated files ins "src-gen". I wonder why it calls JobManager.request() directly instead of using "requestIfNotWaiting()". The queue "awaitingJobs" often already contains requests to index the whole projects. Will those files be indexed multiple times? They even add the IndexAllProject request for that project themself in a recursive "request(IJob)": IndexManager(JobManager).request(IJob) line: 325 <-------------- IndexManager.rebuildIndex(IndexLocation, IPath, boolean) line: 872 IndexManager.rebuildIndex(IndexLocation, IPath) line: 845 IndexManager.aboutToUpdateIndex(IPath, Integer) line: 225 IndexManager$2(IndexRequest).ensureReadyToRun() line: 43 IndexManager(JobManager).request(IJob) line: 325 <-------------- IndexManager.scheduleDocumentIndexing(SearchDocument, IPath, IndexLocation, SearchParticipant) line: 1267 ...
@Stephan according to the Stacktraces it's ClasspathLocation.createAnswer(String, IBinaryType, char[]) that causes all those slow calls to decorateWithExternalAnnotations. It was added with bug 574603 "[null][external] offer more IDE options for specifying external annotation locations". Can you improve on that?
(In reply to Jörg Kubitz from comment #7) > @Stephan according to the Stacktraces it's > ClasspathLocation.createAnswer(String, IBinaryType, char[]) > that causes all those slow calls to decorateWithExternalAnnotations. > It was added with bug 574603 "[null][external] offer more IDE options for > specifying external annotation locations". > > Can you improve on that? Thanks for the heads-up. I need a bit more information: Did you enable JavaCore.CORE_JAVA_BUILD_EXTERNAL_ANNOTATIONS_FROM_ALL_LOCATIONS (which is disabled by default)? If so, this doesn't look like a regression but you are paying the price for a new feature. In this case it might be most promising if we could simply disable this feature during *all indexing* (assuming, that indexes are not interested in null annotations, are they?). Otherwise, if you see this code path invoked without the new option enabled, that would be totally unexpected and I could use an example that demonstrates how we ever get into this situation.
(In reply to Stephan Herrmann from comment #8) > (In reply to Jörg Kubitz from comment #7) > > @Stephan according to the Stacktraces it's > > ClasspathLocation.createAnswer(String, IBinaryType, char[]) > > that causes all those slow calls to decorateWithExternalAnnotations. > > It was added with bug 574603 "[null][external] offer more IDE options for > > specifying external annotation locations". > > > > Can you improve on that? > > Thanks for the heads-up. > > I need a bit more information: > > Did you enable > JavaCore.CORE_JAVA_BUILD_EXTERNAL_ANNOTATIONS_FROM_ALL_LOCATIONS (which is > disabled by default)? no! just put a breakpoint in org.eclipse.jdt.internal.core.builder.ClasspathLocation.createAnswer(String, IBinaryType, char[]) int the "for (ClasspathLocation annotationLocation : this.allLocationsForEEA) " loop. allLocationsForEEA is not null when called from Daemon Thread [Java indexing] (Suspended (breakpoint at line 75 in ClasspathJrtWithReleaseOption)) ClasspathJrtWithReleaseOption.<init>(String, AccessRuleSet, IPath, Collection<ClasspathLocation>, String) line: 75 ClasspathLocation.forJrtSystem(String, AccessRuleSet, IPath, Collection<ClasspathLocation>, String) line: 163 JavaSearchNameEnvironment.mapToClassPathLocation(JavaModelManager, PackageFragmentRoot, IModuleDescription, Collection<ClasspathLocation>) line: 267 JavaSearchNameEnvironment.computeClasspathLocations(JavaProject) line: 200 JavaSearchNameEnvironment.<init>(IJavaProject, ICompilationUnit[]) line: 94 IndexBasedJavaSearchEnvironment.create(List<IJavaProject>, ICompilationUnit[]) line: 27 SourceIndexer.resolveDocument() line: 169 JavaSearchParticipant.resolveDocument(SearchDocument) line: 117 IndexManager.indexResolvedDocument(SearchDocument, SearchParticipant, Index, IPath) line: 662 IndexManager$2.execute(IProgressMonitor) line: 1285 IndexManager(JobManager).run() line: 441 Thread.run() line: 834
(In reply to Jörg Kubitz from comment #9) > (In reply to Stephan Herrmann from comment #8) > > (In reply to Jörg Kubitz from comment #7) > > > @Stephan according to the Stacktraces it's > > > ClasspathLocation.createAnswer(String, IBinaryType, char[]) > > > that causes all those slow calls to decorateWithExternalAnnotations. > > > It was added with bug 574603 "[null][external] offer more IDE options for > > > specifying external annotation locations". > > > > > > Can you improve on that? > > > > Thanks for the heads-up. > > > > I need a bit more information: > > > > Did you enable > > JavaCore.CORE_JAVA_BUILD_EXTERNAL_ANNOTATIONS_FROM_ALL_LOCATIONS (which is > > disabled by default)? > > no! > > just put a breakpoint in > org.eclipse.jdt.internal.core.builder.ClasspathLocation.createAnswer(String, > IBinaryType, char[]) > int the "for (ClasspathLocation annotationLocation : > this.allLocationsForEEA) " loop. allLocationsForEEA is not null when called > from > > > Daemon Thread [Java indexing] (Suspended (breakpoint at line 75 in > ClasspathJrtWithReleaseOption)) > ClasspathJrtWithReleaseOption.<init>(String, AccessRuleSet, IPath, > Collection<ClasspathLocation>, String) line: 75 > ClasspathLocation.forJrtSystem(String, AccessRuleSet, IPath, > Collection<ClasspathLocation>, String) line: 163 > JavaSearchNameEnvironment.mapToClassPathLocation(JavaModelManager, > PackageFragmentRoot, IModuleDescription, Collection<ClasspathLocation>) > line: 267 > JavaSearchNameEnvironment.computeClasspathLocations(JavaProject) line: 200 Thanks, so this is the bug location (I was looking at the normal NameEnvironment, not JavaSearchNameEnvironment). Here, passing locations into mapToClassPathLocation() should only happen when the option is set.
@Jörg, my previous analysis was done looking at an out-dated workspace. Looking at state in master I realize that this has already been improved via bug 578661 (4.23 M3). Could you please retest with a recent build? If that solves the problem for you, this will indicate that the new option allows avoiding the performance penalty. Still I will keep this bug open to see if things can be improved even when the option is enabled. More specifically, I will reconsider whether their's any use case where JavaSearchNameEnvironment needs any of this voodoo.
(In reply to Stephan Herrmann from comment #11) > Still I will keep this bug open to see if things can be improved even when > the option is enabled. More specifically, I will reconsider whether their's > any use case where JavaSearchNameEnvironment needs any of this voodoo. So, what actually is JavaSearchNameEnvironment used for? 3 clients: 1. CompletionEngine (corner case: resolve signature for ctor proposal) 2. SourceIndexer 3. MatchLocator (1) is not interested in any annotations. Feeding external annotations into (2) and (3) would make sense only if we'd want a search for @NonNull et al to find locations super-imposed using EEA. This doesn't sound like a useful scenario. Nor would it be straight forward to implement this, as naturally, an external annotation doesn't yield a source location to match against. So, while the first round of bug 574603 and bug 578661 was targeting completely integrating the new feature into all implementations of INameEnvironment, I'm planning now to remove it completely from JavaSearchNameEnvironment. At that point I will also check if all calls to ClasspathEntry.getExternalAnnotationPath() within JavaSearchNameEnvironment can be removed, too, without loss of functionality.
(In reply to Stephan Herrmann from comment #11) > Could you please retest with a recent build? With jdt master/HEAD i can not reproduce the problem with decorateWithExternalAnnotations anymore.
(In reply to Stephan Herrmann from comment #12) > So, while the first round of bug 574603 and bug 578661 was targeting > completely integrating the new feature into all implementations of > INameEnvironment, I'm planning now to remove it completely from > JavaSearchNameEnvironment. At that point I will also check if all calls to > ClasspathEntry.getExternalAnnotationPath() within JavaSearchNameEnvironment > can be removed, too, without loss of functionality. Stephan, do you still plan to work on this?
(In reply to Andrey Loskutov from comment #14) > (In reply to Stephan Herrmann from comment #12) > > So, while the first round of bug 574603 and bug 578661 was targeting > > completely integrating the new feature into all implementations of > > INameEnvironment, I'm planning now to remove it completely from > > JavaSearchNameEnvironment. At that point I will also check if all calls to > > ClasspathEntry.getExternalAnnotationPath() within JavaSearchNameEnvironment > > can be removed, too, without loss of functionality. > > Stephan, do you still plan to work on this? Yes, thanks for adjusting the milestone.
(In reply to Stephan Herrmann from comment #12) > (In reply to Stephan Herrmann from comment #11) > > Still I will keep this bug open to see if things can be improved even when > > the option is enabled. More specifically, I will reconsider whether there's > > any use case where JavaSearchNameEnvironment needs any of this voodoo. > > So, what actually is JavaSearchNameEnvironment used for? 3 clients: > 1. CompletionEngine (corner case: resolve signature for ctor proposal) > 2. SourceIndexer > 3. MatchLocator > > (1) is not interested in any annotations. > > Feeding external annotations into (2) and (3) would make sense only if we'd > want a search for @NonNull et al to find locations super-imposed using EEA. > This doesn't sound like a useful scenario. Nor would it be straight forward > to implement this, as naturally, an external annotation doesn't yield a > source location to match against. > > So, while the first round of bug 574603 and bug 578661 was targeting > completely integrating the new feature into all implementations of > INameEnvironment, I'm planning now to remove it completely from > JavaSearchNameEnvironment. At that point I will also check if all calls to > ClasspathEntry.getExternalAnnotationPath() within JavaSearchNameEnvironment > can be removed, too, without loss of functionality. Done via https://github.com/eclipse-jdt/eclipse.jdt.core/pull/83
