Summary: | [remotecdt] Further improve progress reporting and cancellation of Remote CDT Launch | ||||||
---|---|---|---|---|---|---|---|
Product: | [Tools] CDT | Reporter: | Martin Oberhuber <mober.at+eclipse> | ||||
Component: | cdt-debug | Assignee: | Anna Dushistova <anna.dushistova> | ||||
Status: | RESOLVED FIXED | QA Contact: | Martin Oberhuber <mober.at+eclipse> | ||||
Severity: | enhancement | ||||||
Priority: | P3 | CC: | johann.draschwandtner | ||||
Version: | 6.0 | Keywords: | contributed | ||||
Target Milestone: | 6.0 | Flags: | mober.at+eclipse:
review+
|
||||
Hardware: | PC | ||||||
OS: | Windows XP | ||||||
Whiteboard: | |||||||
Attachments: |
|
Description
Martin Oberhuber
2008-06-03 06:31:03 EDT
(In reply to comment #0) I'm afraid we cannot change it: it comes from org.eclipse.debug.internal.core.LaunchConfiguration. > (3) When connecting as part of the Launch asks for Password, it does so at 57% > progress already. Should check whether the SubProgressMonitor stuff can > be changed to do this at a lower level -- but that's really minor Have you tried doing a beginTask(1000) rather than beginTask(100) at the beginning of the Launch? -- Should bring 57% down to 6% Why do you think so? I think that it depends of the "relative size" of monitor passed to launch(...). delegate.launch(this, mode, launch, new SubProgressMonitor(monitor, 10)); And it is the last task with monitor. But actually I've just tried--it doesn't help, even with 1000 I have the same 57%. (In reply to comment #2) > Have you tried doing a beginTask(1000) rather than beginTask(100) at the > beginning of the Launch? -- Should bring 57% down to 6% > Hm... I think you're right. When our launch gets 10 ticks assigned from the master monitor, we have no chance whatsoever changing the 57% thing, regardless of how many steps we separate the 10 ticks into. We might perhaps be able to do something via the ILaunchDelegate2#preLaunch() and similar hooks. But it's likely not worth exploring. There are a few more hooks than just launch() which are called before the actual launch. They are meant to build projects etc, which acan be long running, I assume that's why we start at 57%. Created attachment 104159 [details]
patch that fixes some issues mentioned
I, Anna Dushistova, declare that I developed attached code from scratch,
without referencing any 3rd party materials except material licensed under the
EPL. I am authorized by my employer to make this contribution under the EPL.
Comment on attachment 104159 [details]
patch that fixes some issues mentioned
The changes you made are all good or harmless, feel free to commit -- don't forget to set the iplog+ flag on the attachment, and the "contributed" flag on the bug, and edit the tm-log.csv file (because the patch was attached before you became committer).
From reviewing the patch, I found two additional things that you could do after committing this patch (and without iplog+ since you are committer now):
1.) remoteShellExec() is always called with a SubProgressMonitor, so I believe that inside remoteShellExec() you must have monitor.beginTask() / monitor.done() statements. If you beginTask() with 10 work units, it will fit the 7 + 3 separation you made for the SubProgressMonitors.
2.) I'm surprised that you don't check monitor.isCanceled() anywyere. Perhaps it's not necessary since RSE's long running sub tasks do that, though. Just test it and verify that the operation can always be canceled (well I think I checked that some while back and it was fine, so probably it's really not an issue at all).
(In reply to comment #7) > From reviewing the patch, I found two additional things that you could do after > committing this patch (and without iplog+ since you are committer now): I'll leave this bug open for looking into these things then. Comment on attachment 104159 [details] patch that fixes some issues mentioned The "iplog+" must be on the patch, and not on the bug. See http://wiki.eclipse.org/Development_Resources/Automatic_IP_Log#Contributors I fixed 1) and checked 2)--it works fine for me. (In reply to comment #7) > From reviewing the patch, I found two additional things that you could do after > committing this patch (and without iplog+ since you are committer now): > > 1.) remoteShellExec() is always called with a SubProgressMonitor, so I believe > that inside remoteShellExec() you must have monitor.beginTask() / > monitor.done() statements. If you beginTask() with 10 work units, it will fit > the 7 + 3 separation you made for the SubProgressMonitors. > > 2.) I'm surprised that you don't check monitor.isCanceled() anywyere. Perhaps > it's not necessary since RSE's long running sub tasks do that, though. Just > test it and verify that the operation can always be canceled (well I think I > checked that some while back and it was fine, so probably it's really not an > issue at all). > |