Bug 17162 - ManageBreakpointActionDelegate triggers reconcile on selection change
Summary: ManageBreakpointActionDelegate triggers reconcile on selection change
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Debug (show other bugs)
Version: 2.0   Edit
Hardware: PC Windows 2000
: P2 critical (vote)
Target Milestone: 2.0 F2   Edit
Assignee: Darin Swanson CLA
QA Contact:
URL:
Whiteboard:
Keywords: performance
Depends on:
Blocks:
 
Reported: 2002-05-23 05:47 EDT by Erich Gamma CLA
Modified: 2002-05-28 21:36 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 Erich Gamma CLA 2002-05-23 05:47:52 EDT
The ManageBreakpointActionDelegate forces a reconcile of a Java compilation unit 
on a selection changed event (see trace below). A reconcile is an expensive 
operation for a large compilation unit. It is not clear why it is needed. The 
selection change updates the label to either Add/Remove. To do so you should not 
have to know which element the selection is at. To reduce the updating cost the 
label could just be changed to Toggle Breakpoint.

All references to ActionDelegateHelper.getCurrentMemember should be checked.

        at 
org.eclipse.jdt.internal.core.WorkingCopy.reconcile(WorkingCopy.java:357)
        at 
org.eclipse.jdt.internal.debug.ui.actions.ActionDelegateHelper.getCurrentMember(
ActionDelegateHelper.java:138)
        - locked <055BF888> (a org.eclipse.jdt.internal.core.WorkingCopy)
        at 
org.eclipse.jdt.internal.debug.ui.actions.AbstractManageBreakpointActionDelegate
.update(AbstractManageBreakpointActionDelegate.java:45)
        at 
org.eclipse.jdt.internal.debug.ui.actions.ManageBreakpointActionDelegate.selecti
onChanged(ManageBreakpointActionDelegate.java:179)
Comment 1 Darin Wright CLA 2002-05-24 14:30:05 EDT
Can we get rid of the reconcile?
Comment 2 Erich Gamma CLA 2002-05-26 14:23:45 EDT
Just to emphasize the importance. This can result in 3-5 secondes delays when 
working on a large compilation unit. Also any action which triggers a selection 
changed (e.g. shift left) slows down by the same amount.
Comment 3 Darin Swanson CLA 2002-05-26 20:49:15 EDT
I am suprised that this is occurring so frequently...the actions attempt to not 
force the reconcile unless the selection moves into a source range that is not 
contained within the current type which is cached.  So unless there is errors 
in this logic (which I will check tomorrow), within a large compilation unit I 
would not expect any time penalty changing the selection as you are not likely 
moving outside of the type. 

The reason we moved to the "reconcile" is that we were getting 
JavaModelExceptions attempting to get the element at a specific location within 
a compilation unit.  Is there a better way around this?  

We could take the reconcile out and just handle the exception.
Comment 4 Erich Gamma CLA 2002-05-27 07:08:11 EDT
A reconcile should never be triggered just for enabling actions, it is too 
expensive (even if it is rare). For example, when I select a block and do a 
shift left you will always do a reconcile.

>We could take the reconcile out and just handle the exception.
why does adding/removing a breakpoint have to know about the method containing 
the selection? All you need to know is whether there is already a breakpoint at 
this particular line. I would also tolerate a less dynamic menu time that just 
says Toggle Breakpoint. 
Comment 5 Darin Swanson CLA 2002-05-27 15:06:21 EDT
We are actually looking for the "member" that is relevant...for a breakpoint it 
is a type, for add watchpoint it is a field and for add method it is a method.

I agree that until contributed menu items have a better mechanism for updating 
beside selection changed callbacks we will move the actions to only do work 
when the user explictly invokes them.

So we will have Toggle Breakpoint, Toggle Watchpoint, Toggle Method Breakpoint.
Comment 6 Darin Swanson CLA 2002-05-28 13:12:36 EDT
Reworked the action hierarchy.  A reconcile will only happen when the user has 
explictly asked to add/remove a breakpoint (is "running" the action) and this 
will only happen when the current member no longer contains the current 
selection.
Comment 7 Darin Swanson CLA 2002-05-28 13:15:45 EDT
Please verify (DarinW)
Comment 8 Darin Wright CLA 2002-05-28 17:09:01 EDT
Verified reconcile happens on "add/remove" only.

I changed the action labels to "Add/Remove..." rather than "Manage a...". I 
think users will understand this better.

I still have a bug with watchpoints:
(1) When the cursor is in a method, the "Add/Remove Watchpoint" action is 
enabled (and beeps when invoked).
(2) When I select a field in the outliner, the pop-up "Add/Remove Watchpoint" 
action is enabled, but the action on the Run menu is not.
Comment 9 Darin Swanson CLA 2002-05-28 17:49:48 EDT
(1) I don't think there is any way to do better here...how can I know whether I 
can add remove a watchpoint unless I have resolved where I am based on the 
selection and therefore would need to reconcile...

