Summary: | API tooling performance must be improved | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | [Eclipse Project] PDE | Reporter: | Michael Rennie <Michael_Rennie> | ||||||||
Component: | API Tools | Assignee: | Michael Rennie <Michael_Rennie> | ||||||||
Status: | VERIFIED FIXED | QA Contact: | |||||||||
Severity: | major | ||||||||||
Priority: | P3 | CC: | daniel_megert, darin.eclipse, markus.kell.r, Olivier_Thomann, thatnitind | ||||||||
Version: | 3.5 | Keywords: | performance | ||||||||
Target Milestone: | 3.6 M5 | ||||||||||
Hardware: | All | ||||||||||
OS: | All | ||||||||||
Whiteboard: | |||||||||||
Bug Depends on: | 230416, 231936, 233643, 244198, 296258, 297604 | ||||||||||
Bug Blocks: | |||||||||||
Attachments: |
|
Description
Michael Rennie
2009-06-16 11:26:15 EDT
I also thought it's only the first time but today I waited several minutes for several times. Here's a stack dump of the CPU waster: "Worker-235" prio=6 tid=0x05c29400 nid=0x1c68 runnable [0x080de000..0x080dfa14] java.lang.Thread.State: RUNNABLE at java.io.FileInputStream.open(Native Method) at java.io.FileInputStream.<init>(FileInputStream.java:106) at org.eclipse.core.internal.filesystem.local.LocalFile.openInputStream(LocalFile.java:357) at org.eclipse.core.internal.localstore.FileSystemResourceManager.read(FileSystemResourceManager.java:667) at org.eclipse.core.internal.resources.File.getContents(File.java:288) at org.eclipse.core.internal.resources.File.getContents(File.java:277) at org.eclipse.pde.api.tools.internal.model.ResourceApiTypeRoot.getInputStream(ResourceApiTypeRoot.java:48) at org.eclipse.pde.api.tools.internal.model.AbstractApiTypeRoot.getContents(AbstractApiTypeRoot.java:44) at org.eclipse.pde.api.tools.internal.model.AbstractApiTypeRoot.getStructure(AbstractApiTypeRoot.java:71) at org.eclipse.pde.api.tools.internal.builder.ReferenceExtractor.visitInnerClass(ReferenceExtractor.java:1116) at org.objectweb.asm.ClassReader.accept(Unknown Source) at org.objectweb.asm.ClassReader.accept(Unknown Source) at org.eclipse.pde.api.tools.internal.model.ApiType.extractReferences(ApiType.java:161) at org.eclipse.pde.api.tools.internal.builder.ReferenceAnalyzer$Visitor.visit(ReferenceAnalyzer.java:93) at org.eclipse.pde.api.tools.internal.model.FolderApiTypeContainer.doVisit(FolderApiTypeContainer.java:103) at org.eclipse.pde.api.tools.internal.model.FolderApiTypeContainer.doVisit(FolderApiTypeContainer.java:122) at org.eclipse.pde.api.tools.internal.model.FolderApiTypeContainer.doVisit(FolderApiTypeContainer.java:122) at org.eclipse.pde.api.tools.internal.model.FolderApiTypeContainer.doVisit(FolderApiTypeContainer.java:122) at org.eclipse.pde.api.tools.internal.model.FolderApiTypeContainer.doVisit(FolderApiTypeContainer.java:122) at org.eclipse.pde.api.tools.internal.model.FolderApiTypeContainer.doVisit(FolderApiTypeContainer.java:122) at org.eclipse.pde.api.tools.internal.model.FolderApiTypeContainer.doVisit(FolderApiTypeContainer.java:122) at org.eclipse.pde.api.tools.internal.model.FolderApiTypeContainer.accept(FolderApiTypeContainer.java:58) at org.eclipse.pde.api.tools.internal.model.AbstractApiTypeContainer.accept(AbstractApiTypeContainer.java:56) at org.eclipse.pde.api.tools.internal.model.AbstractApiComponent.accept(AbstractApiComponent.java:58) at org.eclipse.pde.api.tools.internal.builder.ReferenceAnalyzer.extractReferences(ReferenceAnalyzer.java:261) at org.eclipse.pde.api.tools.internal.builder.ReferenceAnalyzer.analyze(ReferenceAnalyzer.java:166) at org.eclipse.pde.api.tools.internal.builder.ReferenceAnalyzer.analyze(ReferenceAnalyzer.java:288) at org.eclipse.pde.api.tools.internal.builder.BaseApiAnalyzer.checkApiUsage(BaseApiAnalyzer.java:833) at org.eclipse.pde.api.tools.internal.builder.BaseApiAnalyzer.analyzeComponent(BaseApiAnalyzer.java:261) at org.eclipse.pde.api.tools.internal.builder.ApiAnalysisBuilder.buildAll(ApiAnalysisBuilder.java:383) at org.eclipse.pde.api.tools.internal.builder.ApiAnalysisBuilder.build(ApiAnalysisBuilder.java:283) at org.eclipse.core.internal.events.BuildManager$2.run(BuildManager.java:627) at org.eclipse.core.runtime.SafeRunner.run(SafeRunner.java:42) at org.eclipse.core.internal.events.BuildManager.basicBuild(BuildManager.java:170) at org.eclipse.core.internal.events.BuildManager.basicBuild(BuildManager.java:201) at org.eclipse.core.internal.events.BuildManager$1.run(BuildManager.java:253) at org.eclipse.core.runtime.SafeRunner.run(SafeRunner.java:42) at org.eclipse.core.internal.events.BuildManager.basicBuild(BuildManager.java:256) at org.eclipse.core.internal.events.BuildManager.basicBuildLoop(BuildManager.java:309) at org.eclipse.core.internal.events.BuildManager.build(BuildManager.java:341) at org.eclipse.core.internal.events.AutoBuildJob.doBuild(AutoBuildJob.java:140) at org.eclipse.core.internal.events.AutoBuildJob.run(AutoBuildJob.java:238) at org.eclipse.core.internal.jobs.Worker.run(Worker.java:55) If this can't be made faster then it must be changed so that it is non-blocking. Looking at Dani's stack trace, the builder has been asked to perform an incremental build, but falls back to a full build as the manifest or API filters have changed. Note, the problem with the first time build hit is covered by bug 282739. I opened a specific bug about performance on manifest changes that is related to this: bug 290324. Our ApiModelFactory could be improved to not initialize ApiComponents when they are first created. As it is now, when a new component handle is created we parse the manifest, do some manifest target weaving, do some state computation and finally compact the manifest. This has a major effect on the first build time and memory usage as the restoration of a baseline (workspace and default) causes all of the their saved components to be reloaded when the baseline is opened (even if the component(s) will not be used in the current build). If for example you are doing an incremental build of something in debug.core, we will load and initialize unrelated component handles for no reason. Other things to think about: 1. Investigate a hasChanged() (or the like) method for API descriptions, this would allow us to know if certain aspects of builds should be no-ops - for example if a type changes and there was no API description change, then we would not need to run use scans on it or its dependents 2. Investigate using the PDE model for our workspace baseline. This could potentially give us a win in memory and time as we would not have to create / re-create any bundle model objects and the updating would come for free from PDE. 3. Investigate adding the notion of IOpenable to all of our model objects that can be "opened" and use that to delay any loading, model work until asked for. As it is now, 98% of our model objects are completely loaded when the simplest attribute is asked for. This is not awesome. (In reply to comment #7) > Other things to think about: > 1. Investigate a hasChanged() (or the like) method for API descriptions, this > would allow us to know if certain aspects of builds should be no-ops - for > example if a type changes and there was no API description change, then we > would not need to run use scans on it or its dependents Clarification: When a type changes, we still need to analyze it (because it's use of other API may have changed). However, we only need to analyze its dependents if its API description has changed (it looks like bug 273295 was opened for this reason during 3.5). Created attachment 153017 [details]
fix cut
This first patch makes two major changes:
1. it leverages the PDE state for the workspace baseline - which has now become its own specialization of a baseline. This has a huge effect on perf and memory, as we now have no work to do with our own state management, and we do not fill the workspace baseline with the entire contents of the target platform anymore.
2. it delays the initialization of API components until they are actually asked for something - previously this was done just to create a handle to an API component. This an effect on perf and memory as we do no work to just get a handle, and we do not end up caching anything until it asked for.
All of the smoke tests pass and there are only 2 test failures - both of which traditionally showed up as intermittent failures.
Up next is investigating using PDE state model deltas to completely avoid disposing the workspace baseline after each build.
Some initial runs of the first patch (on my thinkpad): Scenario 'FullSourceBuildTests#testFullBuild()' (average over 15 samples): Elapsed Process: 8.21s (95% in [7.98s, 8.44s]) Scenario 'FullSourceBuildTests#testCleanFullBuild()' (average over 15 samples): Elapsed Process: 11.83s (95% in [11.68s, 11.98s]) Scenario 'IncrementalBuildTests#testIncrementalBuildAll()' (average over 500 samples): Elapsed Process: 354ms (95% in [345ms, 363ms]) Adding bug 296258 as a dependent. It prevents the patch from working properly in the import / open / new project smoke tests Created attachment 153267 [details]
updated patch
updated fix that includes a fix to the PDE dependency manager and some extra synchronization for component initialization
Created attachment 154275 [details]
patch
I noticed that the TagScanner ends up scanning internal types for API descriptions. This is triggered by some compatibility checks were internal types are super types of API classes, etc. We can avoid scanning the types by simply inheriting the visibility of the parent node in this case.
(In reply to comment #13) > Created an attachment (id=154275) [details] > patch > I noticed that the TagScanner ends up scanning internal types for API > descriptions. This is triggered by some compatibility checks were internal > types are super types of API classes, etc. We can avoid scanning the types by > simply inheriting the visibility of the parent node in this case. I see about 25% improvement on the "clean full build" performance test with this. Marking fixed. We have improved performance and reduced memory usage significantly in 3.6. See bug 233643 for a screen shot of performance results. Other specific bugs have been opened where further improvemets can be made. |