Bug 224351 - Regression in performance test FullSourceWorkspaceModelTests#testFindType
Summary: Regression in performance test FullSourceWorkspaceModelTests#testFindType
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.4   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: 3.4 M7   Edit
Assignee: Jerome Lanneluc CLA
QA Contact:
URL:
Whiteboard:
Keywords: performance, test
Depends on:
Blocks:
 
Reported: 2008-03-27 08:42 EDT by Frederic Fusier CLA
Modified: 2008-04-30 02:55 EDT (History)
1 user (show)

See Also:

Attachments
Proposed fix (1.60 KB, patch)
2008-04-02 09:10 EDT, Jerome Lanneluc CLA 		no flags Details | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Frederic Fusier CLA 2008-03-27 08:42:31 EDT 
Our local performance tests show a strong regression (over -40%) on FullSourceWorkspaceModelTests#testFindType since v_845.

Here are the numbers I got for this test:
M3	14.52%
M4	16.08%
M5	16.52%
v_839	16.21%
v_840	14.69%
v_841	15.89%
v_842	16.04%
v_843	14.90%
v_845	-44.42%
p_846	-47.68%

Note that:
1) All other Model tests are OK
2) I hadn't get "official" results for v_844, but it seems to be OK after
   running it test several times in my workspace
3) p_846 was tests results using JDT/Core v_846 on top of a I20080305-1100 build
   This result confirms the regression...
Comment 1 Frederic Fusier CLA 2008-03-27 11:02:58 EDT 
The regression comes from the last change done to fix bug 222339 in PackageFragmentRoot.internalKind() method.

The first released method implementation was:

int internalKind() throws JavaModelException {
	JavaModelManager manager = JavaModelManager.getJavaModelManager();
	PackageFragmentRootInfo info = (PackageFragmentRootInfo) manager.peekAtInfo(this);
	if (info == null) {
		// default to regular getKind()
		return getKind();
	}
	return info.getRootKind();
}

And this was changed to:
int internalKind() throws JavaModelException {
	JavaModelManager manager = JavaModelManager.getJavaModelManager();
	PackageFragmentRootInfo info = (PackageFragmentRootInfo) manager.peekAtInfo(this);
	if (info == null) {
		info = (PackageFragmentRootInfo) openWhenClosed(createElementInfo(), null);
	}
	return info.getRootKind();
}

Revert this change seems to bring back the previous performance results.
Comment 2 Jerome Lanneluc CLA 2008-04-02 09:10:17 EDT 
Created attachment 94527 [details]
Proposed fix

The version with getKind() was faster because it is optimized out in JarPackageFragmentRoot and it always returns K_BINARY. The fix consists in doing the same optimization for internalKind().
Comment 3 Jerome Lanneluc CLA 2008-04-02 10:27:19 EDT 
Fix released for 3.4M7
Comment 4 Maxime Daniel CLA 2008-04-29 09:19:21 EDT 
The regression is visible on the download site (see detailed performance results for 3.4 M6 - from the download page for 3.4 M6, follow 'View the performance results for the current build.', 'org.eclipse.jdt.core*', scroll down to 'FullSourceWorkspaceModelTests#testFindType()'). Frédéric had access to the results for I20080427-2000, and the regression is gone.

Hence,

Verified for 3.4 M7 using build I20080427-2000.
Comment 5 Maxime Daniel CLA 2008-04-30 02:55:32 EDT 
Bright green confirmed on the download page for build I20080427-2000.