Bug 373501 - Huge Performance regression in NonInitialTypingTest#testTypeAMethod()
Summary: Huge Performance regression in NonInitialTypingTest#testTypeAMethod()
Status: RESOLVED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Text (show other bugs)
Version: 3.8   Edit
Hardware: PC Windows 7
: P2 normal (vote)
Target Milestone: 3.8 M7   Edit
Assignee: Deepak Azad CLA
QA Contact:
URL:
Whiteboard:
Keywords: performance
Depends on:
Blocks: 348419
  Show dependency tree
 
Reported: 2012-03-07 06:17 EST by Satyam Kandula CLA
Modified: 2012-04-17 10:08 EDT (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Satyam Kandula CLA 2012-03-07 06:17:09 EST
There is a 1000% performance regression in the test NonInitialTypingTest#testTypeAMethod(). This is observed on both Linux and Windows. It seems to have started on 9th February. I am sorry that I missed catching this. :(.
Comment 1 Dani Megert CLA 2012-03-12 05:37:23 EDT
(In reply to comment #0)
> There is a 1000% performance regression in the test
> NonInitialTypingTest#testTypeAMethod(). This is observed on both Linux and
> Windows. It seems to have started on 9th February. I am sorry that I missed
> catching this. :(.

Since when is this?
Comment 2 Satyam Kandula CLA 2012-03-14 01:38:23 EDT
(In reply to comment #1)
> (In reply to comment #0)
> > There is a 1000% performance regression in the test
> > NonInitialTypingTest#testTypeAMethod(). This is observed on both Linux and
> > Windows. It seems to have started on 9th February. I am sorry that I missed
> > catching this. :(.
> 
> Since when is this?
Noticed from 9th February build.
Comment 3 Deepak Azad CLA 2012-04-16 08:39:48 EDT
I can reproduce a significant slowdown on Windows 7

- master
Elapsed Process:         769ms        (95% in [707ms, 830ms])        Measurable effect: 109ms (1.3 SDs) (required sample size for an effect of 5% of mean: 81)
CPU Time:                917ms        (95% in [812ms, 1.02s])        Measurable effect: 185ms (1.3 SDs) (required sample size for an effect of 5% of mean: 164)

- 3.7.1
Elapsed Process:         389ms        (95% in [342ms, 436ms])        Measurable effect: 83ms (1.3 SDs) (required sample size for an effect of 5% of mean: 182)
CPU Time:                556ms        (95% in [414ms, 698ms])        Measurable effect: 250ms (1.3 SDs) (required sample size for an effect of 5% of mean: 812)

Looks to be another one of SWT/Display/events issue :-(

I profiled using Yourkit and the invocation counts of "org.eclipse.jdt.text.tests.performance.EditorTestHelper.runEventQueue(Display)" are the same in both cases, however, the invocation counts of "org.eclipse.swt.widgets.Display.readAndDispatch()" are significantly different. This in turn causes the reconciler etc to run more times which causes the slowdown.

This is what I think currently, continuing to investigate more...
Comment 4 Markus Keller CLA 2012-04-17 06:37:38 EDT
I think the problem is the DefaultCharacterPairMatcher. The test types just in front of the last '}'. Since we now match on the left of the closing bracket, the matching bracket is updated all the time.

In 3.7, we only matched the '}' on the left of the caret, so the DefaultCharacterPairMatcher always finished without doing anything.
Comment 5 Deepak Azad CLA 2012-04-17 06:48:05 EDT
(In reply to comment #4)
Doh! I should have thought of that :-(

I double checked the relevant time stamps
- The code change was made on 2012-02-09 17:02:07 IST (bug 9503 comment 29)
- The test started failing in the nightly build later the same day (http://download.eclipse.org/eclipse/downloads/drops/I20120320-0800/performance/eplnx2/Scenario108.html#CPUT)

I will tweak the test to not type right next to a bracket and see if that fixes the problem.
Comment 6 Deepak Azad CLA 2012-04-17 07:50:47 EDT
(In reply to comment #5)
> I will tweak the test to not type right next to a bracket and see if that fixes
> the problem.
Bracket matching was indeed the problem :-) I have tweaked the test to not type next to a bracket.

http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/?id=e98faed5bb3f143b029c3501093ed500073f6c35

With this change, the performance test numbers and invocation counts in YourKit are similar to 3.7.1.
Comment 7 Markus Keller CLA 2012-04-17 10:03:46 EDT
> I will tweak the test to not type right next to a bracket

I agree with that, but now the test types the method declaration outside of the class declaration, which is syntactically invalid and not a real-world scenario.

Fixed by pressing Ctrl+Shift+Enter before typing the method:

http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/?id=4b8bbff98ccf9f5c9422558a8f53d6ea93f91995
Comment 8 Markus Keller CLA 2012-04-17 10:08:57 EDT
.. and reverted my bad change to MEASURED_RUNS:
http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/?id=6444a8825b9e7422d30dfef1ee64206a632f8441