Bug 263246 - Optimize .api_description generation on export
Summary: Optimize .api_description generation on export
Status: VERIFIED FIXED
Alias: None
Product: PDE
Classification: Eclipse Project
Component: API Tools (show other bugs)
Version: 3.5   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: 3.5 M6   Edit
Assignee: Michael Rennie CLA
QA Contact:
URL:
Whiteboard:
Keywords: performance
Depends on:
Blocks:
 
Reported: 2009-02-02 11:04 EST by Andrew Niefer CLA
Modified: 2013-04-23 02:50 EDT (History)
3 users (show)

See Also:
aniefer: review+
Michael_Rennie: review+


Attachments
proposed fix (11.18 KB, patch)
2009-02-13 13:06 EST, Michael Rennie CLA
no flags Details | Diff
updated patch (16.67 KB, patch)
2009-02-17 10:34 EST, Michael Rennie CLA
no flags Details | Diff
Complement (2.17 KB, patch)
2009-02-23 14:38 EST, Olivier Thomann CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Niefer CLA 2009-02-02 11:04:01 EST
See bug 258822 comment #4.  

In the particular case of exporting from the workspace with the new option to use the class files compiled in the workspace, .api_description adds significant time to the export.

Since we are using .class files from the workspace instead of recompiling, the time spent generating the api_description could in theory be spent before export.

We should investigate reusing the results of the API Analysis Builder during export.
Comment 1 Olivier Thomann CLA 2009-02-13 11:03:06 EST
I think we should only scan compilation units inside API packages.
Comment 2 Michael Rennie CLA 2009-02-13 13:06:00 EST
Created attachment 125665 [details]
proposed fix

looking at the task code I discovered that we scan every single .class file in the project specified in the task and also re-resolve the java ASTParser options every time we scan a collected .class file.

Testing the fix against jdt.core the class files scanned went from 1181 down to 313 (only API types) and the average time to build the api description for jdt.core lowered to ~500ms
Comment 3 Michael Rennie CLA 2009-02-13 13:11:29 EST
sample buildfile to test with:

<?xml version="1.0" encoding="UTF-8"?>
<project name="api_use_reporting" default="run" basedir=".">
	<property name="pname" value="org.eclipse.jdt.core_3.5.0.v_927"/>
	<property name="project_location" value="C:\Eclipse\workspaces\workspace\org.eclipse.jdt.core"/>
	<property name="binary_locations" value="C:\Eclipse\workspaces\workspace\org.eclipse.jdt.core\bin;C:\Eclipse\workspaces\workspace\org.eclipse.jdt.core\antbin"/>
	<property name="target_location" value="C:\Eclipse\exported\org.eclipse.jdt.core"/>
	
	<target name="run">
	    <apitooling.apigeneration
	    	projectname="${pname}"
	    	project="${project_location}"
	    	binary="${binary_locations}"
	    	target="${target_location}"
	    />
	  </target>

</project>
Comment 4 Michael Rennie CLA 2009-02-13 14:27:50 EST
Andrew and Olivier could you please test the patch? If this patch is sufficient I will copy the same fix over to the deprecated FileGenerator task as well.
Comment 5 Olivier Thomann CLA 2009-02-13 21:35:41 EST
I would be tempted to record the public packages instead of the internal ones. In general there is more internal packages than public packages. Beside this, the patch looks ok. I'll make some benchmarks next week.
Comment 6 Michael Rennie CLA 2009-02-17 10:34:56 EST
Created attachment 125898 [details]
updated patch

You are correct Olivier, we could save an additional bit of memory collecting the API packages and not the internal ones. This patch updates to collecting the API packages and also provides the performance fix for the deprecated generator task.
Comment 7 Michael Rennie CLA 2009-02-20 08:53:14 EST
Applied patch to HEAD.


will wait to mark fixed until we have a few more sample benchmarks
Comment 8 Olivier Thomann CLA 2009-02-23 14:24:12 EST
-1 for now and it looks like we are ending up with empty .api_description files.
I am investigating.
Comment 9 Olivier Thomann CLA 2009-02-23 14:38:56 EST
Created attachment 126494 [details]
Complement

This is required to properly recurse into subfolders.
Comment 10 Olivier Thomann CLA 2009-02-23 14:39:35 EST
Michael, please review.
Comment 11 Olivier Thomann CLA 2009-02-23 23:53:31 EST
I released another patch for this issue.
The directives being null is not enough for API packages. We actually have to check if x-internals is set to true or if an x-friend is set.
Directives can also contain "mandatory", ...

Michael, please verify last change.
Comment 12 Michael Rennie CLA 2009-03-06 11:26:26 EST
verified.
Comment 13 Andrew Niefer CLA 2009-03-06 12:05:58 EST
Looks good to me.  From bug 258822,
I was seeing approx 40 seconds for api descriptions for a total of about 50 seconds for an export.  Now I see the api portion of the export being complete at around the 7-10 second mark and the entire export being finished around 20 seconds.  (This is reusing workspace class files, and with code that is not yet released for pde.ui).
Comment 14 Michael Rennie CLA 2009-03-11 10:44:24 EDT
marking verified