Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 342701 - [performance] Regression in OpenMultipleEditorTest#testOpenMultipleEditors:java[closeAll]()]
Summary: [performance] Regression in OpenMultipleEditorTest#testOpenMultipleEditors:ja...
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Text (show other bugs)
Version: 3.7   Edit
Hardware: PC Windows XP
: P2 normal (vote)
Target Milestone: 3.7   Edit
Assignee: JDT-Text-Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords: performance
Depends on:
Blocks: 313891
  Show dependency tree
 
Reported: 2011-04-13 07:08 EDT by Satyam Kandula CLA
Modified: 2012-09-27 11:50 EDT (History)
4 users (show)

See Also:


Attachments
Profiler screenshot (99.81 KB, image/png)
2011-04-18 17:02 EDT, Oleg Besedin CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Satyam Kandula CLA 2011-04-13 07:08:11 EDT
There is around 10% regression in OpenMultipleEditorTest#testOpenMultipleEditors:java[closeAll]() and OpenMultipleEditorTest#testOpenMultipleEditors:java[closeEach]() on both Linux and Windows. 
This seems to have started from N20110324-2000.
Comment 1 Oleg Besedin CLA 2011-04-15 15:28:47 EDT
So far I am unable to reproduce those results. 

For my computer those are representative numbers:

3.7M6 (before)

  Elapsed Process:       20.83s (no confidence)                       
  Elapsed Process:       19.93s (no confidence)                       
  Elapsed Process:       19.63s (no confidence)                       

  CPU Time:              27.94s (no confidence)                       
  CPU Time:                 28s (no confidence)                       
  CPU Time:               26.4s (no confidence)                       
-----


Current

  Elapsed Process:       18.57s (no confidence)                       
  Elapsed Process:       19.34s (no confidence)                       
  Elapsed Process:       21.24s (no confidence)                       

  CPU Time:              26.64s (no confidence)                       
  CPU Time:              24.63s (no confidence)                       
  CPU Time:              26.96s (no confidence)
Comment 2 Oleg Besedin CLA 2011-04-15 15:30:10 EDT
Before: average 27.4 sec; difference between runs is about 2 sec

Current: average 26.0 sec; difference between runs is about 2 sec.
Comment 3 Dani Megert CLA 2011-04-18 05:37:46 EDT
> Elapsed Process:       20.83s (no confidence)

The "(no confidence)" usually means that the test only runs once or by far not enough times to say anything useful.
Comment 4 Oleg Besedin CLA 2011-04-18 14:14:50 EDT
I was able to duplicate the issue using different setup; details to follow in a bit :-).
Comment 5 Oleg Besedin CLA 2011-04-18 16:53:52 EDT
(In reply to comment #3)
> The "(no confidence)" usually means that the test only runs once or by far not
> enough times to say anything useful.

The test itself consists of opening 99 files. This is done one time and then results are stored in the performance database. 

As far as our performance tests go, this one actually represents one of the better designs:
- it measures something useful to an end user;
- the measured time interval is big enough to smooth random fluctuations

In a better world we'd run the 99-files-opening test a few times (30+) and then process it with some statistics more appropriate then Gaussian distribution, but overall, as far as our performance tests go, this test is fine.
Comment 6 Oleg Besedin CLA 2011-04-18 16:57:40 EDT
The issue is a result of the change made in the bug 285957. The DefaultTextDoubleClickStrategy class now creates BreakIterator’s when constructed.  

On my computer this results in about 10% increase in the time needed to open an empty Java file.

On my computer difference in test with DefaultTextDoubleClickStrategy v.18 vs v.17:
CPU: Median : 9%		Mean : 9%
Elapsed: Median : 10%		Mean : 11%
Comment 7 Oleg Besedin CLA 2011-04-18 17:02:35 EDT
Created attachment 193531 [details]
Profiler screenshot

Screenshot of profiler with relevant portion of the call stack.

[Initially (comment 1) I was not able to reproduce the problem as I used "Frankensten" builds based on M6 and current I-builds. In those builds I did change UI code to different versions but not JDT code. If we had at least I-builds stored for a year that would saved a lot of time that several people put into reproducing this issue.]
Comment 8 Dani Megert CLA 2011-04-19 03:44:03 EDT
(In reply to comment #6)
> The issue is a result of the change made in the bug 285957. The
> DefaultTextDoubleClickStrategy class now creates BreakIterator’s when
> constructed.  

Good catch Oleg! I'm not yet sure though that it accounts for all the regression. The creation of two break iterators newly happens when the editor is opened but that should not be a big deal - at least in general: creating 100'000 times the two break iterators uses less than 200ms on my machine. Now, on the very first invocation we might have to load the class from the JAR and maybe even load the ICU bundle. Since the test itself only runs once the class and plug-in loading could be distributed over the 99 editor opening and result in decreased performance reported by the test.

I have changed the DefaultTextDoubleClickStrategy to only create the break iterators when needed so that the editor opening scenario is no longer affected. This is in HEAD and builds >= I20110419-0800.

Oleg, can you check on your computer whether this indeed fixes the performance tests or whether other issues still exists? Thanks!

>If we had at least
>I-builds stored for a year that would saved a lot of time that several people
>put into reproducing this issue.
I can deliver if you need one (but only linux-gtk and win32).
Comment 9 Dani Megert CLA 2011-04-21 02:06:22 EDT
http://download.eclipse.org/eclipse/downloads/drops/I20110419-1004/performance/org.eclipse.ui.php?fp_type=0 shows that most of the regression is gone.

I mark this as fixed but it would nice to see the numbers from Oleg's machine to compare.
Comment 10 Oleg Besedin CLA 2011-04-26 09:44:01 EDT
Verified in I20110424-2000.

Note that calculations displayed on graphs for some performance tests are currently invalid and raw data has to be used, see bug 343531.