Bug 188656 - [perfs] 2% regression on some Batch Compiler tests since v_756
Summary: [perfs] 2% regression on some Batch Compiler tests since v_756
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.3   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: 3.3 RC2   Edit
Assignee: Olivier Thomann CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-05-23 10:50 EDT by Frederic Fusier CLA
Modified: 2007-05-25 04:26 EDT (History)
2 users (show)

See Also:
jerome_lanneluc: review+
frederic_fusier: review+

Attachments
Proposed patch (1.58 KB, patch)
2007-05-23 11:47 EDT, Frederic Fusier CLA 		no flags Details | Diff
New patch (2.02 KB, patch)
2007-05-23 12:40 EDT, Olivier Thomann CLA 		no flags Details | Diff
Show Obsolete (1) 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 2007-05-23 10:50:18 EDT 
Since JDT/Core v_756, we observed a regression around 2% for Batch Compiler performance tests. Not all tests are concerned, testCompileJDTCoreProjectJavadoc and testCompileJDTCoreProjectAllWarnings seem to be flat.
Comment 1 Frederic Fusier CLA 2007-05-23 10:54:39 EDT 
Seems to be an impact of fix for bug 185768. Doc comment support is now activated when annotations are processed but as this option is set by default, the doc comment support is now always activated for the batch compiler hence the performance regression in our tests...
Comment 2 Frederic Fusier CLA 2007-05-23 11:47:03 EDT 
Created attachment 68369 [details]
Proposed patch

I post estimated perfs numbers when running local tests will be finished
Comment 3 Olivier Thomann CLA 2007-05-23 12:17:30 EDT 
+1. Process annotation should never be set for compliance < 1.6.
Comment 4 Olivier Thomann CLA 2007-05-23 12:23:03 EDT 
Sorry. I withdraw my +1. I checked too quickly.
If the option -proc:none has been set, we should not reenable the annotation processing.
So we should add a check to see if the option is already disabled. I'll provide a new patch.
Comment 5 Olivier Thomann CLA 2007-05-23 12:40:25 EDT 
Created attachment 68381 [details]
New patch
Comment 6 Olivier Thomann CLA 2007-05-23 12:41:00 EDT 
Frédéric, please review.
Comment 7 Frederic Fusier CLA 2007-05-23 12:51:24 EDT 
(In reply to comment #6)
> Frédéric, please review.
> 
+1
Comment 8 Jerome Lanneluc CLA 2007-05-23 13:07:23 EDT 
The comment should reference both bug 185768 and this bug. Otherwise +1.
Comment 9 Frederic Fusier CLA 2007-05-23 13:14:19 EDT 
I got following numbers while running local tests:

Proj	| jdt.core | jdt.core | jdt.core | jdt.core  | swt
sett.	| No_Warn  | Default  | Javadoc  | All_Warns | Default
--------|----------|----------|----------|-----------|--------
v_755	| 53315	   | 51834    | 54310    | 63771     | 23933
patch	| 53412	   | 51840    | 54246    | 63850     | 23978

These numbers show that we are back to M7 numbers with this patch.
Comment 10 Frederic Fusier CLA 2007-05-23 13:16:39 EDT 
I got a mid-air collision which seemed to have flushed review flags!
I reset them...
Comment 11 Frederic Fusier CLA 2007-05-23 13:17:22 EDT 
Olivier, Jerome, can you set again the review flag, please?
Thanks
Comment 12 Olivier Thomann CLA 2007-05-23 13:48:12 EDT 
I'll release for tonight's build.
Comment 13 Olivier Thomann CLA 2007-05-23 14:03:52 EDT 
Released for 3.3RC2.
Comment 14 Frederic Fusier CLA 2007-05-25 04:26:38 EDT 
Verified for 3.3 RC2 with performance tests results:
http://fullmoon.ottawa.ibm.com/downloads/drops/I20070524-0010/performance/org.eclipse.jdt.core.php?