Community
Participate
Working Groups
We have a PDE environment with the following characteristics: 65 plugins with source 210+ source locations declared in our target PDE validations enabled against the plugin.xml files We find that on 3.3.x (We have 3.3.0 and 3.3.1 based clients), the performance of PDE launch is debilitating -- to the point where at 61% it stalls out; constantly working, but never exiting (to the tune of 20+ minutes before developers become impatient). After some analysis, I have traced the problem to a couple of causes: For every project build, there is a marker change event on the plugin.xml (from the PDE validator). A PDE resource change listener (WorkspaceModelManager) kicks off. The resource change listener creates a bundle state delta object and passes it to the PluginModelManager.updateAffectedEntries(). The state delta appears to have an entry for every known plugin (815 in my environment). For each of these 800 some odd entries, it iterates linearly to determine which of these has an underlying resource. For each one that has an underlying resource, it generates a new PDE Classpath Container (RequiredPluginsClasspathContainer). Each of these newly created Classpath Containers must re-compute its resolved dependencies. For each of these dependencies (roughly 8-20 per plugin in our workspace), it appears to generate a search query against all known source locations (210+). For each of these search queries, the same entry (e.g. org.acme.myplugin/src.zip many times over for each plugin that depends on org.acme.myplugin) is appended and linearly searched against the list of known source locations. For each of these possibilities (location + append(source path)), it generates a java.io.File.exists() query to the disk. From a rough sketch of what I've observed, it appears to be on the order of magnitude of O(# of plugin.xml files * # source locations * # of dependencies * # of plugin projects) work! For our environment, that's roughly 65*210*~15*65 hits to the disk to check for an exists condition, just because the PDE validator adjusted the markers on a plugin.xml file. I have prepared a patch that we have begun to use locally against 3.3.1-based clients. The patch includes the following enhancements: (1) Smarter Resource Change Lisener (WorkspaceModelManager) (a) Checks for derived resources; this was being invoked for every compiled or generated artifact from each project (b) Checks for changes against the project description; these changes didn't seem to include any plugin.xml/manifest.mf updates, only .project updates (c) Checks for marker changes on files; avoids excessive "raize and re-build" of the classpath container (d) Uses a switch on the type of resource to avoid excessive instanceof checks (2) Smarter SourceLocationManager (a) Uses a rudimentary caching algorithm to remember where the source path was previously identified. As source locations are declared in extension points, this information is relatively static. (3) Added Debug statement to RequiredPluginsClasspathContainer to demonstrate how many times the classpath is being re-computed even for minor resource deltas (e.g. markers). This is an extremely severe performance problem, and should be considered for release to 3.3.2 as well as 3.4. Thank you for your time and consideration.
Created attachment 80020 [details] A patch to resolve the issue generated against R3_3_maintenance.
There is a bug with the patch that breaks the source attachment feature. The caching scheme for the SourceLocationManager will need to be improved.
Created attachment 80038 [details] Corrects the bug in the simply caching scheme.
I'm not on the PDE team, but thanks for your analysis and possible fix. Your patch is against a PDE-UI component. Moving over to there.
Michael, thanks a lot for the patch! I will go ahead and look into this right away. I understand the severity of the problem so I will work as fast as I can. With that in mind, this patch touches some extremely sensitive PDE code and has to be well analyzed before committing. For instance, I believe the caching in the SourceLocationManager will continue to hold old locations even after the target platform has been changed. This patch is a great head start and I will make any necessary modifications.
Applied change to WorkspaceModelManager to HEAD. Need to work with Michael to find a caching scheme that shows good improvement and scales well.
Had to revert change to WorkspaceModelManager. It seems when a project is checked out of CVS, we get a resouced changed event. The event appears to be a description change on the project. The modification discards this event which will cause the PDE classpath container to not be refreshed. This will cause many CVS projects to no have any Required Plug-ins Classpath Container. See bug 206610 for more details on the side effect.
Did you have to revert the entire change or the single line that ignores changes to the project description? The delta must contain an add event for the plugin.xml/Manifest.mf files. Would a more appropriate fix be to expand the implementation of the isContentChanged() method that takes the file to include a check for : delta.getKind() == ADDED || REMOVED || (CHANGED && (delta.getFlags() & CONTENT) != 0) ?
(In reply to comment #8) > Did you have to revert the entire change or the single line that ignores > changes to the project description? Since we were holding up a rebuild, I went ahead and reverted everything. But I was hoping we might be able to come up with something. I figured even if we weren't I would still go back and remove all those instanceof's :) I will try your suggestion and let you know if it solves the problem.
Did you add a test case that covers bug 206610, so that you don't break it again?
(In reply to comment #10) > Did you add a test case that covers bug 206610, so that you don't break it > again? > See bug 206900.
Any word if the modification suggested in comment 8 resolved the problem? Leaving the project description as a trigger will mean that the classpath will be recomputed twice for each new project (once for ADD/plugin.xml and one for ADD/getFlags & ProjectDescription).
(In reply to comment #12) > Any word if the modification suggested in comment 8 resolved the problem? Michael, sorry for the late reply, I have been running around a lot. The problem is that we are "exiting" when we hit isDescriptionChange() when the element in the project. By doing this, we don't probe any further to get the folders/files which have been changed. When we get the second notification that is the changed project description, it seems to encompass the MANIFEST.MF file being added to the project from CVS. I am not sure how we can get around not processing this notification as this event is important to produce the proper buildpath. I am definitely open to ideas/suggestions, I am far from a builder framework expert :)
Is this related to Bug 206887?
*** Bug 206887 has been marked as a duplicate of this bug. ***
Created attachment 81199 [details] resource listener changes for 3.3 stream Smarter resource listener based on Michael's original patch and his suggestions, targeted for 3.3 Maintenance stream.
Created attachment 81200 [details] smarter resource listener for 3.4 Smarter resource listener based on Michael's original patch and his suggestions, target for 3.4 HEAD.
The updates to the resource listener in WorkspaceModelManager have been included in both HEAD and the 3.3 Maintenance stream. One note, had to exclude skipping resource notifications that contains IProject DESCRIPTION changes. When we skip these notifications, we run into problems with the PDE classpath container not working correctly upon CVS checkout for certain projects. As described in bug 206887, we should only respond to IResourceDelta.ADDED, IResourceDelta.REMOVED, and IResourceDelta.CHANGED (only when the content changes) events. This should eliminate any work being done for IResourceDelta.SYNC and IResourceDelta.MARKER events.
Marking this bug as fixed with the changes dropped into 3.3.2 and 3.4. If the performance has not improved, please reopen bug.
I was looking to see if this fix was in the latest 3.3.2 build and wasn't sure if it was since the version of the PDE/Core plug-in was still 3.3.1. Based on the qualifier, it looks like the fix is available but it would be good for someone to up the version of the plug-in to 3.3.2 just to avoid confusion.
Yes, that could cause some confusion :) Will be fixed in the M-build for 11/21. The fix should have been included since the 11/2 M-Build.
*** Bug 214400 has been marked as a duplicate of this bug. ***