(2) This is a bug.
Comment 10 Darin Swanson CLA 2002-05-28 20:48:15 EDT
Improved the actions some more.
Comment 11 Darin Swanson CLA 2002-05-28 20:48:27 EDT
Please verify (DarinW).
Comment 12 Darin Wright CLA 2002-05-28 21:04:27 EDT
I just got this:

!MESSAGE Reference action not found: additions
!ENTRY org.eclipse.ui 4 2 May 28, 2002 20:13:58.119
!MESSAGE Problems occurred when invoking code from plug-in: org.eclipse.ui.
!STACK 0
java.lang.NullPointerException
	at 
org.eclipse.jdt.internal.debug.ui.actions.ManageWatchpointActionDelegate.setEnab
ledState(ManageWatchpointActionDelegate.java:279)
	at 
org.eclipse.jdt.internal.debug.ui.actions.AbstractManageBreakpointActionDelegate
.update(AbstractManageBreakpointActionDelegate.java:112)
	at 
org.eclipse.jdt.internal.debug.ui.actions.ManageBreakpointActionDelegate.selecti
onChanged(ManageBreakpointActionDelegate.java:174)
	at org.eclipse.ui.internal.PluginAction.refreshEnablement
(PluginAction.java:170)
	at org.eclipse.ui.internal.PluginAction.selectionChanged
(PluginAction.java:232)
	at org.eclipse.ui.internal.PluginAction.selectionChanged
(PluginAction.java:252)
	at org.eclipse.ui.internal.AbstractSelectionService$3.run
(AbstractSelectionService.java:137)
	at org.eclipse.core.internal.runtime.InternalPlatform.run
(InternalPlatform.java:802)
	at org.eclipse.core.runtime.Platform.run(Platform.java:416)
	at org.eclipse.ui.internal.AbstractSelectionService.fireSelection
(AbstractSelectionService.java:135)
	at org.eclipse.ui.internal.AbstractSelectionService.reset
(AbstractSelectionService.java:291)
	at org.eclipse.ui.internal.AbstractSelectionService.partDeactivated
(AbstractSelectionService.java:274)
	at org.eclipse.ui.internal.WWinPartService.partDeactivated
(WWinPartService.java:124)
	at org.eclipse.ui.internal.PartListenerList$4.run
(PartListenerList.java:97)
	at org.eclipse.core.internal.runtime.InternalPlatform.run
(InternalPlatform.java:802)
	at org.eclipse.core.runtime.Platform.run(Platform.java:416)
	at org.eclipse.ui.internal.PartListenerList.firePartDeactivated
(PartListenerList.java:95)
	at org.eclipse.ui.internal.WorkbenchPage.firePartDeactivated
(WorkbenchPage.java:1085)
	at org.eclipse.ui.internal.WorkbenchPage.setActivePart
(WorkbenchPage.java:2068)
	at org.eclipse.ui.internal.WorkbenchPage.setPerspective
(WorkbenchPage.java:2127)
	at org.eclipse.ui.internal.WorkbenchPage.busySetPerspective
(WorkbenchPage.java:566)
	at org.eclipse.ui.internal.WorkbenchPage.access$6
(WorkbenchPage.java:554)
	at org.eclipse.ui.internal.WorkbenchPage$9.run(WorkbenchPage.java:2191)
	at org.eclipse.swt.custom.BusyIndicator.showWhile(BusyIndicator.java:56)
	at org.eclipse.ui.internal.WorkbenchPage.setPerspective
(WorkbenchPage.java:2189)
	at org.eclipse.ui.internal.Workbench.showPerspective
(Workbench.java:1167)
	at 
org.eclipse.debug.internal.ui.launchConfigurations.PerspectiveManager$1.run
(PerspectiveManager.java:133)
	at org.eclipse.swt.widgets.RunnableLock.run(RunnableLock.java:29)
	at org.eclipse.swt.widgets.Synchronizer.runAsyncMessages
(Synchronizer.java:93)
	at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java
(Compiled Code))
	at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java
(Compiled Code))
	at org.eclipse.ui.internal.Workbench.runEventLoop(Workbench.java
(Compiled Code))
	at org.eclipse.ui.internal.Workbench.run(Workbench.java:1068)
	at org.eclipse.core.internal.boot.InternalBootLoader.run
(InternalBootLoader.java:739)
	at org.eclipse.core.boot.BootLoader.run(BootLoader.java:462)
	at java.lang.reflect.Method.invoke(Native Method)
	at org.eclipse.core.launcher.Main.basicRun(Main.java:200)
	at org.eclipse.core.launcher.Main.run(Main.java:643)
	at org.eclipse.core.launcher.Main.main(Main.java:476)
Comment 13 Darin Wright CLA 2002-05-28 21:14:46 EDT
The active part can be null according to the doc.
Comment 14 Darin Wright CLA 2002-05-28 21:22:00 EDT
Fixed. (Re-open to re-assign)
Comment 15 Darin Wright CLA 2002-05-28 21:22:16 EDT
Please verify null check on active part.
Comment 16 Darin Wright CLA 2002-05-28 21:22:27 EDT
Fixed (again)
Comment 17 Darin Swanson CLA 2002-05-28 21:36:21 EDT
Verified.