Bug 88653 - [Commands] performance: 50% of perspective switch in ParameterizedCommand.equals()
Summary: [Commands] performance: 50% of perspective switch in ParameterizedCommand.equ...
Status: VERIFIED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 3.1   Edit
Hardware: All All
: P1 major (vote)
Target Milestone: 3.1 M7   Edit
Assignee: Douglas Pollock CLA
QA Contact:
URL:
Whiteboard:
Keywords: performance
Depends on:
Blocks:
 
Reported: 2005-03-21 12:06 EST by John Arthorne CLA
Modified: 2005-05-11 14:52 EDT (History)
2 users (show)

See Also:


Attachments
Profiler trace output (43.87 KB, text/plain)
2005-03-21 12:07 EST, John Arthorne CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description John Arthorne CLA 2005-03-21 12:06:15 EST
Build: I20050315

Setup:

1) Workspace with three projects, org.eclipse.jdt.core, org.eclipse.swt, and
org.eclipse.swt.win32
2) Editor open on org.eclipse.jdt.internal.compiler.parser.Parser.java
3) Default Java perspective and Resource perspective open

My test was to switch between the two open perspectives about five times using
Ctrl+F8.  The profiler showed 50% of the time in ParameterizedCommand.equals()
and in the methods called by that equals implementation (Util.equals() and
Command.equals()).

The main problem is probably in the number of times this method is called (about
1.6 million times). This seems to be caused by a large number of property change
events:

ExternalActionManager$CommandCallback.bindingManagerChanged is called 30 times.
This in turn calls ActionContributionItem$1.propertyChange, which is called 4787
times.  The number of method calls explodes as you go down the call stack, until
the equals method that is called 1.6 million times.  I will attach profiler
output to show more details.

If the number of change events can't be reduced, it looks like the
implementation of ParameterizedCommand.equals() can be made faster by first
comparing the cached hash code.  Only if the hash codes are the same does it
need to do the remainder of the equality testing.
Comment 1 John Arthorne CLA 2005-03-21 12:07:41 EST
Created attachment 19052 [details]
Profiler trace output
Comment 2 Douglas Pollock CLA 2005-04-07 09:40:30 EDT
Investigate for 3.1 M7.
Comment 3 Douglas Pollock CLA 2005-04-07 15:58:01 EDT
The problem appears to be a typo in the hashCode() method of
ParameterizedCommand.  This is corrected in CVS.  Let me know if you see the
problem again.
Comment 4 John Arthorne CLA 2005-04-08 16:17:35 EDT
Perspective switching feels much snappier in the profiler.  The old hotspots are
completely eliminated:

Before patch (% of total time):
------------------------------
commands.util.Util.equals                 20.29%
commands.Command.equals                   14.82%
commands.ParameterizedCommand.equals      13.03%
java.util.Collections$UnmodifiableMap.get  5.17%

After patch:
-------------------------------
commands.util.Util.equals                  0.44%
commands.Command.equals                    0.23%
commands.ParameterizedCommand.equals       0.46%
java.util.Collections$UnmodifiableMap.get  1.13%

The hotspots during perspective switch are now in SWT code:

Widget.setWindowPos  4.05%
OS.CallWindowProc    2.21%
Control.windowProc   1.80%

It will be interesting to see how big of an impact this has on the automated
perspective switching tests.
Comment 5 Nick Edgar CLA 2005-04-12 11:54:40 EDT
> This seems to be caused by a large number of property change events

Was anything done to reduce the number of property change events?  Or does this
no longer matter?
Comment 6 Douglas Pollock CLA 2005-04-12 12:01:34 EDT
No, the property change events are still fired for every command that has changed.

As a note, the "50%" is misleading.  It is an artifact of the profiler.  The
real savings is actually quite small.  In the order of 5 +- 5 %.  It's small
enough that is almost completely obscured by the standard deviation.
Comment 7 Douglas Pollock CLA 2005-05-11 14:52:41 EDT
Verified by checking how often ParameterizedCommand is called on perspective
switch.  It appears to be called only zero or one times, which is much better
than > 10000 times.