Bug 306172 - [perfs] Invalid test duration for FullSourceWorkspaceTypeHierarchyTests#testPerSuperTypes()
Summary: [perfs] Invalid test duration for FullSourceWorkspaceTypeHierarchyTests#testP...
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.6   Edit
Hardware: All All
: P3 normal (vote)
Target Milestone: 3.6 M7   Edit
Assignee: Satyam Kandula CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 286955
  Show dependency tree
 
Reported: 2010-03-17 07:37 EDT by Frederic Fusier CLA
Modified: 2010-05-21 07:01 EDT (History)
1 user (show)

See Also:

Attachments
Proposed patch (2.09 KB, patch)
2010-04-21 02:01 EDT, Satyam Kandula CLA 		frederic_fusier: review-
Details | Diff
Proposed patch (2.02 KB, patch)
2010-04-22 01:28 EDT, Satyam Kandula CLA 		frederic_fusier: iplog+
frederic_fusier: review+
Details | Diff
Proposed patch for perf_35x (2.37 KB, patch)
2010-04-22 01:31 EDT, Satyam Kandula CLA 		frederic_fusier: iplog+
frederic_fusier: review+
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 2010-03-17 07:37:27 EDT 
Verifying 3.6M6 and I201016-0859 perf results, I noticed an invalid duration for FullSourceWorkspaceTypeHierarchyTests#testPerSuperTypes() perf test.

Note also that the test summary is the same than FullSourceWorkspaceTypeHierarchyTests#testPerfAllTypes() and should be fixed as well...
Comment 1 Satyam Kandula CLA 2010-04-21 02:01:08 EDT 
Created attachment 165527 [details]
Proposed patch

Put in a better tag, increased the WARMUP_COUNT and put it in a loop inside the measuring loop. Now, this test takes a decent time of 160ms on my m/c.
Comment 2 Frederic Fusier CLA 2010-04-21 04:39:19 EDT 
Comment on attachment 165527 [details]
Proposed patch

Do not put the test in the fingerprint until we get enough history to see whether it's stable enough.

Also, is there a specific reason to have increased the number of loops for the warmup? The time does not matter for those loops except that it will make the global time to run the test suite longer...

Do you have run this test on top of perf_35x? I would like to have a patch for this to be ready before the Friday's baseline run and also have an idea of its duration to figure out whether we should expect an improvement or a regression...

TIA
Comment 3 Satyam Kandula CLA 2010-04-22 01:28:58 EDT 
Created attachment 165689 [details]
Proposed patch

Removed the finger print and also changed the inner loop to 20.
Comment 4 Satyam Kandula CLA 2010-04-22 01:31:22 EDT 
Created attachment 165690 [details]
Proposed patch for perf_35x 

Similar as the eariler patch for 3.5
Comment 5 Satyam Kandula CLA 2010-04-22 01:42:09 EDT 
Frederic,
The finger print is removed as for your comment. The warmup count is increased, because, otherwise I was seeing lot of variance in the test results. 
Had to increase the inner loop to 20, as I see that 10 was taking lesser time -- probably my laptop was running on the battery then. 
Now, the test takes 140ms on both Head and 3.5 maintenance branch. The fix that I did for which I had included this test was delivered on 3.5.2 also. Hence, the time should be similar and they are as expected :).
Comment 6 Frederic Fusier CLA 2010-04-22 13:06:24 EDT 
(In reply to comment #4)
> Created an attachment (id=165690) [details]
> Proposed patch for perf_35x 
> 
> Similar as the eariler patch for 3.5

Released in perf_35x branch. I've versionned the project as v_963_perf35xc and updated the map file for today's 6pm baseline run...
Comment 7 Frederic Fusier CLA 2010-04-22 13:09:49 EDT 
(In reply to comment #3)
> Created an attachment (id=165689) [details]
> Proposed patch
> 
> Removed the finger print and also changed the inner loop to 20.

Released for 3.6M7 in HEAD stream.
Comment 8 Frederic Fusier CLA 2010-04-28 09:35:32 EDT 
Verified with performance results of I20100425-2000 build.