Bug 280464 - API tooling performance must be improved
Summary: API tooling performance must be improved
Status: VERIFIED FIXED
Alias: None
Product: PDE
Classification: Eclipse Project
Component: API Tools (show other bugs)
Version: 3.5   Edit
Hardware: All All
: P3 major (vote)
Target Milestone: 3.6 M5   Edit
Assignee: Michael Rennie CLA
QA Contact:
URL:
Whiteboard:
Keywords: performance
Depends on: 230416 231936 233643 244198 296258 297604
Blocks:
  Show dependency tree
 
Reported: 2009-06-16 11:26 EDT by Michael Rennie CLA
Modified: 2010-03-01 09:57 EST (History)
5 users (show)

See Also:


Attachments
fix cut (227.90 KB, patch)
2009-11-24 16:38 EST, Michael Rennie CLA
no flags Details | Diff
updated patch (236.77 KB, patch)
2009-11-27 12:56 EST, Michael Rennie CLA
no flags Details | Diff
patch (7.00 KB, patch)
2009-12-10 16:58 EST, Darin Wright CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Rennie CLA 2009-06-16 11:26:15 EDT
I20090611-1540 (and previous)

It has been experienced that the first build - upon starting the workspace - takes up to a few minutes to complete regardless of the kind of change actually made. 

Part of the reason for this is that we have to re-build the API descriptions - that being said we have to find a way to improve this process and lessen the amount of work done on the first build.

One way we could potentially improve this is to make our build context just a bit smarter for our usage scans, where we end up loading / visiting API descriptions for dependent bundles of the one currently being built, even if the change(s) made does not effect them.
Comment 1 Dani Megert CLA 2009-06-17 09:06:34 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)
Comment 2 Dani Megert CLA 2009-06-17 09:07:15 EDT
If this can't be made faster then it must be changed so that it is non-blocking.
Comment 3 Darin Wright CLA 2009-07-07 14:54:30 EDT
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.
Comment 4 Darin Wright CLA 2009-07-08 09:42:20 EDT
Note, the problem with the first time build hit is covered by bug 282739.
Comment 5 John Arthorne CLA 2009-09-23 16:55:54 EDT
I opened a specific bug about performance on manifest changes that is related to this: bug 290324.
Comment 6 Michael Rennie CLA 2009-11-17 14:50:27 EST
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.
Comment 7 Michael Rennie CLA 2009-11-20 14:30:43 EST
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.
Comment 8 Darin Wright CLA 2009-11-20 14:49:05 EST
(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).
Comment 9 Michael Rennie CLA 2009-11-24 16:38:56 EST
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.
Comment 10 Michael Rennie CLA 2009-11-24 16:42:11 EST
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])
Comment 11 Michael Rennie CLA 2009-11-26 11:53:02 EST
Adding bug 296258 as a dependent. It prevents the patch from working properly in the import / open / new project smoke tests
Comment 12 Michael Rennie CLA 2009-11-27 12:56:55 EST
Created attachment 153267 [details]
updated patch

updated fix that includes a fix to the PDE dependency manager and some extra synchronization for component initialization
Comment 13 Darin Wright CLA 2009-12-10 16:58:37 EST
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.
Comment 14 Darin Wright CLA 2009-12-10 17:16:12 EST
(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.
Comment 15 Darin Wright CLA 2010-01-12 09:34:27 EST
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.